Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Just how 'disposable' is ReaderWriterLockSlim?

I principally follow the IDisposable pattern, and for most classes that is justified. But ReaderWriterLockSlim made me question the viability of applying such pattern. All ReaderWriterLockSlim.Dispose does is close some event handles. So how important is it to Dispose such class with so few resources? In this case, I really wouldn't mind if the GC had to wait another round for the finalizers of the unmanaged resources to finish.

The consequence for applying the IDisposable pattern is considerable however, every class that uses a disposable class now has to implement IDisposable too. In my particular case, I am implementing a wrapper for HashSet. I don't particularly expect the requirement to dispose such object because, accidently, it uses a synchronizer which does.

Are there any reasons not to violate the disposable pattern in this case? While I am eager to, I wouldn't do so in practice, because violating consistency is much worse.

like image 857
toplel32 Avatar asked Dec 30 '14 16:12

toplel32


2 Answers

The problem with unmanaged OS handles is that handles come from a limited supply. The GC is not aware of this.

The pure memory consumption of a handle is not that big. Nothing more than an object in kernel memory and probably hash table entry somewhere.

You are right in that it is not enough to say: "You must always dispose all disposable objects". That rule is too simple. For example the Task class does not need to be disposed. If you know what you are doing you can take a looser stance regarding disposal. Be aware that not all team members might understand this point (now you can leave a link to this answer in the source code...).

If you are sure that you will not leak a lot of handles you can safely do this. Be aware that under edge conditions (load, bugs, ...) you might leak more that you anticipated causing production issues.

like image 91
usr Avatar answered Oct 16 '22 10:10

usr


If this field is static you don't need to dispose of it, it will (righty) have the same lifetime as your application. I see it's not, lets move on.

The correct way to handle an IDisposable is to dispose of it. I think we need a good reason not do this.

Use another lock:

I think the best thing to do is to use Monitor or another lock, which will have the bonus of simplifying your code as well. ConcurrentDictionary and other framework classes seem to take this approach.

You are worried about lock conveys, but I'm not sure this is even solved by ReaderWriterLockSlim, the only real solution is to hold less locks and hold them for less time.

Don't dispose:

This needs a justification. Can you demonstrate needed performance benefits here?

If you have a few of these objects that are long lived, fine, not all disposables are equally weighty (it's not like your leaving a word document open), you will probably get away with it. As it has been pointed out, what is the point of disposing of all this milliseconds before the application closes anyway. I believe the destructor of an IDisposable is in meant to handle situations where the object is not disposed, although you can't be sure when or even if this is called.

If you have a long lived applciation with lots of short-lived usages of this class, however, you may run into trouble. You are baking in your assumptions about the use of your code, just be aware.

like image 39
Nathan Cooper Avatar answered Oct 16 '22 11:10

Nathan Cooper