Make `CancellationTokenSource.Token` accessible after dispose

[JamesNK]

CancellationTokenSource guidance recommends disposing of the CTS once it is no longer used. However, after disposal the Token property throws ObjectDisposedException when accessed:

https://github.com/dotnet/runtime/blob/74c608d67d64675ff840c5888368669777c8aa2c/src/libraries/System.Private.CoreLib/src/System/Threading/CancellationTokenSource.cs#L77-L87

I've seen this be a problem a number of times. For example, in ASP.NET Core a request can be aborted with a signal from the client. This would cancel the CTS that drives the HttpContext.RequestAborted cancellation token and it was then disposed. But server code could still be executing and access HttpContext.RequestAborted. Instead of returning the cancelled token, the code would get an ODE: https://github.com/dotnet/aspnetcore/issues/4422

The fix in ASP.NET Core was to not dispose a canceled CTS (which is against guidance in the docs that say a CTS should be disposed once you're no longer using it). We solved the problem after a couple of attempts, but it felt like a pit of failure that is easy to fall into.

Because of this situation, people who use CTS either need to:

• Be careful about not accessing the Token property after dispose (not an easy feat if dispose and API usage happen on different threads)
• Assign the token value to a field or property on the class (see below)
• Not dispose the CTS

Example of making a type safe by setting the cancellation token to a field:

public class Worker : IAsyncDisposable
{
    private readonly CancellationTokenSource _cts = new();
    private readonly CancellationToken _cancellationToken = _cts.Token; // field can be safely accessed
    private Task? _workerTask;

    public Worker()
    {
        _workerTask = Task.Run(() =>
        {
            // If _cts.Token was used here then there would be the chance of throwing ODE.
            while (_cancellationToken.IsCancellationRequested)
            {
                // do a thing
                await Task.Delay(1000, _cancellationToken);
            }
        });
    }

    public async ValueTask DisposeAsync()
    {
        _cts.Cancel();
        _cts.Dispose();
        try { await _workerTask; } catch (OperationCanceledException) { }
    }
}

CTS feels like it would be simpler to use and less of a pit of failure if CancellationTokenSource.Token could still be accessed after dispose.

Proposed change: https://github.com/dotnet/runtime/commit/31d8d32096228931749484c4a36a559bba8a4cb7

[dotnet-policy-service[bot]]

Tagging subscribers to this area: @mangod9 See info in area-owners.md if you want to be subscribed.

[vladd]

@JamesNK For your example, I would argue that we should not dispose the CTS after cancellation (because the clients may still be using it). The point where we can be sure that the CTS is not used is after await workerTask, not before.

Beside that, maybe workerTask should not use the CTS, only CT, so the CT should be passed down to Worker (so presumably we don't need to store the CT in a field at all).

Of course, there might be a more elaborate example where this simple approach doesn't help.

[JamesNK]

@vladd Yes, that would fix this case. Every example I give there will always be a way to change the code to make it work. CTS isn't broken. But it could be made easier to use with one simple change.

How about this Blazor dialog component that has a CancellationTokenSource:

https://github.com/dotnet/aspire/blob/a41bd1baf4367f6b9394575d75dd995340b4795b/src/Aspire.Dashboard/Components/Dialogs/ExemplarsDialog.razor.cs

OnViewDetailsAsync is triggered by someone clicking on a button in the UI. It passes a cancellation token to a method that does some async work.

Dispose is triggered when the dialog is clicked. It cancels the CTS.

Now, I don't think these two events can race because Blazor has the concept of the UI thread, but perhaps there is an exceptional situation where UI events come in out of order and they could, and it just hasn't happened yet. If Dispose is called and it's then followed by OnViewDetailsAsync then an ObjectDisposedException is thrown when accessing the CancellationTokenSource.Token.

[vladd]

@JamesNK I usually avoid patterns like

cts.Cancel();
cts.Dispose();

by the very same reason: Cancel is a signal to whoever is using the token, so I tend to maintain an inner list of those and await all of them to finish before disposing. (But I understand that this way is tedious and maybe an overkill.)

By the way, CancellationToken after disposal of CTS allows using everything except WaitHandle (which is a bit inconsistent, too).