Runtime: Feature request: CancellationToken as a cross-cutting concern

[dmytro-gokun]

So, i was looking at this issue here: https://github.com/dotnet/corefx/issues/32615 and then at my code where i have to pass CancellationToken all the way down through the call stack.. And then it struck me: why do we have to do all that crap manually in 2019? Is not this CancellationToken a clear example of a cross-cutting concern and best served by the infrastructure? Should not we have something like CancellationScope (similar to TransactionScope) and everything that needs access to CancellationToken can have it somehow (think of Transaction.Current).

I'm sure someone already have thought about this and the idea was rejected because it has some critical flaws. Or was it? :)

Any comment is welcome. Otherwise, I will not be able to sleep tonight, thinking about all those CancellationTokens passed around while there was a way to avoid that :).

[davidfowl]

I think you mean support an ambient CancellationToken (something like CancellationToken.Current). I'm torn because of the cost of something like this and the unintended side effects of cancelling everything in the call chain (it would be very sticky) and how to opt out of the behavior.

[dmytro-gokun]

@davidfowl Thank you for the response.

You are right. I envision it like this (if fully integrated with .NET):

class CancellationScope : IDisposable
{
    public CancellationScope(CancellationToken cancellationToken);
}

class CancellationToken
{
    // ...

    public static CancellationToken Current {get;}

    // ...
}

Current returns CancellationToken.None when outside of a scope.
Then, each method which accepts an (optional) CancellationToken should check if that was passed or not and if it was not (i.e. cancellationToken == CancellationToken.None == default(CancellationToken)) - use CancellationToken.Current. But only in case when it needs to do something with that token (e.g. check if cancellation was requested). If only use of the token is to pass it down the call chain, that method does not need to do anything additional to support scopes. That's the whole idea behind this scope thing. Only async methods at absolute bottom of the call chain need to be fixed so that they are aware of the scope.

You are right that it may produce some unintended consequences for existing code. But only when that code is wrapped with a CancellationScope, which is a code change one should only do after some serious thinking and apply appropriate testing afterwards. So, it's kind of an opt-in feature.

To opt-out when you know you might be in a scope.. is a bit harder. Passing CancellationToken.None or calling a version which does not accept CancellationToken would not help obviously, as that would be translated to CancellationToken.Current. I think for this rare case (cannot think why may legitimatelly want that. can you?), it might make sense to have a special "marker" instance (just like CancellationToken.None). For example:

class CancellationToken
{
    // ...

    // Need to think of a better name.
    public static CancellationToken AbsoluteNone => new CancellationTokenSource().Token;

    // ...
}

So, when you pass CancellationToken.AbsoluteNone, you opt-out of the current cancellation scope altogether. But, if you do not want to encourage such opt-outs, you just do not provide AbsoluteNone in .NET itself.

Does this make sense?

[davidfowl]

You are right that it may produce some unintended consequences for existing code. But only when that code is wrapped with a CancellationScope, which is a code change one should only do after some serious thinking and apply appropriate testing afterwards. So, it's kind of an opt-in feature.

That doesn't help, it's just as viral and it's not opt in. Every component has to use the current token or the entire system doesn't work.

[dmytro-gokun]

That doesn't help, it's just as viral and it's not opt in.

Well, i would argue it is opt-in. Even the code that is aware of cancellation scope will continue working as before until it is explicitly wrapped in one. The act of wrapping is the act of opting in by the person doing the wrapping. That person needs to understand clearly what they're doing.

Every component has to use the current token or the entire system doesn't work.

Well, not every component. Only those which do something with CTs apart of passing them down the call stack. But the leaf components need to be aware. You are right. And that's the exact reason why i asked for it as a .NET feature. Otherwise, it could have been implemented in a separate library.

I would argue that such library may still be useful for an application, so it does not have to pass CTs around. It's just that such an app will have to supply .NET/3rd party libs with CancellationToken.Current explicitly.

