CODEWITHSUNDEEP
c#multithreadingidisposable
Nick
Nick

Reputation: 10499

Should I implement IDisposable for a class containing a Thread

I have a class that uses the Thread class:

class A
{
    public Thread thread
    { get; set; }
}

Should I implement IDisposable and set Thread property to null?

class A : IDisposable
{
    public Thread Thread
    { get; set; }

    protected bool Disposed
    { get; set; }


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

    protected virtual void Dispose(bool disposing)
    {
        if (!this.Disposed)
        {
            if (disposing)
            {
                if (Thread != null)
                    Thread = null;
            }

            Disposed = true;
        }
    }
}

Or not?
Why?

Upvotes: 6

Views: 1731

Answers (2)

Mike
Mike

Reputation: 974

It depends what your thread is doing. If your thread is performing a long running task that may run indefinitely, then I would consider that thread as a resource (which will not be garbage collected). For example consider if the thread is designed to poll some state indefinitely, or consume items from a queue (like a thread-pool thread consumes tasks or a TCP server consumes new connections) etc. In this case, I would say the natural effect of disposing your class would be to free up this thread resource. Setting it to null is not really useful in this case. Rather Dispose should probably involve flagging a synchronization event (or maybe a CancellationToken) to notify the thread that it should finish up its infinite task, and then the disposing thread should wait some time for the thread to finish (join). As always with joins, be careful of a deadlock scenario and consider some alternative action if the thread refuses to terminate. For obvious reasons I would not do this join in the finalizer.

As an example of what I'm meaning, consider the scenario where your class A is actually class MyTcpListener, designed to listen and wait for new TCP connections on a given port indefinitely. Then consider what you expect following (somewhat unlikely) code to do:

using (MyTcpListener listener = new MyTcpListener(port:1234))
{
    // Do something here
}

// Create another one. This would fail if the previous Dispose
// did not unbind from the port.
using (MyTcpListener listener = new MyTcpListener(port:1234))
{
    // Do something else here
}

Assuming I know the constructor of MyTcpListener creates a listener thread, I would expect that after the Dispose call has returned that the MyTcpListener would no longer be bound to the TCP port - i.e. that the TCP listener thread would have fully terminated. It goes without saying that if you didn't provide some mechanism to stop the listener that there would be a resource leak. The stopping mechanism could be a call to some method "Stop", but I personally think the "Dispose" pattern fits this scenario more cleanly since forgetting to stop something does not generally imply a resource leak.

Your code may call for different assumptions, so I would suggest judging it on the scenario. If your thread is short-running, e.g. it has some known finite task to complete and then it will terminate on its own, then I would say that disposing is less critical or perhaps useless.

Upvotes: 2

LightStriker
LightStriker

Reputation: 21004

You implement IDisposable only when your class is handling an unmanaged object, resources or other IDisposable objects. A Thread is not an unmanaged object and will get garbage collected when nothing is referencing it or when the process handling it is terminated. Since Thread is not implementing IDisposable, your class referencing it does not need to implement it either.

Optionally, for IDisposable within the scope of a method, they can be wrapped in a using statement and the Dispose() method is automatically called when the scope is exited.

Upvotes: 8

Related Questions