Cancelling tasks in the Dispose method

Question

I have a class which spawns various tasks which can run indefinitely. When this object is disposed, I want to stop those tasks from running.

Is this the correct approach:

public class MyClass : IDisposable
{
    // Stuff

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

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            queueCancellationTokenSource.Cancel();
            feedCancellationTokenSource.Cancel();
        }
    }
}

Solution

You're on the right track. However, I would suggest waiting for the task to terminate before returning from the Dispose method, in order to avoid race conditions where the task continues to operate after the object has been disposed. Also dispose the CancellationTokenSource.

Update: If you're on .NET Core 3.0 or later, you should make your class implement IAsyncDisposable and await your task from the DisposeAsyncCore method. I've updated the example below to reflect this.

using System;
using System.Threading;
using System.Threading.Tasks;

public class MyClass : IDisposable, IAsyncDisposable
{
    private readonly CancellationTokenSource feedCancellationTokenSource =
        new CancellationTokenSource();
    private readonly Task feedTask;

    public MyClass()
    {
        feedTask = Task.Factory.StartNew(() =>
        {
            while (!feedCancellationTokenSource.IsCancellationRequested)
            {
                // do finite work
            }
        });
    }

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

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            feedCancellationTokenSource.Cancel();
            feedTask.Wait();

            feedCancellationTokenSource.Dispose();
            feedTask.Dispose();
        }
    }

    public async ValueTask DisposeAsync()
    {
        await DisposeAsyncCore().ConfigureAwait(false);
        Dispose(false);
        GC.SuppressFinalize(this);
    }

    protected virtual async ValueTask DisposeAsyncCore()
    {
        feedCancellationTokenSource.Cancel();
        await feedTask.ConfigureAwait(false);

        feedCancellationTokenSource.Dispose();
        feedTask.Dispose();
    }
}

// Sample usage:
public static class Program
{
    public static async Task Main()
    {
        await using (new MyClass())
        {
            // do something else
        }
        
        Console.WriteLine("Done");
    }
}

Comments

[Dave New]

Great answer - thanks. One question: why do you dispose of the task?

[Douglas]

Just because it's safe to do so, since you're guaranteed that the task would have terminated by then. But you don't really need to.

[phil_rawlings]

This works great if the object is disposed manually, or is used within a using block. However if the user locally creates MyClass within a method, the task continues running even when the method returns. I realize that the GC may execute at some time after the method finishes, however even minutes later the task is still running. I tried adding: ~MyClass() { Dispose(false); } and this does get called when the application shuts down, but at that stage obviously skips the task cancel/dispose (correctly). Will the GC ever dispose or only finalize?

[phil_rawlings]

Regarding my comment above: It would seem that the GC will only call the Finalizer: stackoverflow.com/questions/45036/… However, this means that even if the GC eventually decides to call the Finalizer, that it doesn't stop the task. Would it make sense to add ~MyClass() { Dispose(false); } AND also null the references to feedCancellationTokenSource and feedTask outside the if(disposing) block? This would cause an exception in the task - but at least it would stop! (I realize its not recommended to stop the task in the finalizer)

[Douglas]

@phil_rawlings: The GC will only call the finalizer. You really can't do much during the finalizer calls: Per MSDN: “DO NOT access any finalizable objects in the finalizer code path, because there is significant risk that they will have already been finalized. For example, a finalizable object A that has a reference to another finalizable object B cannot reliably use B in A’s finalizer, or vice versa. Finalizers are called in a random order (short of a weak ordering guarantee for critical finalization).”

[Douglas]

@phil_rawlings: I personally wouldn't bother with stopping the task from the finalizer. Consumers are expected to abide by the contract of calling Dispose on IDisposable instances; failing to do so is effectively a programming error on their part. I would specifically recommend against setting feedCancellationTokenSource to null; an unhandled exception from a task could tear down your whole process under certain configurations.

[Douglas]

@phil_rawlings: If you really need to address this, I assume you could introduce a simple volatile bool sentinel flag which is set from the finalizer and observed from the task (along with or instead of the feedCancellationTokenSource). However, there might be some implications I'm missing. This is probably a good candidate for another SO question.

[phil_rawlings]

Asked new question here: stackoverflow.com/questions/36447462/…

[JHBonarius]

9 years later: is this a good pattern? You're calling Wait in the dispose path, thus blocking cleanup. That might have a significant performance hit.

[Douglas]

@JHBonarius: I've updated the example to use IAsyncDisposable (which has been introduced since then). Whether you want to await the Task at all or just fire-and-forget its disposal depends on the actual use-case.