But only if made a part of .NET, the idea gains its full possible power. And you are right, it is not a trivial one-line change in .NET. Many components need to be reviewed/changed. Each component should explicitly document if it supports ambient CTs or not. That is a massive effort, no doubt. But the gain is also massive, IMO. In essence, with this we could make tons of code cancel-friendly with minimal explicit effort from the programmer. That is something to consider.

[GSPP]

These kinds of "scope based contexts" can be very convenient but also very dangerous. Essentially, all code now depends on the value of a thread-local mutable global variable.

This causes non-local effects where surrounding code can modify the behavior of inner code (e.g. library or framework) in ways unforeseen by the developers.

There are existing examples of this. For example, the ExecutionContext. I wonder if the .NET team considers the existence of ExecutionContext a design mistake in the framework.

TransactionScope can be very convenient but it also can lead to nasty bugs that don't show up during testing.

The automatic propagation of SynchronizationContext and TaskScheduler.Current is very convenient as well, but again it leads to nasty bugs and performance costs. In particular, it is the reason we need to apply ConfigureAwait(false) to every single await in library code.

[dmytro-gokun]

@GSPP you are right, it can be dangerous (well, not really dangerous, but really hard to debug), if not used properly. On the other hand, we still have Thread.CurrentCulture & friends, which could be dangerous as well if used improperly. But nobody worries much about that. Imagine the hell that would take place if there were no Thread.CurrentCulture. Then every method had to have an additional "culture" parameter which it had to pass to each nested function call. That would be hell. But that is what we have today with CancellationTokens :). What i'm trying to say, the mere fact that it is "a thread-local mutable global variable" (actually, async-local) should not be the deciding factor. Only usefulness (which pulls us closer to adoption) + potential impact on the existing code (which pushes us away).

[Clockwork-Muse]

But nobody worries much about that. Imagine the hell that would take place if there were no Thread.CurrentCulture. Then every method had to have an additional "culture" parameter which it had to pass to each nested function call. That would be hell.

Personally, I'd actually _prefer_ that. I mean, it would actually make dealing with culture issues cleaner in some server contexts - anything that renders a web page is dealing with _at least two_ cultures (the server's, which should be some invariant, and the visitor's, which should be configurable). I'm always rather nervous around implicit global config state (especially something mutable).

One key difference from TransactionScope is that, in most cases, transactions are only being created by the end consumer (that is, they're written by application writers, not intermediate library writers).
CancellationTokens, however, are used throughout library code, and may not even be intended to be exposed to consumers.

[dmytro-gokun]

@Clockwork-Muse

Personally, I'd actually prefer that. I mean, it would actually make dealing with culture issues cleaner in some server contexts - anything that renders a web page is dealing with at least two cultures (the server's, which should be some invariant, and the visitor's, which should be configurable). I'm always rather nervous around implicit global config state (especially something mutable).

I'm with you on this. And yet, even in the server context, you are somehow relying on request "globals", e.g. user identity. Sure, we can (try) make everything pure & immutable. But that also has it's price (e.g. the fact that now you need to carry your "context" around as a parameter which have to (manually) propagate to each nested call. The question is how crazy you'd like to go down this rabbit hole :)

Also, consider this. When you started doing async/await & passing CTs around... well, your code is far from pure & immutable by that moment :). I mean, it literally has some side-effects (if not, ask yourself why you needed those async/awaits in the first place).

CancellationTokens, however, are used throughout library code, and may not even be intended to be exposed to consumers.

Okay, the thing is that CancellationScope as proposed (https://github.com/dotnet/corefx/issues/38546#issuecomment-502352218) will not affect those internally created tokens whatsoever. The only case when it may have some unintended consequences is when a lib call some async methods passing CT.None to them. I would argue though, it's a bad non-cancel-friendly lib then and would recommend against using it).

In any case, i'm getting the vibe here. I think I'll go ahead and create a lib for use in my apps (I was going to do that anyway) and will just make sure to pass the analog of CancellationToken.Current to any external methods i'm calling from my code, be it .NET methods or external libs' methods. Sure, that will require some additional typing, but at least no 3rd party is harmed this way :).

[stephentoub]

Thanks for the discussion. Closing. A separate library to do what you want sounds like a great place to experiment and address your needs.