C# Using block with an anonymous method referencing the IDisposable object

Question

Consider the following code:

using (var mre = new ManualResetEvent(false))
{
     var bgWkr = new BackgroundWorker();
     bgWkr.DoWork += delegate(object sender, DoWorkEventArgs e)
     {
         var mrEvent = e.Argument as ManualResetEvent;
         // some processing...

         mrEvent.WaitOne();
         // broadcast an event
     };
     bgWkr.RunWorkerAsync(mre);

     // some other processing...

     // hook into the same event
     mre.Set();
}

Let's say that the spawned worker takes a bit of time to complete. We will have left the using block a while ago by the time the worker thread finishes and waits on the ManualResetEvent. I would assume that the mre would have been closed when leaving the using block (given that it would have been disposed) and this would throw an exception at the very least. Is this a safe assumption to make?

This example may not be the best one with the ManualResetEvent but it is to illustrate the case where we access an IDisposable object inside an anonymous method within a using block and the anonymous method is called after we have exited the using block. Is there some mechanism that keeps hold of the disposable object? I don't believe so but would like some confirmation as to why (if there is some sort of voodoo at work) or why not.

Cheers,

score 1 · Accepted Answer · answered Sep 13 '13 at 08:45

Yes, this code is wrong - the outcome is not really defined, but it would be quite reasonable for it to throw an exception at the mrEvent.WaitOne(), since mrEvent is the almost-certainly-now-disposed ManualResetEvent. Technically there's a chance that the worker thread was all ready to go, and the worker thread did its "some processing..." faster than the primary thread did the "some other processing...", but: I wouldn't rely on it. So in most cases: mrEvent is dead already.

As for how to avoid this: perhaps this simply isn't a scenario for using. But it occurs that since the worker thread does a WaitOne, the worker thread's WaitOne cannot complete before the primary thread performs the mre.Set() call - so you could exploit that and move the using to the worker:

 var mre = new ManualResetEvent(false);
 var bgWkr = new BackgroundWorker();
 bgWkr.DoWork += delegate(object sender, DoWorkEventArgs e)
 {
     using(var mrEvent = e.Argument as ManualResetEvent)
     {
         // some processing...

         mrEvent.WaitOne();
     }
     // broadcast an event
 };
 bgWkr.RunWorkerAsync(mre);

 // some other processing...

 // hook into the same event
 mre.Set();

Note, however, that this raises an interesting question of what happens if the primary thread throws an exception in the "some other processing..." - the call to mre.Set() would never be reached, and the worker thread would never exit. You might want to do the mre.Set() in a finally:

 var mre = new ManualResetEvent(false);
 try {
     var bgWkr = new BackgroundWorker();
     bgWkr.DoWork += delegate(object sender, DoWorkEventArgs e)
     {
         using(var mrEvent = e.Argument as ManualResetEvent)
         {
             // some processing...

             mrEvent.WaitOne();
         }
         // broadcast an event
     };
     bgWkr.RunWorkerAsync(mre);

     // some other processing...
 }
 finally {
    // hook into the same event
    mre.Set();
 }

I never considered putting the using in the worker, very clever. I don't have the using in the 1st thread either. In fact I am not cleaning up the mre because in my case the WaitOne() is not in a single BackgroundWorker but rather many threads can call the WaitOne() whereas the Set() only occurs once. I read however that we can sometimes forego the cleanup of the ManualResetEvent if need be. http://stackoverflow.com/a/2358158/762688 — sean.net, Sep 13 '13 at 08:57

score 0 · Answer 2 · answered Sep 13 '13 at 09:32

In response to my comment (rather than proposing the answer to the question), I created a class to close the ManualResetEvent once done with it without the need to track when the last thread has finished using it. Thanks to Marc Gravell for the idea to close it once the WaitOne has completed. I am exposing it here should anybody else need it.

P.S. I'm constrained to .NET 3.5... hence why I am not using the ManualResetEventSlim.

Cheers,

Sean

public class OneTimeManualResetEvent
{
    private ManualResetEvent _mre;
    private volatile bool _closed;
    private readonly object _locksmith = new object();

    public OneTimeManualResetEvent()
    {
        _mre = new ManualResetEvent(false);
        _closed = false;
    }

    public void WaitThenClose()
    {
        if (!_closed)
        {
            _mre.WaitOne();
            if (!_closed)
            {
                lock (_locksmith)
                {
                    Close();
                }
            }
        }
    }

    public void Set()
    {
        if (!_closed)
            _mre.Set();
    }

    private void Close()
    {
        if (!_closed)
        {
            _mre.Close();
            _closed = true;
        }
    }
}

C# Using block with an anonymous method referencing the IDisposable object

2 Answers2