Should a class with a Thread member implement IDisposable?

Question

Let's say I have this class Logger that is logging strings in a low-priority worker thread, which isn't a background thread. Strings are queued in Logger.WriteLine and munched in Logger.Worker. No queued strings are allowed to be lost. Roughly like this (implementation, locking, synchronizing, etc. omitted for clarity):

public class Logger
{
    private Thread workerThread;
    private Queue<String> logTexts;
    private AutoResetEvent logEvent;
    private AutoResetEvent stopEvent;

    // Locks the queue, adds the text to it and sets the log event.
    public void WriteLine(String text);

    // Sets the stop event without waiting for the thread to stop.
    public void AsyncStop();

    // Waits for any of the log event or stop event to be signalled.
    // If log event is set, it locks the queue, grabs the texts and logs them.
    // If stop event is set, it exits the function and the thread.
    private void Worker();
}

Since the worker thread is a foreground thread, I have to be able to deterministically stop it if the process should be able to finish.

Question: Is the general recommendation in this scenario to let Logger implement IDisposable and stop the worker thread in Dispose()? Something like this:

public class Logger : IDisposable
{
    ...

    public void Dispose()
    {
        AsyncStop();
        this.workerThread.Join();
    }
}

Or are there better ways of handling it?

Answer 1

That would certainly work - a Thread qualifies as a resource, etc. The main benefit of IDisposable comes from the using statement, so it really depends on whether the typical use for the owner of the object is to use the object for a duration of time in a single method - i.e.

void Foo() {
    ...
    using(var obj = YourObject()) {
        ... some loop?
    }
    ...
}

If that makes sense (perhaps a work pump), then fine; IDisposable would be helpful for the case when an exception is thrown. If that isn't the typical use then other than highlighting that it needs some kind of cleanup, it isn't quite so helpful.

Answer 2

That's usually the best, as long as you have a deterministic way to dispose the logger (using block on the main part of the app, try/finally, shutdown handler, etc).