5

I'm using an application that synchronizes threads using ManualResetEvent. FxCop told me to dispose those objects. I found the following discussion which told me the same:

Do I need to Dispose() or Close() an EventWaitHandle?

But I don't know when to dispose an instance of a ManualResetEvent.

The following simplified code demonstrates the problem:

private void btn_Click(object sender, EventArgs e)
{
    var mre = new ManualResetEvent(false);
    new Thread(() => this.SetEvent(mre)).Start();
    for (int i = 0; i < 10; ++i)
    {
        new Thread(() => this.WaitFor(mre)).Start();
    }
}

private void SetEvent(ManualResetEvent manualResetEvent)
{
    Thread.Sleep(10000);
    manualResetEvent.Set();
}

private void WaitFor(ManualResetEvent manualResetEvent)
{
    manualResetEvent.WaitOne();
}

The problem is that multiple instances of the ManualResetEvent exist and multiple threads are waiting for each instance.

If I memorize the instances in a list I don't know when to dispose it. Disposing it after WaitOne()-call will dispose it multiple times and maybe it will be disposed while other threads are still waiting.

The thread that created the event doesn't have any reference to it. The setter-thread should not dispose it because there are other threads waiting for this MRE. Each waiting thread isn't able to dispose it as mentioned before.

So the question is: When should this ManualResetEvent be disposed?

Community
  • 1
  • 1
Sebastian Schumann
  • 3,204
  • 19
  • 37
  • 1
    The code is nonsensical and there's only *one* instance of the MRE. Disposing it at the end of the method is fine. – Hans Passant Aug 29 '13 at 14:36
  • 1
    No I don't think so. The code that creates the MRE is a button click event handler. You're able to click it multiple times that creates multiple MREs. The code above is simplified. Disposing the MRE at the end of this method won't work because there're 11 threads that are using this MRE after the method has been finished. The setter-thread waits 10 seconds and 10 other threads are waiting for this MRE. – Sebastian Schumann Aug 29 '13 at 15:27

1 Answers1

5

The ManualResetEvent should be disposed when you no longer need it. Your real question is, "how do I know that I no longer need it?"

Typically something is notified when threads are done, and you call Join on the thread. Or the thread sets some event to indicate that it has finished. If you have multiple threads, they can all signal a CountdownEvent. There are several other ways to manage thread notification.

The point is that if you're allocating resources it's up to you to make sure they're disposed correctly. In your code above, you don't have any way to keep track of which threads are executing or which threads are associated with which ManualResetEvent. If you want to make sure that the MRE is disposed of properly, then you have to track it, not only keep track of the MRE but also which threads are using it, which threads have completed their work, and be notified when all of the threads are done so that you can dispose of things.

In your particular situation, if you really have to use an MRE this way, I would probably create a data structure that contains references to the threads and the MRE, and a CountdownEvent that the threads signal when they're done. Something like:

class WorkUnit
{
    public List<Thread> Threads;
    public ManualResetEvent MRE;
    public CountdownEvent CE;
}

Now, when a thread finishes it does this:

workUnit.CE.Signal();

Some other part of your program (probably the main thread) checks the list of work units periodically. For each item in that list it does this:

if (workUnit.CE.WaitOne(0))
{
    foreach (Thread t in workUnit.Threads)
    {
        t.Join();
    }
    // dispose the MRE and the CE
    // and remove the work unit from the list
}

Yes, it's a lot of work. It's probably best if you can structure your program so that you don't have to do this kind of thing.

Jim Mischel
  • 131,090
  • 20
  • 188
  • 351
  • Thanks Jim. The only problem is that I don't know how many threads will wait for the MRE and that's why I can't initialize a CountDownEvent. But you're right that I should consider a design that knows when the MRE is no longer needed. Currently there's no thread waiting for the end of the worker threads because they calculate some values and display the result in controls. After that the task (oh yes I'm using tasks instead of threads but I think the problem is still the same because a task could set the CountDownEvent using ContinueWith) ends and there's no code that waits for it explicitly. – Sebastian Schumann Aug 30 '13 at 05:36
  • I found a solution for my cases. The setter-thread is created and started in only one method that returns the MRE. The caller of this method knows about the MRE and propagates it to multiple threads but it knows about the receivers. I'm able to wrap the MRE into a handler object that manages the MRE-receivers and wait for there ends. After that the MRE could be released. Thanks for your help. – Sebastian Schumann Aug 30 '13 at 06:29