Re: ReaderWriterLockSlim + Dispose?

On Fri, 04 Apr 2008 21:22:31 -0700, NvrBst <nvrbst@xxxxxxxxx> wrote:

Do you mean that the examples use ReaderWriterLockSlim but don't dispose  
it when the code is done with it?  That's a bug.

http://msdn2.microsoft.com/en-us/library/system.threading.readerwriterlockslim.aspx

Well, as you probably can guess...I think this sample is incomplete, if not buggy.

http://msdn2.microsoft.com/en-us/library/bb355025.aspx

This example stores the instance in a static variable, so IDisposable wouldn't apply.

http://www.bluebytesoftware.com/blog/PermaLink,guid,c4ea3d6d-190a-48f8-a677-44a438d8386b.aspx

This doesn't even include compilable code, so I'd hardly say you should expect it to be complete. Lack of any specific code, IDisposable implementation or otherwise, shouldn't be in any way inferred to mean it can be left out in real code.

To name a few. The 1st MSDN example displays the entire class with no
dispose in it. If you google / google code search
"ReaderWriterLockSlim" I don't think you'll be able to find a code
snipplet or example that uses dispose with RWLS, which I found very
strange. This made me believe that there is something I'm missing?

I can't say whether you're missing something. I do know that a quick Google search yielded this link:
http://blogs.msdn.com/vancem/archive/2006/03/29/564854.aspx

Check out the comments starting with Bill Menees's first comment. It doesn't pertain to ReaderWriterLockSlim per se, but it does perhaps give some insight as to why a class might have disposable stuff in it but not implement IDisposable, or why a class that does implement IDisposable might be considered "safe" to not dispose of.

I'm not saying it's an answer as to why you shouldn't dispose ReaderWriterLockSlim. I'm still of the opinion that a class that implements IDisposable should be disposed of when you're done. But not everyone is necessarily going to agree with my viewpoint.

[...] Since the
client made the macro(script) it could be doing some kind of huge loop
and don't want to wait for it. Cleaning up could result in that
thread trying to use the lock, which is annoying in the case of RWLS-
if its the only disposable thing in the application. With the thread
pool, any situation you don't want to wait for the threads to finish
before cleaning up could result in crashes if you don't check if the
lock is disposed before acquiring it.

I still don't see the problem. For me, it's a fundamental problem for you to start cleaning stuff up before things are done with them. In your example, why does the lock exist? Presumably, to ensure that the multiple macro-processing threads are properly synchronized with each other. So if you dispose that lock before those threads are done, and the threads simply avoid taking the lock if it's been disposed, now you have a situation in which the threads aren't synchronized.

If that's safe now, it was safe before and you didn't need the lock. If it's not safe, then your decision to clean stuff up without terminating the threads created a synchronization bug. Either way, that's showing us that the hypothetical design is wrong in the first place. It's not an example of why disposing a lock class is a problem.

In that particular example, you either need to decide whether you're going to interrupt the threads that need the lock, or you're going to delay cleanup of the lock until the threads are all done. You can't let the threads run _and_ clean up the lock right away.

[...]
Which was another question. Does it really matter if these
"WaitHandles" don't get cleaned up for a while? How does it affect
other things if at all? (Other than maybe some memory that lingering
till GC Collects / AppDomain unloads?). I don't see these WaitHandles
being very big though memory wize though.

Well, generally speaking, it's not so much the size of the object as it is the size of the OS data structures that track the object. I suspect that these issues are _much_ alleviated since the old 16-bit Windows days. However, the OS does continue to provide dispose semantics for these objects, and I think it's unwise to make any assumptions that they are optional.

Could you get away with it? Probably. The fact is, a lot of these sorts of things only wind up biting you some small fraction of the time you ignore them. But that doesn't mean it's generally acceptable design to do so. If anything, it's why you should _always_ be careful to follow the conventions imposed on you by the design, because it can be really difficult to reproduce potential problems at will. This means the problems often come up when you are least prepared or able to deal with them.

I was thinking maybe the other implementations use WaitHandles and
just don't clean them up? How many ways can there be for locks to
wait on an event :)

Well, not all synchronization objects use events. Now, whether it's possible to write a synchronization object without using _any_ disposable unmanaged resources, I can't personally say. But inasmuch as managed code runs with the complete support of the .NET run-time, it seems plausible to me that there could be synchronization objects that have no need of being disposed.

[...]
:) Thanks for the comments, I'm still a little curious on information
though about the topic.

And I hope you get them. I agree that getting more details on the specifics would be useful and interesting.

But I'm still going to suggest that you always dispose classes that implement IDisposable. :) It's one thing for the authors of .NET, who have intimate knowledge with the inner workings of the run-time and the framework, to make a choice to not implement IDisposable even though a class might benefit from it. It's yet another for the rest of us to make decisions contrary to the accepted conventions exposed by .NET. You do so at your own peril. :)

Pete
.

Relevant Pages

  • Re: InvokeRequired/BeginInvoke() on Disposed control?
    ... > already disposed object. ... > Initialize the flag to false.Make a lock object for the closing flag. ... want to use the lock are in the Dispose() override and in the callback... ...
    (microsoft.public.dotnet.languages.csharp)
  • Re: InvokeRequired/BeginInvoke() on Disposed control?
    ... > already disposed object. ... > Initialize the flag to false.Make a lock object for the closing flag. ... want to use the lock are in the Dispose() override and in the callback... ...
    (microsoft.public.dotnet.framework.windowsforms)
  • Re: Request review of this simple threading code
    ... Your code will deadlock upon Dispose. ... does not release that lock until the thread has stopped. ... private bool _stopping = false; ... private void _startReadingData ...
    (microsoft.public.dotnet.languages.csharp)
  • Re: Disposing of pressure treated wood
    ... Lock it up as opposed to what, burying it so somebody's kids ... dispose of the hazard at all. ... Regading the PT lumber scraps, I am reminded of an old saying ...
    (rec.woodworking)
  • Re: Calling DataAdapter.Update() on disposed DataSet?
    ... While it may be safe in the dataset, I'm just afraid that down the line some developer will see this happening in our lower level library, think it's a great short cut to skirt having to dispose of your objects properly and start copying the behaviour with impunity. ... Which is why I kind of wanted to get all of that kind of code out of our lower level libraries even if in the end it makes absolutly no difference. ...
    (microsoft.public.dotnet.framework.adonet)