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
}
Was it helpful?

Solution

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
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top