Is it safe to catch ObjectDisposedException on ManualResetEvent.WaitOne()?

Question

This is closely related to Is it safe to signal and immediately close a ManualResetEvent? and might provide one solution to that problem.

Say I have a bunch of threads that potentially want to do the same work, but only one should be allowed to do it, and the others should wait until the worker is done and use its result.

So basically do I want work to be done only once.

Update: Let me add that this is not an initialization problem that could be addressed using .net 4's Lazy<T>. By once I mean once per task, and those tasks are determined at runtime. This might not be clear from the simplified example below.

Modifying the simple example from Hans Passant's answer to aforementioned question a bit, I guess the following would be safe. (It's slightly different from the use case just described, but in terms of threads and their relations, it is equivalent)

static void Main(string[] args)
{
    ManualResetEvent flag = new ManualResetEvent(false);
    object workResult = null;
    for (int ix = 0; ix < 10; ++ix)
    {
        ThreadPool.QueueUserWorkItem(s =>
        {
            try
            {
                flag.WaitOne();
                Console.WriteLine("Work Item Executed: {0}", workResult);
            }
            catch (ObjectDisposedException)
            {
                Console.WriteLine("Finished before WaitOne: {0}", workResult);
            }
        });
    }
    Thread.Sleep(1000);
    workResult = "asdf";
    flag.Set();
    flag.Close();
    Console.WriteLine("Finished");
}

I guess the core of my question is:

Is a call to WaitOne, aborted due to a ObjectDisposedException, equivalent to a successful call to WaitOne, in terms of memory barriers?

That should ensure safe access to the variable workResult by those other threads.

My guess: It would have to be safe, otherwise how could WaitOne safely figure out that the ManualResetEvent object had been closed in the first place?

Now, years later, I wonder why I bothered calling `Close()` in the first place: I [might as well just let the GC handle closure](http://stackoverflow.com/a/2358158/709537) at some undefined point in the future. — Evgeniy Berezovsky, Sep 24 '15 at 05:26

Anastasiosyal · Accepted Answer · 2012-02-17T11:16:10.240

Here is what I see:

You are getting ObjectDisposedException because your code exhibits the following race condition:
- flag.close can be called before all threads have managed to invoke flag.waitOne.

How you handle this depends on how important the execution of the code is after flag.waitOne.

Here is one approach:

If all the threads that have been spun up really should execute, you could have some extra synchronisation before calling flag.close. You could achieve this by using the StartNew on Task.Factory instead of Thread.QueueUserWorkItem. Tasks can be waited upon for completion and then you would invoke the flag.close thus eliminating the race condition and the need to handle the ObjectDisposedException

Your code would then become:

    static void Main(string[] args)
    {
        ManualResetEvent flag = new ManualResetEvent(false);
        object workResult = null;
        Task[] myTasks = new Task[10];
        for (int ix = 0; ix < myTasks.Length; ++ix)
        {
            myTasks[ix] = Task.Factory.StartNew(() =>
            {
                flag.WaitOne();
                Console.WriteLine("Work Item Executed: {0}", workResult);
            });
        }

        Thread.Sleep(1000);
        workResult = "asdf";
        flag.Set();
        Task.WaitAll(); // Eliminates race condition
        flag.Close();
        Console.WriteLine("Finished");
    }

As you can see above, Tasks allow for extra synchronization that would eliminate the race condition that you are seeing.

As an extra note, ManualResetEvent.waitOne performs a memory barrier, so the workresult variable will be the latest one updated without requiring any further memory barriers or volatile reads.

So to answer your question, if you reaaaaally must avoid additional synchronisation and handle an ObjectDisposed exception by going with your approach, I would argue that a disposed object has not performed a memory barrier for you, you would have to call Thread.MemoryBarrier in your catch block to ensure that the latest value has been read.

But exceptions are expensive and if you can avoid them for normal program execution I do believe that it is prudent to do so.

Good luck!

+1 Thanks for this great answer, that would fix the example I've given and provides good additional advice. I guess the problem with my example is, that it is too simple. Unfortunately, I cannot use Tasks, as I'm on 3.5 (should have specified). Memory barrier issues btw are being taken care of by access to (the same entry in) a ConcurrentDictionary by all threads involved. In any event, your answer actually led me to look into the other concurrency constructs provided by .net, like Monitor wait/pulseAll etc. — Evgeniy Berezovsky, Feb 20 '12 at 01:35

score 0 · Answer 2 · answered Feb 17 '12 at 04:13

0

A few points:

If this is .NET 4, then Lazy is the better way to do this.
Whether it is equivalent from the perspective of memory barriers or not is mostly irrelevant- Exceptions should never be part of the normal code path. I'd assume the behavior is undefined, as this is not an expected use case.

answered Feb 17 '12 at 04:13

Chris Shain

50,833
6
93
125

1) The example is really simplified. This is not an initialization issue for a single workResult. So that rules out Lazy. 2) What is so bad about catching exceptions as part of a normal code path? – Evgeniy Berezovsky Feb 17 '12 at 04:35
Conceptually, it would seem like it should be possible to have a `WaitHandle` derivative which would block threads until it was put into a "permanent green light" state which would no longer use any resources, but would allow any `Wait` operations to immediately and silently proceed; is there any way to do such a thing other than disposing of the resource and catching an `ObjectDisposeException` from the wait? – supercat Feb 28 '14 at 18:45

score 0 · Answer 3 · answered Feb 20 '12 at 05:05

Reflecting a bit more on the actual problem I had to solve, which is a bit more complicated than the simplified example, I decided to use Monitor.Wait and Monitor.PulseAll.

Joe Albahari's Threading in C# proved extremely useful, in this particular case the following section applies: http://www.albahari.com/threading/part4.aspx#_Signaling_with_Wait_and_Pulse

Is it safe to catch ObjectDisposedException on ManualResetEvent.WaitOne()?

3 Answers3