IDisposable and ReaderWriterLockSlim

Question

I have a class MyClass. This class have a field: public ReaderWriterLockSlim rw; (public for an easier example code). Many threads can read data from MyClass using rw.EnterReadLock etc.

Also I have implemented IDisposable interface:

private void Dispose (bool pDisposing)
{
    {
        if (pDisposing) // release managed resources
        {
            if (rw != null)
            {
                rwLockSlim.Dispose ();
                rwLockSlim = null;
            }
        }

        //--- release unmanaged resources
        // some code...
        isDisposed = true; // ...
    }
}

Like you see, problem is when one thread is using MyClass when second thread call Dispose on myClass object. I cannot dispose ReaderWriterLockSlim because it will crash my application. So should I just remove that lines which releases managed resources? Anyway that ReaderWriterLockSlim will be collected by GC in near future, right? (but is this class resource expensive?).
Maybe I should add some lock(syncObject) in Dispose metod or something?

EDIT: also I deal with AllocHGlobal, so I need to wait, until all threads will stop reading/writing to myClass.

Different point of view:

public MyClass : IDisposable
{
            public void EnterReadLock (); // calls rwLockSlim.EnterReadLock,  
                                          // if object is disposed throws Exception

            public void ExitReadLock ();  // same as above

            public void Dispose ();       // wait until all threads exit from locks,
                                          // frees unamanged resources, mark class as disposed
}

score 1 · Accepted Answer · answered Feb 03 '13 at 11:09

This may not be the best answer, rather observations and some thoughts.

Can you simplify your code? I mean, other types shouldn't care if your type throws exceptions under specific concurrency conditions. How will you test this? Public lock object is evil. It must be private unless you want to spent months trying to figure out mysterious bugs.

If a dispose method is called, it means that no other objects should ever use this object. Which means you must ensure first that all threads finished operating on the object first, before calling a dispose method.

Suggestions

Make the lock private
Ensure all threads are finished, before calling to dispose
Add timeouts for safe behaviour
Consider using static lock object
ReaderWriterLockSlim is not resource expensive (to the point you should care about in your application)
If your class uses disposable resources, it is usually better to implement IDisposable and dispose members that need to be disposed.
Don't add any locks in the Dispose method, it shall be simple. This may introduce unpleasant bugs. Because if you forget to call a Dispose manually, it will be non-deterministically called by GC (through a finiliser, which you should add as well to call Dispose)
Correct approach would be to wait until all threads are done and dispose an object

Public ReaderWriterLockSlim was only for example purpose. Like you see in a second example, I made method EnterReadLock() that calls rwLockSlim.EnterReadLock(). Anyway thanks for all suggestions in your answer. — apocalypse, Feb 03 '13 at 11:24
I would also try to avoid a construct like this. From what I can tell it looks like you use a single instance of the `MyClass` class in multiple threads, which is fine. But if you do that then make the methods thread safe and do not rely on other code executing `EnterReadLock` and `ExitReadLock` properly. — Dirk, Feb 03 '13 at 11:25
There are some cases where one wants to wait for threads to finish before calling `Dispose`, but there are some blocking APIs (for things like communication) where `Dispose` is the *only* method which may be called on an object which is waiting for something to happen (on some hardware devices, once an operation is requested, there's no way to cancel it without leaving the device in a state which will be unusable until the device is reconfigured). Under that pattern, calling `Dispose` on an object which is busy should cause the operation that was in progress to throw an exception. — supercat, Oct 29 '13 at 17:15

IDisposable and ReaderWriterLockSlim

1 Answers1