CancellationToken registered callback is called after the token is no longer used

Question

Skeleton code:

 class Thing
 {
    public async Task<int> RunAsync(CancellationToken cancellationToken)
    {
        cancellationToken.Register(() =>
        {
            //doesn't accept a cancellation, this kills it
            this.tcpClient.Dispose();
        });
        ...
    }
    public void Dispose()
    {
       tcpClient.Dispose();
       tcpClient = null;
    }
 }

...

var cancellation = new CancellationTokenSource();
var x = new Thing();
await x.RunAsync(cancellation.Token);
x.Dispose();
//yes I know this is odd in this example, it's just to demo
cancellation.Cancel();

I discovered that if RunAsync has already completed, and the object disposed, then cancelling the associated CancellationTokenSource still calls the registered lambda. Unsurprisingly this causes unexpected behaviour - I found the issue since tcpClient was set to null by Dispose() for example.

Since the method has already exited, why does cancellationToken (a value type) still exist at all? It's a good reminder that in C#, objects may exist after you stop using them but I would have thought method-scoped entities would have disappeared.

What should be different here? Is Register a dangerous method to use or am I just mis-using it?

A note, in practice there would be lotd of `Thing` objects. Some active, others discarded and disposed - the idea being I can cancel them all, unlike this very simple and a bit odd example. — Mr. Boy, Jul 02 '20 at 21:34
The cancellation token doesn't care about where it's used. When the task is cancelled, register is called. https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtoken.register?view=netcore-3.1 — Josh, Jul 02 '20 at 21:35
The register method had captured the tcpClient instance via closure, see this answer https://stackoverflow.com/a/9591622/11582808 — Josh, Jul 02 '20 at 21:37
Have you tried to explictly Dispose the CancellationTokenRegistration? Or in dotnet Core call `Unregister()` at the end of the RunAsync Method? — Fildor, Jul 02 '20 at 21:49
`CancelationToken` is a struct, but `CancelationToken.Register` [internally](https://source.dot.net/#System.Private.CoreLib/CancellationToken.cs,270) binds to classes. — Guru Stron, Jul 02 '20 at 21:54
@Fildor I had totally missed that it returned `CancellationTokenRegistration` and I'm in .net so hadn't seen there was any way to unregister - thanks I think that's the answer I was after! — Mr. Boy, Jul 03 '20 at 08:47

score 1 · Accepted Answer · answered Jul 03 '20 at 10:55

CancellationToken.Register returns a disposable CancellationTokenRegistration. The registration is ended by disposing it.

So, wrap your registration in a using block, then the callback will no longer be invoked by a Cancel, as soon as the block ended.

using (cancellationToken.Register(() =>
        {
            //doesn't accept a cancellation, this kills it
            this.tcpClient.Dispose();
        }))
{
// your code...
}

I would still advise to make your code robust in case this.tcpClient is null. I'm not sure, but if we put a long sleep inside the registered callback and cancel from a separate thread, it might still be possible to start executing the callback and then finishing the RunAsync, disposing Thing and then reaching the point in callback, where this.tcpClient is accessed.

CancellationToken registered callback is called after the token is no longer used

1 Answers1