Implement graceful cancellation

[Tyrrrz]

Closes #47

Ignore the massive file moves, the important bits are in Command.Execution.cs and ProcessEx.cs.

Useful links:

• https://gist.github.com/ycherkes/b216e78ae7ee4418504320f861337ed4
• https://github.com/madelson/MedallionShell/blob/3fddb89860842ffc836a0d0f69b161f67e4aa7c4/MedallionShell/Signals/ProcessSignaler.cs

[codecov[bot]]

Codecov Report

Merging #179 (8889be2) into master (e83b52d) will decrease coverage by 0.55%. The diff coverage is 87.92%.

@@            Coverage Diff             @@
##           master     #179      +/-   ##
==========================================
- Coverage   93.22%   92.66%   -0.56%     
==========================================
  Files          39       43       +4     
  Lines         915     1023     +108     
  Branches       86       91       +5     
==========================================
+ Hits          853      948      +95     
- Misses         39       49      +10     
- Partials       23       26       +3     

Impacted Files	Coverage Δ
CliWrap/Buffered/BufferedCommandResult.cs	100.00% <ø> (ø)
CliWrap/Builders/CredentialsBuilder.cs	100.00% <ø> (ø)
CliWrap/Command.PipeOperators.cs	100.00% <ø> (ø)
CliWrap/Command.cs	100.00% <ø> (+1.67%)	:arrow_up:
CliWrap/CommandResult.cs	100.00% <ø> (ø)
CliWrap/CommandResultValidation.cs	100.00% <ø> (ø)
CliWrap/Credentials.cs	100.00% <ø> (ø)
CliWrap/EventStream/CommandEvent.cs	100.00% <ø> (ø)
CliWrap/PipeSource.cs	100.00% <ø> (ø)
CliWrap/PipeTarget.cs	96.55% <ø> (ø)
... and 14 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[Tyrrrz]

Current proposed usage (taken from tests on this branch):

[Fact(Timeout = 15000)]
public async Task Command_execution_can_be_canceled_gracefully()
{
    // Arrange
    var stdOutLines = new List<string>();

    using var cts = new CommandCancellationTokenSource();
    cts.CancelGracefullyAfter(TimeSpan.FromSeconds(0.5));

    var cmd = Cli.Wrap("dotnet")
        .WithArguments(a => a
            .Add(Dummy.Program.FilePath)
            .Add("sleep")
            .Add("--duration").Add("00:00:20")
        ) | stdOutLines.Add;

    // Act
    var task = cmd.ExecuteAsync(cts.Token);

    // Assert
    var ex = await Assert.ThrowsAnyAsync<OperationCanceledException>(() => task);
    ProcessEx.IsRunning(task.ProcessId).Should().BeFalse();
    ex.CancellationToken.Should().Be(cts.Token);

    stdOutLines.Should().ContainEquivalentOf("Operation cancelled gracefully.");
    stdOutLines.Should().NotContainEquivalentOf("Done.");
}

[ycherkes]

Looks awesome to me. I just figured out that my code for windows isn't working in unit tests. So the external executable is required.

[Tyrrrz]

@ycherkes

I just figured out that my code for windows isn't working in unit tests. So the external executable is required.

In your own unit tests or here? Because I haven't implemented it for windows here yet.

As far as I could tell, an external executable is only needed if the process is running in a different console. In practice, that should mostly only happen if the parent process is a GUI application and does not have any console attached, in which case briefly attaching the target console should be painless, I hope. 🤞🏻

In any case, my design for this feature is that it can silently fail. Even in a situation where SIGINT is passed successfully, the underlying process may still choose to completely disregard it. For that reason, graceful cancellation shouldn't be seen as a guarantee, and it should be "okay" to just do nothing in situations where it's hard to implement (such as this case on Windows with separate console windows).

[ycherkes]

In your own unit tests or here? Because I haven't implemented it for windows here yet.

I put my implementation to your code and got a Timeout exception for all graceful cancellation tests. Because neither JetBrains, nor MS test adapters don't have a Console Window:

    [DllImport("kernel32.dll")]
    static extern IntPtr GetConsoleWindow();

    [Fact]
    public void CheckConsole()
    {
        var consolePtr = GetConsoleWindow();
        // It fails
        consolePtr.Should().NotBe(IntPtr.Zero);
    }

So the execution flow reaches this code and CTRL_C doesn't reach the target process.

[Tyrrrz]

In your own unit tests or here? Because I haven't implemented it for windows here yet.

I put my implementation to your code and got a Timeout exception for all graceful cancellation tests. Because neither JetBrains, nor MS test adapters don't have a Console Window:

 [DllImport("kernel32.dll")]
 static extern IntPtr GetConsoleWindow();

 [Fact]
 public void CheckConsole()
 {
     var consolePtr = GetConsoleWindow();
     // It fails
     consolePtr.Should().NotBe(IntPtr.Zero);
 }

So the execution flow reaches this code and CTRL_C doesn't reach the target process.

Ah, makes sense. Would an external process even help here? Since there is no console to attach to.

[ycherkes]

Yup, I've verified it. Created process still has its own Console, so MedallionShell.ProcessSignaler works as expected. Also, I tried a similar but c++ based unmanaged exe. Works too.