From c72bce11a2e8184bfb11f052232db1c1ff9247d7 Mon Sep 17 00:00:00 2001 From: Damian Milosz Date: Fri, 26 Apr 2024 09:28:39 +0200 Subject: [PATCH 1/3] Implement pipingTask timeout --- .../Commands/RunProcessCommand.cs | 37 +++++++++++++++++++ CliWrap.Tests/CancellationSpecs.cs | 34 +++++++++++++++++ CliWrap/Command.Execution.cs | 33 ++++++++++++++--- CliWrap/Exceptions/PipesCancelledException.cs | 31 ++++++++++++++++ 4 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 CliWrap.Tests.Dummy/Commands/RunProcessCommand.cs create mode 100644 CliWrap/Exceptions/PipesCancelledException.cs diff --git a/CliWrap.Tests.Dummy/Commands/RunProcessCommand.cs b/CliWrap.Tests.Dummy/Commands/RunProcessCommand.cs new file mode 100644 index 00000000..cab9348e --- /dev/null +++ b/CliWrap.Tests.Dummy/Commands/RunProcessCommand.cs @@ -0,0 +1,37 @@ +using System; +using System.Diagnostics; +using System.Threading.Tasks; +using CliFx; +using CliFx.Attributes; +using CliFx.Infrastructure; + +namespace CliWrap.Tests.Dummy.Commands; + +[Command("run process")] +public class RunProcessCommand : ICommand +{ + [CommandOption("path")] + public string FilePath { get; init; } = string.Empty; + + [CommandOption("arguments")] + public string Arguments { get; init; } = string.Empty; + + public ValueTask ExecuteAsync(IConsole console) + { + var startInfo = new ProcessStartInfo + { + FileName = FilePath, + Arguments = Arguments, + UseShellExecute = false, + CreateNoWindow = true + }; + + var process = new Process(); + process.StartInfo = startInfo; + process.Start(); + + console.Output.WriteLine(process.Id); + + return default; + } +} diff --git a/CliWrap.Tests/CancellationSpecs.cs b/CliWrap.Tests/CancellationSpecs.cs index 6f4ed0c0..835553d0 100644 --- a/CliWrap.Tests/CancellationSpecs.cs +++ b/CliWrap.Tests/CancellationSpecs.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using CliWrap.Buffered; using CliWrap.EventStream; +using CliWrap.Exceptions; using CliWrap.Tests.Utils; using FluentAssertions; using Xunit; @@ -111,6 +112,39 @@ public async Task I_can_execute_a_command_with_buffering_and_cancel_it_immediate ex.CancellationToken.Should().Be(cts.Token); } + [Fact(Timeout = 10000)] + public async Task I_can_execute_a_command_and_cancel_piping_when_command_is_finished_but_child_process_remains_running() + { + // Arrange + var pipesCts = new CancellationTokenSource(); + var cmd = Cli.Wrap(Dummy.Program.FilePath) + .WithArguments( + [ + "run", + "process", + "--path", + Dummy.Program.FilePath, + "--arguments", + "sleep 00:00:15" + ] + ) + .WithStandardOutputPipe(PipeTarget.ToDelegate(_ => { })) + .WithStandardErrorPipe(PipeTarget.ToDelegate(_ => { })); + + // Act + pipesCts.CancelAfter(TimeSpan.FromSeconds(1)); + var executeAsync = async () => + await cmd.ExecuteAsync(CancellationToken.None, CancellationToken.None, pipesCts.Token); + + // Assert + var ex = await Assert.ThrowsAnyAsync( + async () => await executeAsync() + ); + + ex.CancellationToken.Should().Be(pipesCts.Token); + ex.ExitCode.Should().Be(0); + } + [Fact(Timeout = 15000)] public async Task I_can_execute_a_command_with_buffering_and_cancel_it_after_a_delay() { diff --git a/CliWrap/Command.Execution.cs b/CliWrap/Command.Execution.cs index d5d74da5..b9ed45f6 100644 --- a/CliWrap/Command.Execution.cs +++ b/CliWrap/Command.Execution.cs @@ -189,7 +189,8 @@ await StandardErrorPipe private async Task ExecuteAsync( ProcessEx process, CancellationToken forcefulCancellationToken = default, - CancellationToken gracefulCancellationToken = default + CancellationToken gracefulCancellationToken = default, + CancellationToken pipesCancellationToken = default ) { using var _ = process; @@ -219,11 +220,15 @@ private async Task ExecuteAsync( .Register(process.Interrupt) .ToAsyncDisposable(); + using var pipeStdOutErrCts = CancellationTokenSource.CreateLinkedTokenSource( + forcefulCancellationToken, + pipesCancellationToken + ); // Start piping streams in the background var pipingTask = Task.WhenAll( PipeStandardInputAsync(process, stdInCts.Token), - PipeStandardOutputAsync(process, forcefulCancellationToken), - PipeStandardErrorAsync(process, forcefulCancellationToken) + PipeStandardOutputAsync(process, pipeStdOutErrCts.Token), + PipeStandardErrorAsync(process, pipeStdOutErrCts.Token) ); try @@ -249,6 +254,7 @@ private async Task ExecuteAsync( || ex.CancellationToken == gracefulCancellationToken || ex.CancellationToken == waitTimeoutCts.Token || ex.CancellationToken == stdInCts.Token + || ex.CancellationToken == pipeStdOutErrCts.Token ) { } // Throw if forceful cancellation was requested. @@ -265,6 +271,17 @@ private async Task ExecuteAsync( + $"Underlying process ({process.Name}#{process.Id}) was gracefully terminated." ); + if (pipesCancellationToken.IsCancellationRequested) + { + throw new PipesCancelledException( + process.ExitCode, + process.StartTime, + process.ExitTime, + $"Process ({process.Name}#{process.Id}) has exited, but piping was terminated due to timeout.", + pipesCancellationToken + ); + } + // Validate the exit code if required if (process.ExitCode != 0 && Validation.IsZeroExitCodeValidationEnabled()) { @@ -296,7 +313,8 @@ Command execution failed because the underlying process ({process.Name}#{process // TODO: (breaking change) use optional parameters and remove the other overload public CommandTask ExecuteAsync( CancellationToken forcefulCancellationToken, - CancellationToken gracefulCancellationToken + CancellationToken gracefulCancellationToken, + CancellationToken pipesCancellationToken = default ) { var process = new ProcessEx(CreateStartInfo()); @@ -311,7 +329,12 @@ CancellationToken gracefulCancellationToken var processId = process.Id; return new CommandTask( - ExecuteAsync(process, forcefulCancellationToken, gracefulCancellationToken), + ExecuteAsync( + process, + forcefulCancellationToken, + gracefulCancellationToken, + pipesCancellationToken + ), processId ); } diff --git a/CliWrap/Exceptions/PipesCancelledException.cs b/CliWrap/Exceptions/PipesCancelledException.cs new file mode 100644 index 00000000..d5fa942f --- /dev/null +++ b/CliWrap/Exceptions/PipesCancelledException.cs @@ -0,0 +1,31 @@ +using System; +using System.Threading; + +namespace CliWrap.Exceptions; + +/// +/// The exception that is thrown when the pipes are cancelled. +/// +public class PipesCancelledException( + int exitCode, + DateTimeOffset startTime, + DateTimeOffset exitTime, + string message, + CancellationToken token +) : OperationCanceledException(message, token) +{ + /// + /// Exit code returned by the process. + /// + public int ExitCode { get; } = exitCode; + + /// + /// Time when the process started. + /// + public DateTimeOffset StartTime { get; } = startTime; + + /// + /// Time when the process exited. + /// + public DateTimeOffset ExitTime { get; } = exitTime; +} From 57e8d7608d547d85e4d09d7abbcc82e5e2131f0a Mon Sep 17 00:00:00 2001 From: Damian Milosz Date: Sat, 27 Apr 2024 14:56:48 +0200 Subject: [PATCH 2/3] Add WithPipingTimeout to command configuration --- .../Commands/RunProcessCommand.cs | 5 +- CliWrap.Tests/CancellationSpecs.cs | 12 ++--- CliWrap/Command.Execution.cs | 26 ++++----- CliWrap/Command.cs | 53 +++++++++++++++---- CliWrap/Exceptions/PipesCancelledException.cs | 5 +- CliWrap/ICommandConfiguration.cs | 8 ++- 6 files changed, 68 insertions(+), 41 deletions(-) diff --git a/CliWrap.Tests.Dummy/Commands/RunProcessCommand.cs b/CliWrap.Tests.Dummy/Commands/RunProcessCommand.cs index cab9348e..e2126793 100644 --- a/CliWrap.Tests.Dummy/Commands/RunProcessCommand.cs +++ b/CliWrap.Tests.Dummy/Commands/RunProcessCommand.cs @@ -1,5 +1,4 @@ -using System; -using System.Diagnostics; +using System.Diagnostics; using System.Threading.Tasks; using CliFx; using CliFx.Attributes; @@ -30,8 +29,6 @@ public ValueTask ExecuteAsync(IConsole console) process.StartInfo = startInfo; process.Start(); - console.Output.WriteLine(process.Id); - return default; } } diff --git a/CliWrap.Tests/CancellationSpecs.cs b/CliWrap.Tests/CancellationSpecs.cs index 835553d0..6e5e7cac 100644 --- a/CliWrap.Tests/CancellationSpecs.cs +++ b/CliWrap.Tests/CancellationSpecs.cs @@ -112,11 +112,12 @@ public async Task I_can_execute_a_command_with_buffering_and_cancel_it_immediate ex.CancellationToken.Should().Be(cts.Token); } +#if (NETCOREAPP || NETSTANDARD2_0_OR_GREATER) + [Fact(Timeout = 10000)] public async Task I_can_execute_a_command_and_cancel_piping_when_command_is_finished_but_child_process_remains_running() { // Arrange - var pipesCts = new CancellationTokenSource(); var cmd = Cli.Wrap(Dummy.Program.FilePath) .WithArguments( [ @@ -129,21 +130,20 @@ public async Task I_can_execute_a_command_and_cancel_piping_when_command_is_fini ] ) .WithStandardOutputPipe(PipeTarget.ToDelegate(_ => { })) - .WithStandardErrorPipe(PipeTarget.ToDelegate(_ => { })); + .WithStandardErrorPipe(PipeTarget.ToDelegate(_ => { })) + .WithPipingTimeout(TimeSpan.FromSeconds(1)); // Act - pipesCts.CancelAfter(TimeSpan.FromSeconds(1)); - var executeAsync = async () => - await cmd.ExecuteAsync(CancellationToken.None, CancellationToken.None, pipesCts.Token); + var executeAsync = async () => await cmd.ExecuteAsync(); // Assert var ex = await Assert.ThrowsAnyAsync( async () => await executeAsync() ); - ex.CancellationToken.Should().Be(pipesCts.Token); ex.ExitCode.Should().Be(0); } +#endif [Fact(Timeout = 15000)] public async Task I_can_execute_a_command_with_buffering_and_cancel_it_after_a_delay() diff --git a/CliWrap/Command.Execution.cs b/CliWrap/Command.Execution.cs index b9ed45f6..0db5f739 100644 --- a/CliWrap/Command.Execution.cs +++ b/CliWrap/Command.Execution.cs @@ -189,8 +189,7 @@ await StandardErrorPipe private async Task ExecuteAsync( ProcessEx process, CancellationToken forcefulCancellationToken = default, - CancellationToken gracefulCancellationToken = default, - CancellationToken pipesCancellationToken = default + CancellationToken gracefulCancellationToken = default ) { using var _ = process; @@ -221,8 +220,7 @@ private async Task ExecuteAsync( .ToAsyncDisposable(); using var pipeStdOutErrCts = CancellationTokenSource.CreateLinkedTokenSource( - forcefulCancellationToken, - pipesCancellationToken + forcefulCancellationToken ); // Start piping streams in the background var pipingTask = Task.WhenAll( @@ -244,7 +242,10 @@ private async Task ExecuteAsync( // If the pipe is still trying to transfer data, this will cause it to abort. await stdInCts.CancelAsync(); - // Wait until piping is done and propagate exceptions + // Cancel piping after specified time. + pipeStdOutErrCts.CancelAfter(PipingTimeout); + + // Wait until piping is done or cancelled and propagate exceptions await pipingTask.ConfigureAwait(false); } // Swallow exceptions caused by internal and user-provided cancellations, @@ -271,14 +272,13 @@ private async Task ExecuteAsync( + $"Underlying process ({process.Name}#{process.Id}) was gracefully terminated." ); - if (pipesCancellationToken.IsCancellationRequested) + if (pipeStdOutErrCts.IsCancellationRequested) { throw new PipesCancelledException( process.ExitCode, process.StartTime, process.ExitTime, - $"Process ({process.Name}#{process.Id}) has exited, but piping was terminated due to timeout.", - pipesCancellationToken + $"Process ({process.Name}#{process.Id}) has exited, but piping was terminated due to timeout." ); } @@ -313,8 +313,7 @@ Command execution failed because the underlying process ({process.Name}#{process // TODO: (breaking change) use optional parameters and remove the other overload public CommandTask ExecuteAsync( CancellationToken forcefulCancellationToken, - CancellationToken gracefulCancellationToken, - CancellationToken pipesCancellationToken = default + CancellationToken gracefulCancellationToken ) { var process = new ProcessEx(CreateStartInfo()); @@ -329,12 +328,7 @@ public CommandTask ExecuteAsync( var processId = process.Id; return new CommandTask( - ExecuteAsync( - process, - forcefulCancellationToken, - gracefulCancellationToken, - pipesCancellationToken - ), + ExecuteAsync(process, forcefulCancellationToken, gracefulCancellationToken), processId ); } diff --git a/CliWrap/Command.cs b/CliWrap/Command.cs index 27bed170..3a7da953 100644 --- a/CliWrap/Command.cs +++ b/CliWrap/Command.cs @@ -19,7 +19,8 @@ public partial class Command( CommandResultValidation validation, PipeSource standardInputPipe, PipeTarget standardOutputPipe, - PipeTarget standardErrorPipe + PipeTarget standardErrorPipe, + TimeSpan pipingTimeout ) : ICommandConfiguration { /// @@ -35,7 +36,8 @@ public Command(string targetFilePath) CommandResultValidation.ZeroExitCode, PipeSource.Null, PipeTarget.Null, - PipeTarget.Null + PipeTarget.Null, + TimeSpan.FromMilliseconds(int.MaxValue) ) { } /// @@ -50,6 +52,9 @@ public Command(string targetFilePath) /// public Credentials Credentials { get; } = credentials; + /// + public TimeSpan PipingTimeout { get; } = pipingTimeout; + /// public IReadOnlyDictionary EnvironmentVariables { get; } = environmentVariables; @@ -80,7 +85,8 @@ public Command WithTargetFile(string targetFilePath) => Validation, StandardInputPipe, StandardOutputPipe, - StandardErrorPipe + StandardErrorPipe, + PipingTimeout ); /// @@ -101,7 +107,8 @@ public Command WithArguments(string arguments) => Validation, StandardInputPipe, StandardOutputPipe, - StandardErrorPipe + StandardErrorPipe, + PipingTimeout ); /// @@ -147,7 +154,25 @@ public Command WithWorkingDirectory(string workingDirPath) => Validation, StandardInputPipe, StandardOutputPipe, - StandardErrorPipe + StandardErrorPipe, + PipingTimeout + ); + + /// + /// Creates a copy of this command, setting the piping timeout to the specified value. + /// + public Command WithPipingTimeout(TimeSpan pipingTimeout) => + new( + TargetFilePath, + Arguments, + WorkingDirPath, + Credentials, + EnvironmentVariables, + Validation, + StandardInputPipe, + StandardOutputPipe, + StandardErrorPipe, + pipingTimeout ); /// @@ -164,7 +189,8 @@ public Command WithCredentials(Credentials credentials) => Validation, StandardInputPipe, StandardOutputPipe, - StandardErrorPipe + StandardErrorPipe, + PipingTimeout ); /// @@ -196,7 +222,8 @@ public Command WithEnvironmentVariables( Validation, StandardInputPipe, StandardOutputPipe, - StandardErrorPipe + StandardErrorPipe, + PipingTimeout ); /// @@ -226,7 +253,8 @@ public Command WithValidation(CommandResultValidation validation) => validation, StandardInputPipe, StandardOutputPipe, - StandardErrorPipe + StandardErrorPipe, + PipingTimeout ); /// @@ -243,7 +271,8 @@ public Command WithStandardInputPipe(PipeSource source) => Validation, source, StandardOutputPipe, - StandardErrorPipe + StandardErrorPipe, + PipingTimeout ); /// @@ -260,7 +289,8 @@ public Command WithStandardOutputPipe(PipeTarget target) => Validation, StandardInputPipe, target, - StandardErrorPipe + StandardErrorPipe, + PipingTimeout ); /// @@ -277,7 +307,8 @@ public Command WithStandardErrorPipe(PipeTarget target) => Validation, StandardInputPipe, StandardOutputPipe, - target + target, + PipingTimeout ); /// diff --git a/CliWrap/Exceptions/PipesCancelledException.cs b/CliWrap/Exceptions/PipesCancelledException.cs index d5fa942f..fca8dc50 100644 --- a/CliWrap/Exceptions/PipesCancelledException.cs +++ b/CliWrap/Exceptions/PipesCancelledException.cs @@ -10,9 +10,8 @@ public class PipesCancelledException( int exitCode, DateTimeOffset startTime, DateTimeOffset exitTime, - string message, - CancellationToken token -) : OperationCanceledException(message, token) + string message +) : OperationCanceledException(message) { /// /// Exit code returned by the process. diff --git a/CliWrap/ICommandConfiguration.cs b/CliWrap/ICommandConfiguration.cs index 5a101d0b..694bcd1c 100644 --- a/CliWrap/ICommandConfiguration.cs +++ b/CliWrap/ICommandConfiguration.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; namespace CliWrap; @@ -37,6 +38,11 @@ public interface ICommandConfiguration /// CommandResultValidation Validation { get; } + /// + /// How long to wait for piping to be finished after the process exits. + /// + public TimeSpan PipingTimeout { get; } + /// /// Pipe source for the standard input stream of the underlying process. /// From 07006051988b33a3b532b87631c1b5b56e960dbf Mon Sep 17 00:00:00 2001 From: Damian Milosz Date: Mon, 29 Apr 2024 09:41:51 +0200 Subject: [PATCH 3/3] Make PipingTimeout optional --- CliWrap/Command.Execution.cs | 7 +++++-- CliWrap/Command.cs | 6 +++--- CliWrap/ICommandConfiguration.cs | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/CliWrap/Command.Execution.cs b/CliWrap/Command.Execution.cs index 0db5f739..bbd82fb9 100644 --- a/CliWrap/Command.Execution.cs +++ b/CliWrap/Command.Execution.cs @@ -242,8 +242,11 @@ private async Task ExecuteAsync( // If the pipe is still trying to transfer data, this will cause it to abort. await stdInCts.CancelAsync(); - // Cancel piping after specified time. - pipeStdOutErrCts.CancelAfter(PipingTimeout); + if (PipingTimeout != null) + { + // Cancel piping after specified time. + pipeStdOutErrCts.CancelAfter(PipingTimeout.Value); + } // Wait until piping is done or cancelled and propagate exceptions await pipingTask.ConfigureAwait(false); diff --git a/CliWrap/Command.cs b/CliWrap/Command.cs index 3a7da953..31937bbf 100644 --- a/CliWrap/Command.cs +++ b/CliWrap/Command.cs @@ -20,7 +20,7 @@ public partial class Command( PipeSource standardInputPipe, PipeTarget standardOutputPipe, PipeTarget standardErrorPipe, - TimeSpan pipingTimeout + TimeSpan? pipingTimeout ) : ICommandConfiguration { /// @@ -37,7 +37,7 @@ public Command(string targetFilePath) PipeSource.Null, PipeTarget.Null, PipeTarget.Null, - TimeSpan.FromMilliseconds(int.MaxValue) + null ) { } /// @@ -53,7 +53,7 @@ public Command(string targetFilePath) public Credentials Credentials { get; } = credentials; /// - public TimeSpan PipingTimeout { get; } = pipingTimeout; + public TimeSpan? PipingTimeout { get; } = pipingTimeout; /// public IReadOnlyDictionary EnvironmentVariables { get; } = diff --git a/CliWrap/ICommandConfiguration.cs b/CliWrap/ICommandConfiguration.cs index 694bcd1c..88f466bf 100644 --- a/CliWrap/ICommandConfiguration.cs +++ b/CliWrap/ICommandConfiguration.cs @@ -41,7 +41,7 @@ public interface ICommandConfiguration /// /// How long to wait for piping to be finished after the process exits. /// - public TimeSpan PipingTimeout { get; } + TimeSpan? PipingTimeout { get; } /// /// Pipe source for the standard input stream of the underlying process.