Search code examples
c#multithreadingwaithandle

WaitHandle is closed when it shouldn't have


This code works, for most of time, so I'm thinking of some race condition. Result class is immutable, but I don't think the issue is with that class.

public Result GetResult()
{
    using (var waitHandle = new ManualResetEvent(false))
    {
        Result result = null;

        var completedHandler = new WorkCompletedEventHandler((o, e) =>
        {
            result = e.Result;

            // somehow waitHandle is closed, thus exception occurs here
            waitHandle.Set();
        });
        try
        {
            this.worker.Completed += completedHandler;

            // starts working on separate thread
            // when done, this.worker invokes its Completed event
            this.worker.RunWork();

            waitHandle.WaitOne();

            return new WorkResult(result);
        }
        finally
        {
            this.worker.Completed -= completedHandler;
        }
    }
}

Edit: Apologies, I've missed a call to this.worker.RunWork() right before calling GetResult() method. This apparently resulted (sometimes) in doing same job twice, though I'm not sure why waitHandle got closed before waitHandle.Set(), despite having Completed event firing twice. This hasn't compromised the IO work at all (results were correct; after I've changed the code to manually close the waitHandle).

Therefore, Iridium's answer should be closest answer (if not the right one), even though the question wasn't complete.


Solution

  • There doesn't seem anything particularly problematic in the code you've given, which would suggest that there is perhaps something in the code you've not shown that's causing the problem. I'm assuming that the worker you're using is part of your codebase (rather than part of the .NET BCL like BackgroundWorker?) It may be worth posting the code for that, in case there is an issue there that's causing the problem.

    If for example, the same worker is used repeatedly from multiple threads (or has a bug in which Completed can be raised more than once for the same piece of work), then if the worker uses the "usual" means for invoking an event handler, i.e.:

    var handler = Completed;
    if (handler != null)
    {
        handler(...);
    }
    

    You could have an instance where var handler = Completed; is executed before the finally clause (and so before the completedHandler has been detached from the Completed event), but handler(...) is called after the using(...) block is exited (and so after the ManualResetEvent has been disposed). Your event handler will then be executed after waitHandle is disposed, and the exception you are seeing will be thrown.