views:

213

answers:

2

We were surprised to learn today that threads waiting on a ManualResetEvent continue waiting on the event even when it's closed. We would have expected that calling Close() would implicitly signal the waiting threads.

We tracked this down as a reason some of our windows services were not shutting down as fast as we'd like. We're changing all of our Dispose implementations that close ManualResetEvent references to call Set first.

Can anyone explain why Close doesn't implicitly call Set? When would you want a waiting thread to continue waiting?

Here's our test code to demonstrate our findings:

    private static readonly Stopwatch _timer = Stopwatch.StartNew();

    public static void Test()
    {

        var sync = new ManualResetEvent(false);

        ThreadPool.QueueUserWorkItem(state =>
                                         {
                                             Log("ThreadPool enter, waiting 250ms...");
                                             sync.WaitOne(250);
                                             Log("ThreadPool exit");
                                         });

        Log("Main sleeping 100");
        Thread.Sleep(100);
        Log("Main about to close");
        // sync.Set();      // Is Set called implicitly?  No...
        sync.Close();

        Log("Main waiting for exit 500ms");
        Thread.Sleep(500);
    }

    private static void Log(string text)
    {
        Console.WriteLine("{0:0} {1}", _timer.ElapsedMilliseconds, text);  
    }

When we run this code with the Set call commented, we get this..

0 Main sleeping 100
0 ThreadPool enter, waiting 250ms...
103 Main about to close
103 Main waiting for exit 500ms
259 ThreadPool exit

When we explicitly call Set we get this..

0 Main sleeping 100
0 ThreadPool enter, waiting 250ms...
98 Main about to close
98 ThreadPool exit
98 Main waiting for exit 500ms
+2  A: 

Close is a means of disposing of the object (Close and Dispose on this class yield identical behavior). It does not affect the state of the handle. To assume that, in all cases, the user would want a thread waiting on a handle that I closed to continue does not seem reasonable. In fact, the fact that the handle is in use should indicate that you shouldn't be calling Close in the first place.

It's not a matter of "why shouldn't Set be called implicitly?", it's a conceptual problem: If you're calling Close, you should no longer care about the object. Use Set and Reset to control the flow of execution among threads; don't call Close (or Dispose) on any object, WaitHandles included, until they are no longer in use.

Adam Robinson
To add to that, it would cause hard to trace race conditions, even in the example of the OP, you'd get in trouble if the sync.Close(); happened to be called before the thread pool got around to do sync.WaitOne();
nos
The problem I have with this whole thing is that a WaitHandle is designed to support synchronization between threads, so I feel like Set on Close and event Wait after Close should be handled implicitly. No worries though, we created a custom extended class that adds the functionality we want.
Sam
@Sam: I'm glad you found a solution, but, again, the problem is that your philosophy about the object's lifetime does not match that of the designers: the mere fact that you *care* about what state the handle is in means that you, by their design, are not yet ready to call `Close`.
Adam Robinson
+1  A: 

These synchronization events are based on Win32 wait handles, and Close() method only releases them (like Dispose()) without signaling, and waiting threads keep waiting.

Al Bundy