IDisposable implementation for the class which holds threads

Question

Good morning! Let's assume we have the following class:

class MultithreadOperation : IDisposable
{
    private IList<Thread> operationThreads;

    public void StartOperation()
    {
          // Initialize and start threads and put them to operationThreads
    }

    public void StopOperation()
    {
          // Aborts each thread.
    }

    public void Dispose()
    {
          Dispose(true);
          GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
         if (!disposed)
         {
              disposed = true;
              if (disposing)
              {
                    // Release managed resources.
                    #1:
                    StopOperation();
              }
              // Release unmanaged resources.
              #2:
              StopOperation();
         }
    }

    ~MultithreadOperation()
    {
         Dispose(false);
    }
}

Actually, I need to stop all the threads if the instance is disposed. Also, I need to stop all the threads if the instance is garbage collected (otherwise, the threads will be still alive, which is bad for me). Definitely, it is totally legal to invoke StopOperation() method in place #1.

I want to know are there any pitfalls if we invoke StopOperation() in place #2 ? As I understand, the list of threads can be already garbage collected when the ~MultithreadOperation() is executing. Also, I've seen a lot of recommendations to avoid any code which refers to the managed resources in the Finalize implementation, especially the instance fields.

Also, It would be interesting to hear about different approaches for this problem. Thanks!

Answer 1

This is completely inappropriate. A user of your class that doesn't know the details well will assume that calling Dispose() will do something innocent. A bit of cleanup, nothing fancy. She will not expect the program to be left in a completely unpredictable state with threads mutating that state aborted at random locations with no way to recover state.

It is also complete redrum on the finalizer thread, aborting a thread can take many seconds if the thread is not in an alertable state. The CLR will abort the finalizer thread after 2 seconds and terminate the program, giving no diagnostic at all about the true reason the program crashed.

Answer 2

It is not safe to place a call to StopOperation in position #2 because:

• The List of threads or the Thread instances themselves may have already been collected.
• It is not clear if your StopOperation blocks until all threads have acknowledged a stop signal, but if it does then it could hang up the finalizer thread.

Another point is that you should not be aborting threads (via Thread.Abort) in this case. The reason is because the abort injects an out-of-band (asynchronous) exception into the target thread at nondeterministic points. Because these injection points are random (and often nonintuitive) it can corrupt the state (by bailing out early in the middle of a write or other more complex operation) in the application domain or even the entire process.

The best way to handle this is to do the following:

• Have the thread poll or listen for a stop signal. You can see my answer here for tips on how to do this.
• Have the StopOperation signal whatever stopping mechanism you chose. Usually you want to wait until all threads have received the signal and shutdown on their own. You can wait for a thread by calling Thread.Join.
• Call StopOperation in the Dispose method at position #1 only.

Now how do we deal with callers that do not play nice and avoid calling Dispose? This a bit more tricky. I see two general patterns for dealing with this.

• Have the finalizer toggle a stopping flag. That flag must be a value type otherwise it would be subject to garbage collection. Each thread must poll this flag periodically. That means you should not perform any blocking calls that wait indefinitely. Remember, you cannot call Thread.Interrupt to poke a thread since that requires a reference to a Thread instance.
• Maintain a list of active threads in a static collection variable that way you could safely enumerate them in a finalizer. However, I must admit that I am little rusty on the collection rules regarding static references during application domain unloads (ie. the application shutsdown). So I am not certain that it is entirely safe.