Adding in cancellation support for InvokeAsync

[Keboo]

Fixes #1469

There is a slight behavior change in this. Previously, if you did not request a CancellationToken in a command handler, it would not register up on the Console.CancelKeyPress and AppDomain.CurrentDomain.ProcessExit events.
With these changes it now always registers for those events if you call CommandLineBuilder.CancelOnProcessTermination (which is part of the defaults; so likely often).

Key areas to review are CommandLineBuilderExtensions and InvocationContext changes.

[jonsequitur]

Suggested change

	throw new InvalidOperationException($"Cannot add additional linked cancellation tokens once {nameof(InvocationContext)}.{nameof(CancellationToken)} has been invoked");
	throw new InvalidOperationException($"Cannot add additional linked cancellation tokens once {nameof(InvocationContext)}.{nameof(CancellationToken)} has been cancelled");

[Keboo]

I don't think this is correct. The failure occurs if the Lazy has already been evaluated, meaning something requested the CancellationToken from the context. It may or may not be cancelled at this point.

[jonsequitur]

This initially caught my eye because I wasn't sure what it means to "invoke" a CancellationToken. If I saw this exception message, I'm not sure I'd understand what I should do differently. What can we do to make this error clearly actionable for developers?

[tmds]

Previously, if you did not request a CancellationToken in a command handler, it would not register up on the Console.CancelKeyPress and AppDomain.CurrentDomain.ProcessExit events. With these changes it now always registers for those events if you call CommandLineBuilder.CancelOnProcessTermination (which is part of the defaults; so likely often).

I haven't looked at the code yet, but it's important to know this:

Requesting the CancellationToken means you 'promise' to use that token for terminating the application.
If it is unconditionally enabled then applications that are not actually using the token will no longer terminate on SIGTERM or Ctrl+C.

[tmds]

Requesting the CancellationToken means you 'promise' to use that token for terminating the application.
If it is unconditionally enabled then applications that are not actually using the token will no longer terminate on SIGTERM or Ctrl+C.

@Keboo is this handled somehow?

[Keboo]

Hi @tmds sorry for the long delay. I just got back to this and addressed the merge issues. After digging a bit more I did refactor the code so that it will not register for Console.CancelKeyPress or AppDomain.CurrentDomain.ProcessExit unless the command's handler requests a CancellationToken. I believe this addresses your concern above. Thank you very much for your feedback.

[sakno]

GetCancellationToken forces creation of final cancellation token. As a result, any subsequent calls to AddLinkedCancellationToken will not work. For instance, I'm using GetCancellationToken to inject token to the binding context through middleware to make the token available for all asynchronous command handlers via custom binder. If I put the middleware too early in the chain, nobody else will be able to add the linked token in the chain.

[sakno]

I would like to propose a different approach to maintain linked token sources within InvocationContext:

public sealed class InvocationContext : IDisposable
{
    private readonly CancellationToken _token; // cached token to avoid ObjectDisposedException due to concurrency between Dispose() and Cancel()
    private readonly LinkedList<CancellationTokenRegistration> _registrations = new();
    private volatile CancellationTokenSource? _source;
    private volatile int _canceled;

    public InvocationContext(...)
    {
        _source = new();
        _token = _source.Token;
    }

    public CancellationToken GetCancellationToken() => _token;

    // internal visible for easy access from program termination callbacks
    internal void Cancel()
    {
       using (var source = Interlocked.Exchange(ref _source, null))
       {
           source?.Cancel();
       }
    }

    public void LinkToken(CancellationToken token)
    {
        _registrations.AddLast(token.Register(this.Cancel));
    }

    // explicit to prevent invocation from middleware or any other parts of user code
    void IDisposable.Dispose()
    {
        Interlocked.Exchange(ref _source, null)?.Dispose();
        foreach (var registration in _registrations)
           registration.Dispose();
    }
}

[Keboo]

@jonsequitur not sure if this is important to you or not, but explicit interface members are not shown.