From 2d7843da970b8a412abbb2d2ad3de9010804e5f7 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 5 Nov 2018 13:57:00 +0100 Subject: [PATCH 01/42] Support Ctrl+C handling --- src/System.CommandLine/IConsole.cs | 2 + src/System.CommandLine/Invocation/Binder.cs | 4 ++ .../Invocation/InvocationContext.cs | 18 ++++++- .../Invocation/InvocationPipeline.cs | 49 ++++++++++--------- .../Invocation/SystemConsole.cs | 23 +++++++++ 5 files changed, 70 insertions(+), 26 deletions(-) diff --git a/src/System.CommandLine/IConsole.cs b/src/System.CommandLine/IConsole.cs index cdfc09df53..b90aeb2085 100644 --- a/src/System.CommandLine/IConsole.cs +++ b/src/System.CommandLine/IConsole.cs @@ -37,5 +37,7 @@ public interface IConsole : IDisposable bool IsVirtualTerminal { get; } void TryEnableVirtualTerminal(); + + Action CancelKeyPress { set; } } } diff --git a/src/System.CommandLine/Invocation/Binder.cs b/src/System.CommandLine/Invocation/Binder.cs index fc0b13a72b..b700b5ff40 100644 --- a/src/System.CommandLine/Invocation/Binder.cs +++ b/src/System.CommandLine/Invocation/Binder.cs @@ -33,6 +33,10 @@ public static object[] GetMethodArguments( { arguments.Add(context.Console); } + else if (parameterInfo.ParameterType == typeof(CancellationToken)) + { + arguments.Add(context.Canceled); + } else { var argument = context.ParseResult diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index 2788934d24..9eb4db1cda 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -1,11 +1,15 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.Threading; + namespace System.CommandLine.Invocation { public sealed class InvocationContext : IDisposable { - private readonly IDisposable _onDispose; + private readonly CancellationTokenSource _cts = new CancellationTokenSource(); + private readonly bool _disposeConsole; + private readonly Action _cancelAction; public InvocationContext( ParseResult parseResult, @@ -24,6 +28,8 @@ public InvocationContext( Console = SystemConsole.Create(); _onDispose = Console; } + + console.CancelKeyPress = () => Cancel(); } public Parser Parser { get; } @@ -36,9 +42,17 @@ public InvocationContext( public IInvocationResult InvocationResult { get; set; } + public CancellationToken Canceled => _cts.Token; + + public void Cancel() => _cts.Cancel(); + public void Dispose() { - _onDispose?.Dispose(); + Console.CancelKeyPress = null; + if (_disposeConsole) + { + Console.Dispose(); + } } } } diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index 3649e2e7a6..6c43bfc187 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -22,39 +22,40 @@ public InvocationPipeline( public async Task InvokeAsync(IConsole console = null) { - var context = new InvocationContext(parseResult, + using (var context = new InvocationContext(parseResult, parser, - console); - - var invocations = new List(context.Parser.Configuration.Middleware); - - invocations.Add(async (invocationContext, next) => + console)) { - if (invocationContext - .ParseResult - .CommandResult - .Command is Command command) - { - var handler = command.Handler; + var invocations = new List(context.Parser.Configuration.Middleware); - if (handler != null) + invocations.Add(async (invocationContext, next) => + { + if (invocationContext + .ParseResult + .CommandResult + .Command is Command command) { - context.ResultCode = await handler.InvokeAsync(invocationContext); + var handler = command.Handler; + + if (handler != null) + { + context.ResultCode = await handler.InvokeAsync(invocationContext); + } } - } - }); + }); - var invocationChain = invocations.Aggregate( - (first, second) => - ((ctx, next) => - first(ctx, - c => second(c, next)))); + var invocationChain = invocations.Aggregate( + (first, second) => + ((ctx, next) => + first(ctx, + c => second(c, next)))); - await invocationChain(context, invocationContext => Task.CompletedTask); + await invocationChain(context, invocationContext => Task.CompletedTask); - context.InvocationResult?.Apply(context); + context.InvocationResult?.Apply(context); - return context.ResultCode; + return context.ResultCode; + } } } } diff --git a/src/System.CommandLine/Invocation/SystemConsole.cs b/src/System.CommandLine/Invocation/SystemConsole.cs index ecc421427f..0cd0c3108e 100644 --- a/src/System.CommandLine/Invocation/SystemConsole.cs +++ b/src/System.CommandLine/Invocation/SystemConsole.cs @@ -11,6 +11,7 @@ internal class SystemConsole : IConsole private VirtualTerminalMode _virtualTerminalMode; private readonly ConsoleColor _initialForegroundColor; private readonly ConsoleColor _initialBackgroundColor; + private readonly ConsoleCancelEventHandler _cancelEventHandler; internal SystemConsole() { @@ -99,10 +100,32 @@ private void ResetConsole() public void Dispose() { + CancelKeyPress = null; ResetConsole(); GC.SuppressFinalize(this); } + public Action CancelKeyPress + { + set + { + if (value != null) + { + if (_cancelEventHandler != null) + { + throw new ArgumentException("Action is already assigned.", nameof(value)); + } + _cancelEventHandler = (o, e) => { e.Cancel = true; value(); }; + ConsoleExtensions.CancelKeyPress += _cancelEventHandler; + } + else if (_cancelEventHandler != null) + { + ConsoleExtensions.CancelKeyPress -= _cancelEventHandler; + _cancelEventHandler = null; + } + } + } + ~SystemConsole() { ResetConsole(); From 235bf17416ec6661e11450d9d31b9548be71ceb8 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 13 Nov 2018 09:22:58 +0100 Subject: [PATCH 02/42] Reimplement using middleware --- src/System.CommandLine/IConsole.cs | 2 +- src/System.CommandLine/Invocation/Binder.cs | 1 + .../Invocation/InvocationContext.cs | 6 +---- .../Invocation/InvocationExtensions.cs | 23 +++++++++++++++++++ .../Invocation/SystemConsole.cs | 23 +++---------------- 5 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/System.CommandLine/IConsole.cs b/src/System.CommandLine/IConsole.cs index b90aeb2085..45db6ab78f 100644 --- a/src/System.CommandLine/IConsole.cs +++ b/src/System.CommandLine/IConsole.cs @@ -38,6 +38,6 @@ public interface IConsole : IDisposable void TryEnableVirtualTerminal(); - Action CancelKeyPress { set; } + event ConsoleCancelEventHandler CancelKeyPress; } } diff --git a/src/System.CommandLine/Invocation/Binder.cs b/src/System.CommandLine/Invocation/Binder.cs index b700b5ff40..73cf64d463 100644 --- a/src/System.CommandLine/Invocation/Binder.cs +++ b/src/System.CommandLine/Invocation/Binder.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; +using System.Threading; namespace System.CommandLine.Invocation { diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index 9eb4db1cda..e08795f213 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -9,7 +9,6 @@ public sealed class InvocationContext : IDisposable { private readonly CancellationTokenSource _cts = new CancellationTokenSource(); private readonly bool _disposeConsole; - private readonly Action _cancelAction; public InvocationContext( ParseResult parseResult, @@ -26,10 +25,8 @@ public InvocationContext( else { Console = SystemConsole.Create(); - _onDispose = Console; + _disposeConsole = true; } - - console.CancelKeyPress = () => Cancel(); } public Parser Parser { get; } @@ -48,7 +45,6 @@ public InvocationContext( public void Dispose() { - Console.CancelKeyPress = null; if (_disposeConsole) { Console.Dispose(); diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index dcfaf13a72..6125bd628c 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -14,6 +14,29 @@ namespace System.CommandLine.Invocation { public static class InvocationExtensions { + public static CommandLineBuilder CancelOnCancelKey(this CommandLineBuilder builder) + { + builder.AddMiddleware(async (context, next) => + { + ConsoleCancelEventHandler handler = (_, args) => + { + args.Cancel = true; + context.Cancel(); + }; + try + { + context.Console.CancelKeyPress += handler; + await next(context); + } + finally + { + context.Console.CancelKeyPress -= handler; + } + }, CommandLineBuilder.MiddlewareOrder.Middle); + + return builder; + } + public static CommandLineBuilder UseMiddleware( this CommandLineBuilder builder, InvocationMiddleware middleware) diff --git a/src/System.CommandLine/Invocation/SystemConsole.cs b/src/System.CommandLine/Invocation/SystemConsole.cs index 0cd0c3108e..73f33c41f7 100644 --- a/src/System.CommandLine/Invocation/SystemConsole.cs +++ b/src/System.CommandLine/Invocation/SystemConsole.cs @@ -11,7 +11,6 @@ internal class SystemConsole : IConsole private VirtualTerminalMode _virtualTerminalMode; private readonly ConsoleColor _initialForegroundColor; private readonly ConsoleColor _initialBackgroundColor; - private readonly ConsoleCancelEventHandler _cancelEventHandler; internal SystemConsole() { @@ -100,30 +99,14 @@ private void ResetConsole() public void Dispose() { - CancelKeyPress = null; ResetConsole(); GC.SuppressFinalize(this); } - public Action CancelKeyPress + public event ConsoleCancelEventHandler CancelKeyPress { - set - { - if (value != null) - { - if (_cancelEventHandler != null) - { - throw new ArgumentException("Action is already assigned.", nameof(value)); - } - _cancelEventHandler = (o, e) => { e.Cancel = true; value(); }; - ConsoleExtensions.CancelKeyPress += _cancelEventHandler; - } - else if (_cancelEventHandler != null) - { - ConsoleExtensions.CancelKeyPress -= _cancelEventHandler; - _cancelEventHandler = null; - } - } + add => Console.CancelKeyPress += value; + remove => Console.CancelKeyPress -= value; } ~SystemConsole() From bf6528f7e2f5f9c62de4d0711235ff797d020503 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 13 Nov 2018 09:45:00 +0100 Subject: [PATCH 03/42] Add CancelOnUnload --- .../Invocation/InvocationExtensions.cs | 28 +++++++++++++++++++ .../System.CommandLine.csproj | 1 + 2 files changed, 29 insertions(+) diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index 6125bd628c..6ac5bd21ef 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -6,7 +6,9 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.Loader; using System.Text; +using System.Threading; using System.Threading.Tasks; using static System.Environment; @@ -37,6 +39,32 @@ public static CommandLineBuilder CancelOnCancelKey(this CommandLineBuilder build return builder; } + public static CommandLineBuilder CancelOnUnload(this CommandLineBuilder builder) + { + builder.AddMiddleware(async (context, next) => + { + var mre = new ManualResetEventSlim(initialState: false); + Action handler = _ => + { + context.Cancel(); + mre.Wait(); + Environment.ExitCode = context.ResultCode; + }; + try + { + AssemblyLoadContext.Default.Unloading += handler; + await next(context); + } + finally + { + AssemblyLoadContext.Default.Unloading -= handler; + mre.Set(); + } + }, CommandLineBuilder.MiddlewareOrder.Middle); + + return builder; + } + public static CommandLineBuilder UseMiddleware( this CommandLineBuilder builder, InvocationMiddleware middleware) diff --git a/src/System.CommandLine/System.CommandLine.csproj b/src/System.CommandLine/System.CommandLine.csproj index 6d632eb88c..66895b341c 100644 --- a/src/System.CommandLine/System.CommandLine.csproj +++ b/src/System.CommandLine/System.CommandLine.csproj @@ -13,6 +13,7 @@ + From 1076155c511a8f119e3c8172dc2ad234d9a81162 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 13 Nov 2018 09:53:09 +0100 Subject: [PATCH 04/42] Undo changes to InvocationPipeline --- .../Invocation/InvocationPipeline.cs | 49 +++++++++---------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index 6c43bfc187..3649e2e7a6 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -22,40 +22,39 @@ public InvocationPipeline( public async Task InvokeAsync(IConsole console = null) { - using (var context = new InvocationContext(parseResult, + var context = new InvocationContext(parseResult, parser, - console)) - { - var invocations = new List(context.Parser.Configuration.Middleware); + console); + + var invocations = new List(context.Parser.Configuration.Middleware); - invocations.Add(async (invocationContext, next) => + invocations.Add(async (invocationContext, next) => + { + if (invocationContext + .ParseResult + .CommandResult + .Command is Command command) { - if (invocationContext - .ParseResult - .CommandResult - .Command is Command command) - { - var handler = command.Handler; + var handler = command.Handler; - if (handler != null) - { - context.ResultCode = await handler.InvokeAsync(invocationContext); - } + if (handler != null) + { + context.ResultCode = await handler.InvokeAsync(invocationContext); } - }); + } + }); - var invocationChain = invocations.Aggregate( - (first, second) => - ((ctx, next) => - first(ctx, - c => second(c, next)))); + var invocationChain = invocations.Aggregate( + (first, second) => + ((ctx, next) => + first(ctx, + c => second(c, next)))); - await invocationChain(context, invocationContext => Task.CompletedTask); + await invocationChain(context, invocationContext => Task.CompletedTask); - context.InvocationResult?.Apply(context); + context.InvocationResult?.Apply(context); - return context.ResultCode; - } + return context.ResultCode; } } } From 0da0b473c419173fc3e7fb8c31811d96ef04b7a8 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 13 Nov 2018 09:54:47 +0100 Subject: [PATCH 05/42] Undo changes to InvocationContext --- src/System.CommandLine/Invocation/InvocationContext.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index e08795f213..c260fe0a07 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -7,8 +7,8 @@ namespace System.CommandLine.Invocation { public sealed class InvocationContext : IDisposable { + private readonly IDisposable _onDispose; private readonly CancellationTokenSource _cts = new CancellationTokenSource(); - private readonly bool _disposeConsole; public InvocationContext( ParseResult parseResult, @@ -25,7 +25,7 @@ public InvocationContext( else { Console = SystemConsole.Create(); - _disposeConsole = true; + _onDispose = Console; } } @@ -45,10 +45,7 @@ public InvocationContext( public void Dispose() { - if (_disposeConsole) - { - Console.Dispose(); - } + _onDispose?.Dispose(); } } } From 5180d09861050da45a56f44560a1c1cf16cdceda Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 13 Nov 2018 10:00:54 +0100 Subject: [PATCH 06/42] Update MiddlewareOrder --- src/System.CommandLine/Builder/CommandLineBuilder.cs | 3 ++- src/System.CommandLine/Invocation/InvocationExtensions.cs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine/Builder/CommandLineBuilder.cs b/src/System.CommandLine/Builder/CommandLineBuilder.cs index 3ba3c8afff..a8195d0d46 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilder.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilder.cs @@ -61,7 +61,8 @@ internal void AddMiddleware( internal static class MiddlewareOrder { - public const int ExceptionHandler = int.MinValue; + public const int UnloadHandler = int.MinValue; + public const int ExceptionHandler = UnloadHandler + 100; public const int Configuration = ExceptionHandler + 100; public const int Preprocessing = Configuration + 100; public const int AfterPreprocessing = Preprocessing + 100; diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index 6ac5bd21ef..79d10aee85 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -34,7 +34,7 @@ public static CommandLineBuilder CancelOnCancelKey(this CommandLineBuilder build { context.Console.CancelKeyPress -= handler; } - }, CommandLineBuilder.MiddlewareOrder.Middle); + }, CommandLineBuilder.MiddlewareOrder.Configuration); return builder; } @@ -60,7 +60,7 @@ public static CommandLineBuilder CancelOnUnload(this CommandLineBuilder builder) AssemblyLoadContext.Default.Unloading -= handler; mre.Set(); } - }, CommandLineBuilder.MiddlewareOrder.Middle); + }, CommandLineBuilder.MiddlewareOrder.UnloadHandler); return builder; } From 3866b9e6a7e384f5a01360745d146d56939672d8 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 13 Nov 2018 18:20:04 +0100 Subject: [PATCH 07/42] Add CancelOnCancelKeyTests --- .../CancelOnCancelKeyTests.cs | 52 +++++++++++++++++++ src/System.CommandLine.Tests/TestConsole.cs | 5 ++ src/System.CommandLine/IConsole.cs | 2 +- .../Invocation/CommandHandler.cs | 4 ++ .../Invocation/InvocationExtensions.cs | 10 ++-- .../Invocation/SystemConsole.cs | 31 +++++++++-- 6 files changed, 93 insertions(+), 11 deletions(-) create mode 100644 src/System.CommandLine.Tests/CancelOnCancelKeyTests.cs diff --git a/src/System.CommandLine.Tests/CancelOnCancelKeyTests.cs b/src/System.CommandLine.Tests/CancelOnCancelKeyTests.cs new file mode 100644 index 0000000000..1e90c4500f --- /dev/null +++ b/src/System.CommandLine.Tests/CancelOnCancelKeyTests.cs @@ -0,0 +1,52 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.CommandLine.Builder; +using System.CommandLine.Invocation; +using System.Threading; +using System.Threading.Tasks; +using FluentAssertions; +using Xunit; + +namespace System.CommandLine.Tests +{ + public class CancelOnCancelKeyTests + { + private readonly TestConsole _console = new TestConsole(); + + [Fact] + public async Task ConsoleCancelKey_cancels_InvocationContext_CancellationToken() + { + const int timeout = 100000; + + bool isCanceled = false; + bool hasTimedout = false; + + Task invocation = new CommandLineBuilder() + .AddCommand(new Command("the-command", + handler: CommandHandler.Create(async ct => + { + try + { + await Task.Delay(timeout, ct); + hasTimedout = true; + } + catch (TaskCanceledException) + { + isCanceled = true; + } + }))) + .CancelOnCancelKey() + .Build() + .InvokeAsync("the-command", _console); + + _console.EmitCancelKeyPress(); + + var result = await invocation; + + Assert.True(hasTimedout || isCanceled); + Assert.False(hasTimedout); + Assert.True(isCanceled); + } + } +} diff --git a/src/System.CommandLine.Tests/TestConsole.cs b/src/System.CommandLine.Tests/TestConsole.cs index 1193f96331..ccd4c71de6 100644 --- a/src/System.CommandLine.Tests/TestConsole.cs +++ b/src/System.CommandLine.Tests/TestConsole.cs @@ -313,5 +313,10 @@ public ForegroundColorChanged(ConsoleColor foregroundColor) public ConsoleColor ForegroundColor { get; } } + + public Action CancelKeyPress { set; get; } + + public void EmitCancelKeyPress() + => CancelKeyPress?.Invoke(); } } diff --git a/src/System.CommandLine/IConsole.cs b/src/System.CommandLine/IConsole.cs index 45db6ab78f..b90aeb2085 100644 --- a/src/System.CommandLine/IConsole.cs +++ b/src/System.CommandLine/IConsole.cs @@ -38,6 +38,6 @@ public interface IConsole : IDisposable void TryEnableVirtualTerminal(); - event ConsoleCancelEventHandler CancelKeyPress; + Action CancelKeyPress { set; } } } diff --git a/src/System.CommandLine/Invocation/CommandHandler.cs b/src/System.CommandLine/Invocation/CommandHandler.cs index 8d8d68179d..dc89013fd4 100644 --- a/src/System.CommandLine/Invocation/CommandHandler.cs +++ b/src/System.CommandLine/Invocation/CommandHandler.cs @@ -20,6 +20,10 @@ public static ICommandHandler Create( Action action) => new MethodBindingCommandHandler(action); + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + public static ICommandHandler Create( Action action) => new MethodBindingCommandHandler(action); diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index 79d10aee85..9c3865442a 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -20,19 +20,15 @@ public static CommandLineBuilder CancelOnCancelKey(this CommandLineBuilder build { builder.AddMiddleware(async (context, next) => { - ConsoleCancelEventHandler handler = (_, args) => - { - args.Cancel = true; - context.Cancel(); - }; + Action handler = () => context.Cancel(); try { - context.Console.CancelKeyPress += handler; + context.Console.CancelKeyPress = handler; await next(context); } finally { - context.Console.CancelKeyPress -= handler; + context.Console.CancelKeyPress = null; } }, CommandLineBuilder.MiddlewareOrder.Configuration); diff --git a/src/System.CommandLine/Invocation/SystemConsole.cs b/src/System.CommandLine/Invocation/SystemConsole.cs index 73f33c41f7..b778dd60ff 100644 --- a/src/System.CommandLine/Invocation/SystemConsole.cs +++ b/src/System.CommandLine/Invocation/SystemConsole.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; using System.CommandLine.Rendering; using System.IO; @@ -11,6 +12,8 @@ internal class SystemConsole : IConsole private VirtualTerminalMode _virtualTerminalMode; private readonly ConsoleColor _initialForegroundColor; private readonly ConsoleColor _initialBackgroundColor; + private ConsoleCancelEventHandler _consoleCancelEventHandler; + private Action _cancelKeyPress; internal SystemConsole() { @@ -103,10 +106,32 @@ public void Dispose() GC.SuppressFinalize(this); } - public event ConsoleCancelEventHandler CancelKeyPress + public Action CancelKeyPress { - add => Console.CancelKeyPress += value; - remove => Console.CancelKeyPress -= value; + set + { + if (value == null) + { + Console.CancelKeyPress -= _consoleCancelEventHandler; + _consoleCancelEventHandler = null; + } + else if (_consoleCancelEventHandler == null) + { + _consoleCancelEventHandler = OnConsoleCancelEvent; + Console.CancelKeyPress += _consoleCancelEventHandler; + } + _cancelKeyPress = value; + } + } + + private void OnConsoleCancelEvent(object sender, ConsoleCancelEventArgs args) + { + Action cancelKeyPress = _cancelKeyPress; + if (cancelKeyPress != null) + { + args.Cancel = true; + cancelKeyPress?.Invoke(); + } } ~SystemConsole() From b7930c75570aa6e02e66b4dbb99bacd8ef68fef3 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 14 Nov 2018 16:01:30 +0100 Subject: [PATCH 08/42] Make InvocationContext aware if cancellation is enabled --- .../CancelOnCancelKeyTests.cs | 52 ---------------- .../InvocationPipelineTests.cs | 59 +++++++++++++++++++ src/System.CommandLine.Tests/TestConsole.cs | 5 -- src/System.CommandLine/IConsole.cs | 2 - src/System.CommandLine/Invocation/Binder.cs | 3 +- .../Invocation/InvocationContext.cs | 37 +++++++++++- .../Invocation/InvocationExtensions.cs | 20 +++++-- .../Invocation/SystemConsole.cs | 30 ---------- 8 files changed, 109 insertions(+), 99 deletions(-) delete mode 100644 src/System.CommandLine.Tests/CancelOnCancelKeyTests.cs diff --git a/src/System.CommandLine.Tests/CancelOnCancelKeyTests.cs b/src/System.CommandLine.Tests/CancelOnCancelKeyTests.cs deleted file mode 100644 index 1e90c4500f..0000000000 --- a/src/System.CommandLine.Tests/CancelOnCancelKeyTests.cs +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.CommandLine.Builder; -using System.CommandLine.Invocation; -using System.Threading; -using System.Threading.Tasks; -using FluentAssertions; -using Xunit; - -namespace System.CommandLine.Tests -{ - public class CancelOnCancelKeyTests - { - private readonly TestConsole _console = new TestConsole(); - - [Fact] - public async Task ConsoleCancelKey_cancels_InvocationContext_CancellationToken() - { - const int timeout = 100000; - - bool isCanceled = false; - bool hasTimedout = false; - - Task invocation = new CommandLineBuilder() - .AddCommand(new Command("the-command", - handler: CommandHandler.Create(async ct => - { - try - { - await Task.Delay(timeout, ct); - hasTimedout = true; - } - catch (TaskCanceledException) - { - isCanceled = true; - } - }))) - .CancelOnCancelKey() - .Build() - .InvokeAsync("the-command", _console); - - _console.EmitCancelKeyPress(); - - var result = await invocation; - - Assert.True(hasTimedout || isCanceled); - Assert.False(hasTimedout); - Assert.True(isCanceled); - } - } -} diff --git a/src/System.CommandLine.Tests/InvocationPipelineTests.cs b/src/System.CommandLine.Tests/InvocationPipelineTests.cs index 6500dce9bb..a3bad10be6 100644 --- a/src/System.CommandLine.Tests/InvocationPipelineTests.cs +++ b/src/System.CommandLine.Tests/InvocationPipelineTests.cs @@ -6,6 +6,7 @@ using FluentAssertions; using System.Linq; using System.Reflection; +using System.Threading; using System.Threading.Tasks; using Xunit; @@ -139,5 +140,63 @@ public async Task Invocation_can_be_short_circuited_by_middleware_by_not_calling middlewareWasCalled.Should().BeTrue(); handlerWasCalled.Should().BeFalse(); } + + [Fact] + public async Task Invocation_can_be_cancelled_by_middleware_before_cancellation_enabled() + { + var command = new Command("the-command"); + command.Handler = CommandHandler.Create((InvocationContext context) => + { + CancellationToken ct = context.EnableCancellation(); + // The middleware canceled the request + Assert.True(ct.IsCancellationRequested); + }); + + var parser = new CommandLineBuilder() + .UseMiddleware(async (context, next) => + { + bool cancellationEnabled = context.Cancel(); + // Cancellation is not yet enabled + Assert.False(cancellationEnabled); + await next(context); + }) + .AddCommand(command) + .Build(); + + await parser.InvokeAsync("the-command", new TestConsole()); + } + + [Fact] + public async Task Invocation_can_be_cancelled_by_middleware_after_cancellation_enabled() + { + const int timeout = 5000; + + var command = new Command("the-command"); + command.Handler = CommandHandler.Create(async (InvocationContext context) => + { + CancellationToken ct = context.EnableCancellation(); + // The middleware hasn't cancelled our request yet. + Assert.False(ct.IsCancellationRequested); + + await Task.Delay(timeout, ct); + }); + + var parser = new CommandLineBuilder() + .UseMiddleware(async (context, next) => + { + Task task = next(context); + + bool cancellationEnabled = context.Cancel(); + // Cancellation is enabled by next + Assert.True(cancellationEnabled); + + await task; + }) + .AddCommand(command) + .Build(); + + var invocation = parser.InvokeAsync("the-command", new TestConsole()); + await Assert.ThrowsAnyAsync(() => invocation); + } } } diff --git a/src/System.CommandLine.Tests/TestConsole.cs b/src/System.CommandLine.Tests/TestConsole.cs index ccd4c71de6..1193f96331 100644 --- a/src/System.CommandLine.Tests/TestConsole.cs +++ b/src/System.CommandLine.Tests/TestConsole.cs @@ -313,10 +313,5 @@ public ForegroundColorChanged(ConsoleColor foregroundColor) public ConsoleColor ForegroundColor { get; } } - - public Action CancelKeyPress { set; get; } - - public void EmitCancelKeyPress() - => CancelKeyPress?.Invoke(); } } diff --git a/src/System.CommandLine/IConsole.cs b/src/System.CommandLine/IConsole.cs index b90aeb2085..cdfc09df53 100644 --- a/src/System.CommandLine/IConsole.cs +++ b/src/System.CommandLine/IConsole.cs @@ -37,7 +37,5 @@ public interface IConsole : IDisposable bool IsVirtualTerminal { get; } void TryEnableVirtualTerminal(); - - Action CancelKeyPress { set; } } } diff --git a/src/System.CommandLine/Invocation/Binder.cs b/src/System.CommandLine/Invocation/Binder.cs index 73cf64d463..7cf61edd6b 100644 --- a/src/System.CommandLine/Invocation/Binder.cs +++ b/src/System.CommandLine/Invocation/Binder.cs @@ -36,7 +36,8 @@ public static object[] GetMethodArguments( } else if (parameterInfo.ParameterType == typeof(CancellationToken)) { - arguments.Add(context.Canceled); + CancellationToken ct = context.EnableCancellation(); + arguments.Add(ct); } else { diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index c260fe0a07..4b1446cd72 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -8,7 +8,9 @@ namespace System.CommandLine.Invocation public sealed class InvocationContext : IDisposable { private readonly IDisposable _onDispose; - private readonly CancellationTokenSource _cts = new CancellationTokenSource(); + private CancellationTokenSource _cts; + private bool _isCancellationEnabled; + private readonly object _gate = new object(); public InvocationContext( ParseResult parseResult, @@ -39,9 +41,38 @@ public InvocationContext( public IInvocationResult InvocationResult { get; set; } - public CancellationToken Canceled => _cts.Token; + // Indicates the invocation can be cancelled. + // This returns a CancellationToken that will be set when the invocation + // is cancelled. The method may return a CancellationToken that is already + // cancelled. + public CancellationToken EnableCancellation() + { + lock (_gate) + { + _isCancellationEnabled = true; + if (_cts == null) + { + _cts = new CancellationTokenSource(); + } + return _cts.Token; + } + } - public void Cancel() => _cts.Cancel(); + // The return value indicates if the Invocation has cancellation enabled. + // When Cancel returns false, the Middleware may decide to forcefully + // end the process, for example, by calling Environment.Exit. + public bool Cancel() + { + lock (_gate) + { + if (_cts == null) + { + _cts = new CancellationTokenSource(); + } + _cts.Cancel(); + return _isCancellationEnabled; + } + } public void Dispose() { diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index 9c3865442a..db81b77142 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -20,15 +20,21 @@ public static CommandLineBuilder CancelOnCancelKey(this CommandLineBuilder build { builder.AddMiddleware(async (context, next) => { - Action handler = () => context.Cancel(); + ConsoleCancelEventHandler handler = (_, args) => + { + if (context.Cancel()) + { + args.Cancel = true; + } + }; try { - context.Console.CancelKeyPress = handler; + Console.CancelKeyPress += handler; await next(context); } finally { - context.Console.CancelKeyPress = null; + Console.CancelKeyPress -= handler; } }, CommandLineBuilder.MiddlewareOrder.Configuration); @@ -42,9 +48,11 @@ public static CommandLineBuilder CancelOnUnload(this CommandLineBuilder builder) var mre = new ManualResetEventSlim(initialState: false); Action handler = _ => { - context.Cancel(); - mre.Wait(); - Environment.ExitCode = context.ResultCode; + if (context.Cancel()) + { + mre.Wait(); + Environment.ExitCode = context.ResultCode; + } }; try { diff --git a/src/System.CommandLine/Invocation/SystemConsole.cs b/src/System.CommandLine/Invocation/SystemConsole.cs index b778dd60ff..d5770bb2f8 100644 --- a/src/System.CommandLine/Invocation/SystemConsole.cs +++ b/src/System.CommandLine/Invocation/SystemConsole.cs @@ -12,8 +12,6 @@ internal class SystemConsole : IConsole private VirtualTerminalMode _virtualTerminalMode; private readonly ConsoleColor _initialForegroundColor; private readonly ConsoleColor _initialBackgroundColor; - private ConsoleCancelEventHandler _consoleCancelEventHandler; - private Action _cancelKeyPress; internal SystemConsole() { @@ -106,34 +104,6 @@ public void Dispose() GC.SuppressFinalize(this); } - public Action CancelKeyPress - { - set - { - if (value == null) - { - Console.CancelKeyPress -= _consoleCancelEventHandler; - _consoleCancelEventHandler = null; - } - else if (_consoleCancelEventHandler == null) - { - _consoleCancelEventHandler = OnConsoleCancelEvent; - Console.CancelKeyPress += _consoleCancelEventHandler; - } - _cancelKeyPress = value; - } - } - - private void OnConsoleCancelEvent(object sender, ConsoleCancelEventArgs args) - { - Action cancelKeyPress = _cancelKeyPress; - if (cancelKeyPress != null) - { - args.Cancel = true; - cancelKeyPress?.Invoke(); - } - } - ~SystemConsole() { ResetConsole(); From 7374992f06de1e8bdf11aed7d6f3c7bc7cbe585c Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 15 Nov 2018 09:17:04 +0100 Subject: [PATCH 09/42] Merge CancelOn* methods into UseConsoleLifetime and improve interaction with UseExceptionHandler --- .../InvocationPipelineTests.cs | 3 +- .../UseExceptionHandlerTests.cs | 16 ++++++ .../Builder/CommandLineBuilder.cs | 4 +- .../Invocation/InvocationContext.cs | 3 ++ .../Invocation/InvocationExtensions.cs | 53 +++++++------------ .../Invocation/MethodBindingCommandHandler.cs | 19 +++++-- .../System.CommandLine.csproj | 1 - 7 files changed, 57 insertions(+), 42 deletions(-) diff --git a/src/System.CommandLine.Tests/InvocationPipelineTests.cs b/src/System.CommandLine.Tests/InvocationPipelineTests.cs index a3bad10be6..0c0ce03407 100644 --- a/src/System.CommandLine.Tests/InvocationPipelineTests.cs +++ b/src/System.CommandLine.Tests/InvocationPipelineTests.cs @@ -77,9 +77,8 @@ public void When_command_handler_throws_then_InvokeAsync_does_not_handle_the_exc Func invoke = async () => await parser.InvokeAsync("the-command", _console); invoke.Should() - .Throw() + .Throw() .Which - .InnerException .Message .Should() .Be("oops!"); diff --git a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs index 4e67ac60bc..2a7e2c4334 100644 --- a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs +++ b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs @@ -3,6 +3,7 @@ using System.CommandLine.Builder; using System.CommandLine.Invocation; +using System.Threading; using System.Threading.Tasks; using FluentAssertions; using Xunit; @@ -73,6 +74,21 @@ public async Task UseExceptionHandler_catches_command_handler_exceptions_and_wri _console.Error.ToString().Should().Contain("System.Exception: oops!"); } + [Fact] + public async Task UseExceptionHandler_doesnt_write_operationcancelledexception_details_when_context_is_cancelled() + { + var parser = new CommandLineBuilder() + .UseMiddleware((InvocationContext context) => context.Cancel()) + .AddCommand("the-command", "", + cmd => cmd.OnExecute((CancellationToken ct) => ct.ThrowIfCancellationRequested())) + .UseExceptionHandler() + .Build(); + + await parser.InvokeAsync("the-command", _console); + + _console.Error.ToString().Should().Equals(""); + } + [Fact] public async Task Declaration_of_UseExceptionHandler_can_come_before_other_middleware() { diff --git a/src/System.CommandLine/Builder/CommandLineBuilder.cs b/src/System.CommandLine/Builder/CommandLineBuilder.cs index a8195d0d46..e6b6428716 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilder.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilder.cs @@ -61,8 +61,8 @@ internal void AddMiddleware( internal static class MiddlewareOrder { - public const int UnloadHandler = int.MinValue; - public const int ExceptionHandler = UnloadHandler + 100; + public const int ProcessExit = int.MinValue; + public const int ExceptionHandler = ProcessExit + 100; public const int Configuration = ExceptionHandler + 100; public const int Preprocessing = Configuration + 100; public const int AfterPreprocessing = Preprocessing + 100; diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index 4b1446cd72..e764adfb7b 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -74,6 +74,9 @@ public bool Cancel() } } + public bool IsCancelled => + _cts?.Token.IsCancellationRequested == true; + public void Dispose() { _onDispose?.Dispose(); diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index db81b77142..887c0f65e9 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -6,7 +6,6 @@ using System.IO; using System.Linq; using System.Reflection; -using System.Runtime.Loader; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -16,55 +15,39 @@ namespace System.CommandLine.Invocation { public static class InvocationExtensions { - public static CommandLineBuilder CancelOnCancelKey(this CommandLineBuilder builder) + public static CommandLineBuilder UseConsoleLifetime(this CommandLineBuilder builder) { builder.AddMiddleware(async (context, next) => { - ConsoleCancelEventHandler handler = (_, args) => + ConsoleCancelEventHandler consoleHandler = (_, args) => { if (context.Cancel()) { args.Cancel = true; } }; - try - { - Console.CancelKeyPress += handler; - await next(context); - } - finally - { - Console.CancelKeyPress -= handler; - } - }, CommandLineBuilder.MiddlewareOrder.Configuration); - - return builder; - } - - public static CommandLineBuilder CancelOnUnload(this CommandLineBuilder builder) - { - builder.AddMiddleware(async (context, next) => - { - var mre = new ManualResetEventSlim(initialState: false); - Action handler = _ => + var blockProcessExit = new ManualResetEventSlim(initialState: false); + EventHandler processExitHandler = (_1, _2) => { if (context.Cancel()) { - mre.Wait(); + blockProcessExit.Wait(); Environment.ExitCode = context.ResultCode; } }; try { - AssemblyLoadContext.Default.Unloading += handler; + Console.CancelKeyPress += consoleHandler; + AppDomain.CurrentDomain.ProcessExit += processExitHandler; await next(context); } finally { - AssemblyLoadContext.Default.Unloading -= handler; - mre.Set(); + Console.CancelKeyPress -= consoleHandler; + AppDomain.CurrentDomain.ProcessExit -= processExitHandler; + blockProcessExit.Set(); } - }, CommandLineBuilder.MiddlewareOrder.UnloadHandler); + }, CommandLineBuilder.MiddlewareOrder.ProcessExit); return builder; } @@ -130,11 +113,15 @@ public static CommandLineBuilder UseExceptionHandler( } catch (Exception exception) { - context.Console.ResetColor(); - context.Console.ForegroundColor = ConsoleColor.Red; - context.Console.Error.Write("Unhandled exception: "); - context.Console.Error.WriteLine(exception.ToString()); - context.Console.ResetColor(); + if (!(context.IsCancelled && + exception is OperationCanceledException)) + { + context.Console.ResetColor(); + context.Console.ForegroundColor = ConsoleColor.Red; + context.Console.Error.Write("Unhandled exception: "); + context.Console.Error.WriteLine(exception.ToString()); + context.Console.ResetColor(); + } context.ResultCode = 1; } }, order: CommandLineBuilder.MiddlewareOrder.ExceptionHandler); diff --git a/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs b/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs index a4c525ceae..d19a312a05 100644 --- a/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs +++ b/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs @@ -32,13 +32,24 @@ public Task InvokeAsync(InvocationContext context) object value = null; - if (_delegate != null) + try { - value = _delegate.DynamicInvoke(arguments); + if (_delegate != null) + { + value = _delegate.DynamicInvoke(arguments); + } + else + { + value = _method.Invoke(_target, arguments); + } } - else + catch (TargetInvocationException te) { - value = _method.Invoke(_target, arguments); + if (te.InnerException != null) + { + throw te.InnerException; + } + throw; } return CommandHandler.GetResultCodeAsync(value); diff --git a/src/System.CommandLine/System.CommandLine.csproj b/src/System.CommandLine/System.CommandLine.csproj index 66895b341c..6d632eb88c 100644 --- a/src/System.CommandLine/System.CommandLine.csproj +++ b/src/System.CommandLine/System.CommandLine.csproj @@ -13,7 +13,6 @@ - From 1fcab8b1e6d0242c6c6cddd646cfee908200e7de Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 15 Nov 2018 09:35:21 +0100 Subject: [PATCH 10/42] Unwrap InvokeAsync TargetInvocationException --- .../Invocation/MethodBindingCommandHandler.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs b/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs index b615185166..b4702d5db2 100644 --- a/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs +++ b/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs @@ -27,7 +27,18 @@ public MethodBindingCommandHandler(MethodBinderBase methodBinder) public Task InvokeAsync(InvocationContext context) { - return _methodBinder.InvokeAsync(context); + try + { + return _methodBinder.InvokeAsync(context); + } + catch (TargetInvocationException te) + { + if (te.InnerException != null) + { + throw te.InnerException; + } + throw; + } } } } From 256fb39f5704427a35d27cc4e47d1ce8c6d87574 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 15 Nov 2018 09:35:47 +0100 Subject: [PATCH 11/42] Bind CancellationToken --- src/System.CommandLine/Invocation/MethodBinderBase.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/System.CommandLine/Invocation/MethodBinderBase.cs b/src/System.CommandLine/Invocation/MethodBinderBase.cs index 68b2a39918..2273b2050b 100644 --- a/src/System.CommandLine/Invocation/MethodBinderBase.cs +++ b/src/System.CommandLine/Invocation/MethodBinderBase.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; +using System.Threading; using System.Threading.Tasks; namespace System.CommandLine.Invocation @@ -76,6 +77,11 @@ public static object[] BindMethodArguments( { arguments.Add(context.Console); } + else if (parameterInfo.ParameterType == typeof(CancellationToken)) + { + CancellationToken ct = context.EnableCancellation(); + arguments.Add(ct); + } else { var argument = context.ParseResult From 48a9af8bb577f392d42d2914c7fc905ed7b79146 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 15 Nov 2018 09:38:19 +0100 Subject: [PATCH 12/42] Undo changes to SystemConsole --- src/System.CommandLine/Invocation/SystemConsole.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.CommandLine/Invocation/SystemConsole.cs b/src/System.CommandLine/Invocation/SystemConsole.cs index d5770bb2f8..ecc421427f 100644 --- a/src/System.CommandLine/Invocation/SystemConsole.cs +++ b/src/System.CommandLine/Invocation/SystemConsole.cs @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -using System; using System.CommandLine.Rendering; using System.IO; From 952e314ed3d750dfedd707cf44aef2648c481e0b Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 15 Nov 2018 10:05:45 +0100 Subject: [PATCH 13/42] UseExceptionHandler: also print a message when the invocation gets cancelled --- .../UseExceptionHandlerTests.cs | 2 +- .../Invocation/InvocationExtensions.cs | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs index 95f95eca43..7b98080504 100644 --- a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs +++ b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs @@ -86,7 +86,7 @@ public async Task UseExceptionHandler_doesnt_write_operationcancelledexception_d await parser.InvokeAsync("the-command", _console); - _console.Error.ToString().Should().Equals(""); + _console.Error.ToString().Should().Contain("The operation was cancelled."); } [Fact] diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index 887c0f65e9..6ea58248b4 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -113,15 +113,19 @@ public static CommandLineBuilder UseExceptionHandler( } catch (Exception exception) { - if (!(context.IsCancelled && - exception is OperationCanceledException)) + context.Console.ResetColor(); + context.Console.ForegroundColor = ConsoleColor.Red; + if (context.IsCancelled && + exception is OperationCanceledException) + { + context.Console.Error.WriteLine("The operation was cancelled."); + } + else { - context.Console.ResetColor(); - context.Console.ForegroundColor = ConsoleColor.Red; context.Console.Error.Write("Unhandled exception: "); context.Console.Error.WriteLine(exception.ToString()); - context.Console.ResetColor(); } + context.Console.ResetColor(); context.ResultCode = 1; } }, order: CommandLineBuilder.MiddlewareOrder.ExceptionHandler); From d903c2f23bb323f774c48f58ea6a639af4e69e48 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 15 Nov 2018 11:13:49 +0100 Subject: [PATCH 14/42] Undo special handling of OperationCanceledException in UseExceptionHandler. The user should catch this exception. --- .../UseExceptionHandlerTests.cs | 15 --------------- .../Invocation/InvocationExtensions.cs | 12 ++---------- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs index 7b98080504..f5a210a189 100644 --- a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs +++ b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs @@ -74,21 +74,6 @@ public async Task UseExceptionHandler_catches_command_handler_exceptions_and_wri _console.Error.ToString().Should().Contain("System.Exception: oops!"); } - [Fact] - public async Task UseExceptionHandler_doesnt_write_operationcancelledexception_details_when_context_is_cancelled() - { - var parser = new CommandLineBuilder() - .UseMiddleware((InvocationContext context) => context.Cancel()) - .AddCommand("the-command", "", - cmd => cmd.OnExecute((CancellationToken ct) => ct.ThrowIfCancellationRequested())) - .UseExceptionHandler() - .Build(); - - await parser.InvokeAsync("the-command", _console); - - _console.Error.ToString().Should().Contain("The operation was cancelled."); - } - [Fact] public async Task Declaration_of_UseExceptionHandler_can_come_before_other_middleware() { diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index 6ea58248b4..91f3c75460 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -115,16 +115,8 @@ public static CommandLineBuilder UseExceptionHandler( { context.Console.ResetColor(); context.Console.ForegroundColor = ConsoleColor.Red; - if (context.IsCancelled && - exception is OperationCanceledException) - { - context.Console.Error.WriteLine("The operation was cancelled."); - } - else - { - context.Console.Error.Write("Unhandled exception: "); - context.Console.Error.WriteLine(exception.ToString()); - } + context.Console.Error.Write("Unhandled exception: "); + context.Console.Error.WriteLine(exception.ToString()); context.Console.ResetColor(); context.ResultCode = 1; } From e5e8c322a4b812691c9097a5a03982bfb84bbdb9 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 16 Nov 2018 11:57:39 +0100 Subject: [PATCH 15/42] Add Linux test for UseConsoleLifetime --- .../CollectionHelpers.cs | 18 ++ .../LinuxOnlyTheory.cs | 20 ++ .../RemoteExecution.cs | 74 +++++++ .../RemoteExecutor.cs | 201 ++++++++++++++++++ .../System.CommandLine.Tests.csproj | 3 +- .../UseConsoleLifetimeTests.cs | 76 +++++++ .../Invocation/CommandHandler.cs | 107 +++++++++- 7 files changed, 489 insertions(+), 10 deletions(-) create mode 100644 src/System.CommandLine.Tests/CollectionHelpers.cs create mode 100644 src/System.CommandLine.Tests/LinuxOnlyTheory.cs create mode 100644 src/System.CommandLine.Tests/RemoteExecution.cs create mode 100644 src/System.CommandLine.Tests/RemoteExecutor.cs create mode 100644 src/System.CommandLine.Tests/UseConsoleLifetimeTests.cs diff --git a/src/System.CommandLine.Tests/CollectionHelpers.cs b/src/System.CommandLine.Tests/CollectionHelpers.cs new file mode 100644 index 0000000000..5f0c1e69a9 --- /dev/null +++ b/src/System.CommandLine.Tests/CollectionHelpers.cs @@ -0,0 +1,18 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Generic; + +namespace System.CommandLine.Tests +{ + public static class CollectionHelpers + { + public static void AddRange(this ICollection destination, IEnumerable source) + { + foreach (T item in source) + { + destination.Add(item); + } + } + } +} \ No newline at end of file diff --git a/src/System.CommandLine.Tests/LinuxOnlyTheory.cs b/src/System.CommandLine.Tests/LinuxOnlyTheory.cs new file mode 100644 index 0000000000..79d28a87c4 --- /dev/null +++ b/src/System.CommandLine.Tests/LinuxOnlyTheory.cs @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Runtime.InteropServices; +using Microsoft.DotNet.PlatformAbstractions; +using Xunit; + +namespace System.CommandLine.Tests +{ + public class LinuxOnlyTheory : TheoryAttribute + { + public LinuxOnlyTheory() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + this.Skip = "This test requires Linux to run"; + } + } + } +} \ No newline at end of file diff --git a/src/System.CommandLine.Tests/RemoteExecution.cs b/src/System.CommandLine.Tests/RemoteExecution.cs new file mode 100644 index 0000000000..1f3c168dec --- /dev/null +++ b/src/System.CommandLine.Tests/RemoteExecution.cs @@ -0,0 +1,74 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Diagnostics; +using System.IO; +using Xunit; +using Xunit.Sdk; + +namespace System.CommandLine.Tests +{ + public class RemoteExecution : IDisposable + { + private const int FailWaitTimeoutMilliseconds = 60 * 1000; + private string _exceptionFile; + + public RemoteExecution(Process process, string className, string methodName, string exceptionFile) + { + Process = process; + ClassName = className; + MethodName = methodName; + _exceptionFile = exceptionFile; + } + + public Process Process { get; private set; } + public string ClassName { get; private set; } + public string MethodName { get; private set; } + + public void Dispose() + { + GC.SuppressFinalize(this); // before Dispose(true) in case the Dispose call throws + Dispose(disposing: true); + } + + private void Dispose(bool disposing) + { + Assert.True(disposing, $"A test {ClassName}.{MethodName} forgot to Dispose() the result of RemoteInvoke()"); + + if (Process != null) + { + Assert.True(Process.WaitForExit(FailWaitTimeoutMilliseconds), + $"Timed out after {FailWaitTimeoutMilliseconds}ms waiting for remote process {Process.Id}"); + + // A bit unorthodox to do throwing operations in a Dispose, but by doing it here we avoid + // needing to do this in every derived test and keep each test much simpler. + try + { + if (File.Exists(_exceptionFile)) + { + throw new RemoteExecutionException(File.ReadAllText(_exceptionFile)); + } + } + finally + { + if (File.Exists(_exceptionFile)) + { + File.Delete(_exceptionFile); + } + + // Cleanup + try { Process.Kill(); } + catch { } // ignore all cleanup errors + } + + Process.Dispose(); + Process = null; + } + } + + private sealed class RemoteExecutionException : XunitException + { + internal RemoteExecutionException(string stackTrace) : base("Remote process failed with an unhandled exception.", stackTrace) { } + } + } +} \ No newline at end of file diff --git a/src/System.CommandLine.Tests/RemoteExecutor.cs b/src/System.CommandLine.Tests/RemoteExecutor.cs new file mode 100644 index 0000000000..45c2e161e2 --- /dev/null +++ b/src/System.CommandLine.Tests/RemoteExecutor.cs @@ -0,0 +1,201 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Runtime.InteropServices; +using System.Text; +using System.Threading.Tasks; + +namespace System.CommandLine.Tests +{ + public class RemoteExecutor + { + public static int Main(string[] args) + { + if (args.Length < 3) + { + Console.Error.WriteLine("This is not the program you are looking for. Run 'dotnet test' instead."); + return -1; + } + + string typeName = args[0]; + string methodName = args[1]; + string exceptionFile = args[2]; + string[] methodArguments = args.Skip(3).ToArray(); + + Type type = null; + MethodInfo methodInfo = null; + object instance = null; + int exitCode = 0; + try + { + type = typeof(RemoteExecutor).Assembly.GetType(typeName); + methodInfo = type.GetTypeInfo().GetDeclaredMethod(methodName); + instance = null; + + if (!methodInfo.IsStatic) + { + instance = Activator.CreateInstance(type); + } + + object result = methodInfo.Invoke(instance, new object[] { methodArguments }); + if (result is Task task) + { + exitCode = task.GetAwaiter().GetResult(); + } + else if (result is int exit) + { + exitCode = exit; + } + } + catch (Exception exc) + { + if (exc is TargetInvocationException && exc.InnerException != null) + exc = exc.InnerException; + + var output = new StringBuilder(); + output.AppendLine(); + output.AppendLine("Child exception:"); + output.AppendLine(" " + exc); + output.AppendLine(); + output.AppendLine("Child process:"); + output.AppendLine(string.Format(" {0} {1}", type, methodInfo)); + output.AppendLine(); + + if (methodArguments.Length > 0) + { + output.AppendLine("Child arguments:"); + output.AppendLine(" " + string.Join(", ", methodArguments)); + } + + File.WriteAllText(exceptionFile, output.ToString()); + } + finally + { + (instance as IDisposable)?.Dispose(); + } + + return exitCode; + } + + public static RemoteExecution Execute(Func mainMethod, string[] args = null, ProcessStartInfo psi = null) + => Execute(mainMethod.GetMethodInfo(), args, psi); + + public static RemoteExecution Execute(Func> mainMethod, string[] args = null, ProcessStartInfo psi = null) + => Execute(mainMethod.GetMethodInfo(), args, psi); + + private static RemoteExecution Execute(MethodInfo methodInfo, string[] args, ProcessStartInfo psi) + { + Type declaringType = methodInfo.DeclaringType; + string className = declaringType.FullName; + string methodName = methodInfo.Name; + string exceptionFile = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + + string dotnetExecutable = Process.GetCurrentProcess().MainModule.FileName; + string thisAssembly = typeof(RemoteExecutor).Assembly.Location; + string entryAssemblyWithoutExtension = Path.Combine(Path.GetDirectoryName(Assembly.GetEntryAssembly().Location), + Path.GetFileNameWithoutExtension(Assembly.GetEntryAssembly().Location)); + string runtimeConfig = GetApplicationArgument("--runtimeconfig"); + if (runtimeConfig == null) + { + runtimeConfig = entryAssemblyWithoutExtension + ".runtimeconfig.json"; + } + string depsFile = GetApplicationArgument("--depsfile"); + if (depsFile == null) + { + depsFile = entryAssemblyWithoutExtension + ".deps.json"; + } + + if (psi == null) + { + psi = new ProcessStartInfo(); + } + psi.FileName = dotnetExecutable; + psi.ArgumentList.AddRange(new[] { "exec", "--runtimeconfig", runtimeConfig, "--depsfile", depsFile, thisAssembly, + className, methodName, exceptionFile }); + if (args != null) + { + psi.ArgumentList.AddRange(args); + } + + Process process = Process.Start(psi); + + return new RemoteExecution(process, className, methodName, exceptionFile); + } + + private static string GetApplicationArgument(string name) + { + string[] arguments = GetApplicationArguments(); + for (int i = 0; i < arguments.Length - 1; i++) + { + if (arguments[i] == name) + { + return arguments[i + 1]; + } + } + return null; + } + + private static string[] s_arguments; + + private static string[] GetApplicationArguments() + { + // Environment.GetCommandLineArgs doesn't include arguments passed to the runtime. + // We use a native API to get all arguments. + + if (s_arguments != null) + { + return s_arguments; + } + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + s_arguments = File.ReadAllText($"/proc/{Process.GetCurrentProcess().Id}/cmdline").Split(new[] { '\0' }); + } + else if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + System.IntPtr ptr = GetCommandLine(); + string commandLine = Marshal.PtrToStringAuto(ptr); + s_arguments = CommandLineToArgs(commandLine); + } + else + { + throw new PlatformNotSupportedException($"{nameof(GetApplicationArguments)} is not supported on this platform."); + } + + return s_arguments; + } + + [DllImport("kernel32.dll", CharSet = CharSet.Auto)] + private static extern System.IntPtr GetCommandLine(); + + [DllImport("shell32.dll", SetLastError = true)] + private static extern IntPtr CommandLineToArgvW([MarshalAs(UnmanagedType.LPWStr)] string lpCmdLine, out int pNumArgs); + + public static string[] CommandLineToArgs(string commandLine) + { + int argc; + var argv = CommandLineToArgvW(commandLine, out argc); + if (argv == IntPtr.Zero) + throw new System.ComponentModel.Win32Exception(); + try + { + var args = new string[argc]; + for (var i = 0; i < args.Length; i++) + { + var p = Marshal.ReadIntPtr(argv, i * IntPtr.Size); + args[i] = Marshal.PtrToStringUni(p); + } + + return args; + } + finally + { + Marshal.FreeHGlobal(argv); + } + } + } +} \ No newline at end of file diff --git a/src/System.CommandLine.Tests/System.CommandLine.Tests.csproj b/src/System.CommandLine.Tests/System.CommandLine.Tests.csproj index c05f76491c..338735989b 100644 --- a/src/System.CommandLine.Tests/System.CommandLine.Tests.csproj +++ b/src/System.CommandLine.Tests/System.CommandLine.Tests.csproj @@ -1,7 +1,8 @@ - netcoreapp2.0 + netcoreapp2.1 latest + false diff --git a/src/System.CommandLine.Tests/UseConsoleLifetimeTests.cs b/src/System.CommandLine.Tests/UseConsoleLifetimeTests.cs new file mode 100644 index 0000000000..baeb2d9785 --- /dev/null +++ b/src/System.CommandLine.Tests/UseConsoleLifetimeTests.cs @@ -0,0 +1,76 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.CommandLine.Builder; +using System.CommandLine.Invocation; +using System.Diagnostics; +using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace System.CommandLine.Tests +{ + public class UseConsoleLifetimeTests + { + private const int SIGINT = 2; + private const int SIGTERM = 15; + + [LinuxOnlyTheory] + [InlineData(SIGINT)] // Console.CancelKeyPress + [InlineData(SIGTERM)] // AppDomain.CurrentDomain.ProcessExit + public async Task Foo(int signo) + { + const string ChildProcessWaiting = "Waiting for the command to be cancelled"; + const string ChildProcessCancelling = "Gracefully handling cancellation"; + const int ExpectedExitCode = 42; + + Func> childProgram = (string[] args) => + new CommandLineBuilder() + .AddCommand(new Command("the-command", + handler: CommandHandler.Create(async ct => + { + const int FailTimeoutMilliseconds = 10000; + + try + { + Console.WriteLine(ChildProcessWaiting); + await Task.Delay(FailTimeoutMilliseconds, ct); + } + catch (OperationCanceledException) + { + // Sleep a little to ensure we are really blocking the ProcessExit event. + Thread.Sleep(500); + Console.WriteLine(ChildProcessCancelling); + + return ExpectedExitCode; + } + + Assert.True(false, "The operation was not cancelled."); + return 1; + }))) + .UseConsoleLifetime() + .Build() + .InvokeAsync("the-command"); + + using (RemoteExecution program = RemoteExecutor.Execute(childProgram, psi: new ProcessStartInfo { RedirectStandardOutput = true })) + { + System.Diagnostics.Process process = program.Process; + + string childState = await process.StandardOutput.ReadLineAsync(); + Assert.Equal(ChildProcessWaiting, childState); + + Assert.Equal(0, kill(process.Id, signo)); + + childState = await process.StandardOutput.ReadLineAsync(); + Assert.Equal(ChildProcessCancelling, childState); + + process.WaitForExit(); + Assert.Equal(ExpectedExitCode, process.ExitCode); + } + } + + [DllImport("libc", SetLastError = true)] + private static extern int kill(int pid, int sig); + } +} \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/CommandHandler.cs b/src/System.CommandLine/Invocation/CommandHandler.cs index dc89013fd4..3f75a9fd2a 100644 --- a/src/System.CommandLine/Invocation/CommandHandler.cs +++ b/src/System.CommandLine/Invocation/CommandHandler.cs @@ -10,14 +10,72 @@ public static class CommandHandler { public static ICommandHandler Create( MethodInfo method, - object target = null) => + object target = null) => new MethodBindingCommandHandler(method, target); - public static ICommandHandler Create(Action action) => + public static ICommandHandler Create(Action action) => new MethodBindingCommandHandler(action); public static ICommandHandler Create( - Action action) => + Action action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Action action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Action action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Action action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Action action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Action action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Action action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create(Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create(Func action) => new MethodBindingCommandHandler(action); public static ICommandHandler Create( @@ -25,27 +83,58 @@ public static ICommandHandler Create( new MethodBindingCommandHandler(action); public static ICommandHandler Create( - Action action) => + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create(Func> action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func> action) => + new MethodBindingCommandHandler(action); + + public static ICommandHandler Create( + Func> action) => new MethodBindingCommandHandler(action); public static ICommandHandler Create( - Action action) => + Func> action) => new MethodBindingCommandHandler(action); public static ICommandHandler Create( - Action action) => + Func> action) => new MethodBindingCommandHandler(action); public static ICommandHandler Create( - Action action) => + Func> action) => new MethodBindingCommandHandler(action); public static ICommandHandler Create( - Action action) => + Func> action) => new MethodBindingCommandHandler(action); public static ICommandHandler Create( - Action action) => + Func> action) => new MethodBindingCommandHandler(action); internal static async Task GetResultCodeAsync(object value) From bdacb7201686dfcb02479a1fa50a631172276e16 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 16 Nov 2018 12:03:35 +0100 Subject: [PATCH 16/42] MethodBindingCommandHandler: restyle TargetInvocationException check --- .../Invocation/MethodBindingCommandHandler.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs b/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs index b4702d5db2..19e53fc8e4 100644 --- a/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs +++ b/src/System.CommandLine/Invocation/MethodBindingCommandHandler.cs @@ -31,13 +31,10 @@ public Task InvokeAsync(InvocationContext context) { return _methodBinder.InvokeAsync(context); } - catch (TargetInvocationException te) + // InvokeAsync wraps exceptions thrown by the method in a TargetInvocationException + catch (TargetInvocationException te) when (te.InnerException != null) { - if (te.InnerException != null) - { - throw te.InnerException; - } - throw; + throw te.InnerException; } } } From 17da00ae4d837dc43a35ceed99bb57ae950edff4 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 16 Nov 2018 12:04:18 +0100 Subject: [PATCH 17/42] UseExceptionHandlerTests: undo changes --- src/System.CommandLine.Tests/UseExceptionHandlerTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs index f5a210a189..643645bdb1 100644 --- a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs +++ b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs @@ -3,7 +3,6 @@ using System.CommandLine.Builder; using System.CommandLine.Invocation; -using System.Threading; using System.Threading.Tasks; using FluentAssertions; using Xunit; From 9600be2a17c6136afa7627a16663c076e0504ef4 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 16 Nov 2018 12:16:24 +0100 Subject: [PATCH 18/42] Add some comments to clarify how the termination event handlers work --- src/System.CommandLine/Invocation/InvocationExtensions.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index 91f3c75460..39b26f7024 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -23,6 +23,9 @@ public static CommandLineBuilder UseConsoleLifetime(this CommandLineBuilder buil { if (context.Cancel()) { + // Stop the process from terminating. + // Since the context was cancelled, the invocation should + // finish and Main will return. args.Cancel = true; } }; @@ -31,6 +34,9 @@ public static CommandLineBuilder UseConsoleLifetime(this CommandLineBuilder buil { if (context.Cancel()) { + // The process exits as soon as the event handler returns. + // We need to block until the invocation finishes and + // set the return value here (because Main will not return). blockProcessExit.Wait(); Environment.ExitCode = context.ResultCode; } From 95f53e39bb73198b8b07f0816eb5ecf0f930f958 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 16 Nov 2018 13:14:50 +0100 Subject: [PATCH 19/42] Use some new names --- .../InvocationPipelineTests.cs | 12 ++++----- .../Invocation/InvocationContext.cs | 27 ++++++++++--------- .../Invocation/InvocationExtensions.cs | 6 +++-- .../Invocation/MethodBinderBase.cs | 2 +- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/src/System.CommandLine.Tests/InvocationPipelineTests.cs b/src/System.CommandLine.Tests/InvocationPipelineTests.cs index ea4cbd7842..47cc920621 100644 --- a/src/System.CommandLine.Tests/InvocationPipelineTests.cs +++ b/src/System.CommandLine.Tests/InvocationPipelineTests.cs @@ -146,7 +146,7 @@ public async Task Invocation_can_be_cancelled_by_middleware_before_cancellation_ var command = new Command("the-command"); command.Handler = CommandHandler.Create((InvocationContext context) => { - CancellationToken ct = context.EnableCancellation(); + context.AddCancellationHandling(out CancellationToken ct); // The middleware canceled the request Assert.True(ct.IsCancellationRequested); }); @@ -154,9 +154,9 @@ public async Task Invocation_can_be_cancelled_by_middleware_before_cancellation_ var parser = new CommandLineBuilder() .UseMiddleware(async (context, next) => { - bool cancellationEnabled = context.Cancel(); + context.Cancel(out bool isCancelling); // Cancellation is not yet enabled - Assert.False(cancellationEnabled); + Assert.False(isCancelling); await next(context); }) .AddCommand(command) @@ -173,7 +173,7 @@ public async Task Invocation_can_be_cancelled_by_middleware_after_cancellation_e var command = new Command("the-command"); command.Handler = CommandHandler.Create(async (InvocationContext context) => { - CancellationToken ct = context.EnableCancellation(); + context.AddCancellationHandling(out CancellationToken ct); // The middleware hasn't cancelled our request yet. Assert.False(ct.IsCancellationRequested); @@ -185,9 +185,9 @@ public async Task Invocation_can_be_cancelled_by_middleware_after_cancellation_e { Task task = next(context); - bool cancellationEnabled = context.Cancel(); + context.Cancel(out bool isCancelling); // Cancellation is enabled by next - Assert.True(cancellationEnabled); + Assert.True(isCancelling); await task; }) diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index e764adfb7b..7552fd80c9 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -9,7 +9,7 @@ public sealed class InvocationContext : IDisposable { private readonly IDisposable _onDispose; private CancellationTokenSource _cts; - private bool _isCancellationEnabled; + private bool _isCancellationSupported; private readonly object _gate = new object(); public InvocationContext( @@ -41,27 +41,28 @@ public InvocationContext( public IInvocationResult InvocationResult { get; set; } - // Indicates the invocation can be cancelled. - // This returns a CancellationToken that will be set when the invocation - // is cancelled. The method may return a CancellationToken that is already - // cancelled. - public CancellationToken EnableCancellation() + /// + /// Indicates the invocation can be cancelled. + /// + /// token used by the caller to implement cancellation handling. + public void AddCancellationHandling(out CancellationToken cancellationToken) { lock (_gate) { - _isCancellationEnabled = true; + _isCancellationSupported = true; if (_cts == null) { _cts = new CancellationTokenSource(); } - return _cts.Token; + cancellationToken = _cts.Token; } } - // The return value indicates if the Invocation has cancellation enabled. - // When Cancel returns false, the Middleware may decide to forcefully - // end the process, for example, by calling Environment.Exit. - public bool Cancel() + /// + /// Cancels the invocation. + /// + /// returns whether the invocation is being cancelled. + public void Cancel(out bool isCancelling) { lock (_gate) { @@ -70,7 +71,7 @@ public bool Cancel() _cts = new CancellationTokenSource(); } _cts.Cancel(); - return _isCancellationEnabled; + isCancelling = _isCancellationSupported; } } diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index 39b26f7024..160e51dcd3 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -21,7 +21,8 @@ public static CommandLineBuilder UseConsoleLifetime(this CommandLineBuilder buil { ConsoleCancelEventHandler consoleHandler = (_, args) => { - if (context.Cancel()) + context.Cancel(out bool isCancelling); + if (isCancelling) { // Stop the process from terminating. // Since the context was cancelled, the invocation should @@ -32,7 +33,8 @@ public static CommandLineBuilder UseConsoleLifetime(this CommandLineBuilder buil var blockProcessExit = new ManualResetEventSlim(initialState: false); EventHandler processExitHandler = (_1, _2) => { - if (context.Cancel()) + context.Cancel(out bool isCancelling); + if (isCancelling) { // The process exits as soon as the event handler returns. // We need to block until the invocation finishes and diff --git a/src/System.CommandLine/Invocation/MethodBinderBase.cs b/src/System.CommandLine/Invocation/MethodBinderBase.cs index 2273b2050b..7e7abef412 100644 --- a/src/System.CommandLine/Invocation/MethodBinderBase.cs +++ b/src/System.CommandLine/Invocation/MethodBinderBase.cs @@ -79,7 +79,7 @@ public static object[] BindMethodArguments( } else if (parameterInfo.ParameterType == typeof(CancellationToken)) { - CancellationToken ct = context.EnableCancellation(); + context.AddCancellationHandling(out CancellationToken ct); arguments.Add(ct); } else From 0ae088162da976d7375ec4834564658ede66070c Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 16 Nov 2018 13:50:29 +0100 Subject: [PATCH 20/42] Fix ExitCode when UseConsoleLifetime with non cancellable invocation --- .../Invocation/InvocationExtensions.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index 160e51dcd3..36b0c821a6 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -33,15 +33,21 @@ public static CommandLineBuilder UseConsoleLifetime(this CommandLineBuilder buil var blockProcessExit = new ManualResetEventSlim(initialState: false); EventHandler processExitHandler = (_1, _2) => { + // The process exits as soon as the event handler returns. + // We provide a return value using Environment.ExitCode + // because Main will not finish executing. context.Cancel(out bool isCancelling); if (isCancelling) { - // The process exits as soon as the event handler returns. - // We need to block until the invocation finishes and - // set the return value here (because Main will not return). + // Wait for the invocation to finish. blockProcessExit.Wait(); + Environment.ExitCode = context.ResultCode; } + else + { + Environment.ExitCode = 1; + } }; try { From 85d5c07ae797bfb9e1db3dc2b152dbebba8167be Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 16 Nov 2018 13:52:04 +0100 Subject: [PATCH 21/42] More renaming --- ...oleLifetimeTests.cs => CancelOnProcessTerminationTests.cs} | 4 ++-- src/System.CommandLine/Invocation/InvocationExtensions.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename src/System.CommandLine.Tests/{UseConsoleLifetimeTests.cs => CancelOnProcessTerminationTests.cs} (96%) diff --git a/src/System.CommandLine.Tests/UseConsoleLifetimeTests.cs b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs similarity index 96% rename from src/System.CommandLine.Tests/UseConsoleLifetimeTests.cs rename to src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs index baeb2d9785..77e2059d44 100644 --- a/src/System.CommandLine.Tests/UseConsoleLifetimeTests.cs +++ b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs @@ -11,7 +11,7 @@ namespace System.CommandLine.Tests { - public class UseConsoleLifetimeTests + public class CancelOnProcessTerminationTests { private const int SIGINT = 2; private const int SIGTERM = 15; @@ -49,7 +49,7 @@ public async Task Foo(int signo) Assert.True(false, "The operation was not cancelled."); return 1; }))) - .UseConsoleLifetime() + .CancelOnProcessTermination() .Build() .InvokeAsync("the-command"); diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index 36b0c821a6..18df2290cc 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -15,7 +15,7 @@ namespace System.CommandLine.Invocation { public static class InvocationExtensions { - public static CommandLineBuilder UseConsoleLifetime(this CommandLineBuilder builder) + public static CommandLineBuilder CancelOnProcessTermination(this CommandLineBuilder builder) { builder.AddMiddleware(async (context, next) => { From 3a078fab40352cc6cb87468740e7b63b206e6958 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 16 Nov 2018 14:02:42 +0100 Subject: [PATCH 22/42] Add another test --- .../CancelOnProcessTerminationTests.cs | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs index 77e2059d44..5df02ffc6a 100644 --- a/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs +++ b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs @@ -19,7 +19,7 @@ public class CancelOnProcessTerminationTests [LinuxOnlyTheory] [InlineData(SIGINT)] // Console.CancelKeyPress [InlineData(SIGTERM)] // AppDomain.CurrentDomain.ProcessExit - public async Task Foo(int signo) + public async Task CancelOnProcessTermination_cancels_on_process_termination(int signo) { const string ChildProcessWaiting = "Waiting for the command to be cancelled"; const string ChildProcessCancelling = "Gracefully handling cancellation"; @@ -70,6 +70,44 @@ public async Task Foo(int signo) } } + [LinuxOnlyTheory] + [InlineData(SIGINT)] // Console.CancelKeyPress + [InlineData(SIGTERM)] // AppDomain.CurrentDomain.ProcessExit + public async Task CancelOnProcessTermination_non_cancellable_invocation_doesnt_block_termination_and_returns_non_zero_exit_code(int signo) + { + const string ChildProcessSleeping = "Sleeping"; + + Func> childProgram = (string[] args) => + new CommandLineBuilder() + .AddCommand(new Command("the-command", + handler: CommandHandler.Create(() => + { + const int FailTimeoutMilliseconds = 10000; + + Console.WriteLine(ChildProcessSleeping); + Thread.Sleep(FailTimeoutMilliseconds); + + Assert.True(false, "The operation was not cancelled."); + return 1; + }))) + .CancelOnProcessTermination() + .Build() + .InvokeAsync("the-command"); + + using (RemoteExecution program = RemoteExecutor.Execute(childProgram, psi: new ProcessStartInfo { RedirectStandardOutput = true })) + { + System.Diagnostics.Process process = program.Process; + + string childState = await process.StandardOutput.ReadLineAsync(); + Assert.Equal(ChildProcessSleeping, childState); + + Assert.Equal(0, kill(process.Id, signo)); + + process.WaitForExit(); + Assert.NotEqual(0, process.ExitCode); + } + } + [DllImport("libc", SetLastError = true)] private static extern int kill(int pid, int sig); } From 2d28edf3cd37f2e6de20a6b8f43d4e2a45d49adb Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 19 Nov 2018 16:53:00 +0100 Subject: [PATCH 23/42] DragonFruit: change TargetFramework to netcoreapp2.1 to fix CI build --- samples/DragonFruit/DragonFruit.csproj | 2 +- .../System.CommandLine.DragonFruit.Tests.csproj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/DragonFruit/DragonFruit.csproj b/samples/DragonFruit/DragonFruit.csproj index 7316f58b8d..81e821ecdd 100644 --- a/samples/DragonFruit/DragonFruit.csproj +++ b/samples/DragonFruit/DragonFruit.csproj @@ -2,7 +2,7 @@ Exe - netcoreapp2.0 + netcoreapp2.1 true diff --git a/src/System.CommandLine.DragonFruit.Tests/System.CommandLine.DragonFruit.Tests.csproj b/src/System.CommandLine.DragonFruit.Tests/System.CommandLine.DragonFruit.Tests.csproj index 53d9c178a8..4ead3f8df8 100644 --- a/src/System.CommandLine.DragonFruit.Tests/System.CommandLine.DragonFruit.Tests.csproj +++ b/src/System.CommandLine.DragonFruit.Tests/System.CommandLine.DragonFruit.Tests.csproj @@ -1,6 +1,6 @@  - netcoreapp2.0 + netcoreapp2.1 AutoGeneratedProgram true From 7e23c0b57856ff0bc5d73403fb68d04d99b8dfab Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 20 Nov 2018 10:18:30 +0100 Subject: [PATCH 24/42] Update DotNetCliVersion to 2.1.500 --- build/Versions.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Versions.props b/build/Versions.props index 86e0f27d0d..759d1d0112 100644 --- a/build/Versions.props +++ b/build/Versions.props @@ -19,7 +19,7 @@ - 2.1.400 + 2.1.500 1.0.0-beta-62503-02 2.1.4 From 0ae2773e33ccf65df334c0d4134d760291ae88a7 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 20 Nov 2018 16:58:34 +0100 Subject: [PATCH 25/42] Update global.json sdk version to 2.1.500 --- global.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/global.json b/global.json index 71273c22ff..b62f6cc57f 100644 --- a/global.json +++ b/global.json @@ -1,5 +1,5 @@ { "sdk": { - "version": "2.1.400" + "version": "2.1.500" } } From f6bed06f3ebf73855f779a88457088fa183f5f8b Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 20 Nov 2018 17:13:47 +0100 Subject: [PATCH 26/42] InvocationExtensionsTest: help the compiler pick a Create overload --- src/System.CommandLine.Tests/InvocationExtensionsTests.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/System.CommandLine.Tests/InvocationExtensionsTests.cs b/src/System.CommandLine.Tests/InvocationExtensionsTests.cs index 724384a7f8..1e4b3d2c02 100644 --- a/src/System.CommandLine.Tests/InvocationExtensionsTests.cs +++ b/src/System.CommandLine.Tests/InvocationExtensionsTests.cs @@ -51,6 +51,11 @@ public async Task RootCommand_InvokeAsync_returns_1_when_handler_throws() { wasCalled = true; throw new Exception("oops!"); + + // Help the compiler pick a CommandHandler.Create overload. +#pragma warning disable CS0162 // Unreachable code detected + return 0; +#pragma warning restore CS0162 }); var resultCode = await rootCommand.InvokeAsync(""); From 03a89de4dc9b48bef0bb246d0c7b3ed44bfeca8b Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 20 Nov 2018 17:14:17 +0100 Subject: [PATCH 27/42] AddCancellationHandling: return instead of using an out parameter --- src/System.CommandLine.Tests/InvocationPipelineTests.cs | 4 ++-- src/System.CommandLine/Invocation/InvocationContext.cs | 6 +++--- src/System.CommandLine/Invocation/MethodBinderBase.cs | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/System.CommandLine.Tests/InvocationPipelineTests.cs b/src/System.CommandLine.Tests/InvocationPipelineTests.cs index 47cc920621..c318116f2d 100644 --- a/src/System.CommandLine.Tests/InvocationPipelineTests.cs +++ b/src/System.CommandLine.Tests/InvocationPipelineTests.cs @@ -146,7 +146,7 @@ public async Task Invocation_can_be_cancelled_by_middleware_before_cancellation_ var command = new Command("the-command"); command.Handler = CommandHandler.Create((InvocationContext context) => { - context.AddCancellationHandling(out CancellationToken ct); + CancellationToken ct = context.AddCancellationHandling(); // The middleware canceled the request Assert.True(ct.IsCancellationRequested); }); @@ -173,7 +173,7 @@ public async Task Invocation_can_be_cancelled_by_middleware_after_cancellation_e var command = new Command("the-command"); command.Handler = CommandHandler.Create(async (InvocationContext context) => { - context.AddCancellationHandling(out CancellationToken ct); + CancellationToken ct = context.AddCancellationHandling(); // The middleware hasn't cancelled our request yet. Assert.False(ct.IsCancellationRequested); diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index 7552fd80c9..bc0ab15891 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -44,8 +44,8 @@ public InvocationContext( /// /// Indicates the invocation can be cancelled. /// - /// token used by the caller to implement cancellation handling. - public void AddCancellationHandling(out CancellationToken cancellationToken) + /// Token used by the caller to implement cancellation handling. + public CancellationToken AddCancellationHandling() { lock (_gate) { @@ -54,7 +54,7 @@ public void AddCancellationHandling(out CancellationToken cancellationToken) { _cts = new CancellationTokenSource(); } - cancellationToken = _cts.Token; + return _cts.Token; } } diff --git a/src/System.CommandLine/Invocation/MethodBinderBase.cs b/src/System.CommandLine/Invocation/MethodBinderBase.cs index 04cf628ec0..1cbe34df46 100644 --- a/src/System.CommandLine/Invocation/MethodBinderBase.cs +++ b/src/System.CommandLine/Invocation/MethodBinderBase.cs @@ -79,7 +79,7 @@ public static object[] BindMethodArguments( } else if (parameterInfo.ParameterType == typeof(CancellationToken)) { - context.AddCancellationHandling(out CancellationToken ct); + CancellationToken ct = context.AddCancellationHandling(); arguments.Add(ct); } else From 1ea9e2f5599b6bdd51f615fd4d6b6759f4b45a5c Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 20 Nov 2018 17:18:45 +0100 Subject: [PATCH 28/42] Rename IsCancelled to IsCancellationRequested --- .../InvocationPipelineTests.cs | 21 ++++++++++++++----- .../Invocation/InvocationContext.cs | 2 +- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/System.CommandLine.Tests/InvocationPipelineTests.cs b/src/System.CommandLine.Tests/InvocationPipelineTests.cs index c318116f2d..6ebad4b29c 100644 --- a/src/System.CommandLine.Tests/InvocationPipelineTests.cs +++ b/src/System.CommandLine.Tests/InvocationPipelineTests.cs @@ -154,9 +154,15 @@ public async Task Invocation_can_be_cancelled_by_middleware_before_cancellation_ var parser = new CommandLineBuilder() .UseMiddleware(async (context, next) => { + Assert.False(context.IsCancellationRequested); + context.Cancel(out bool isCancelling); + // Cancellation is not yet enabled Assert.False(isCancelling); + + Assert.True(context.IsCancellationRequested); + await next(context); }) .AddCommand(command) @@ -183,13 +189,18 @@ public async Task Invocation_can_be_cancelled_by_middleware_after_cancellation_e var parser = new CommandLineBuilder() .UseMiddleware(async (context, next) => { - Task task = next(context); + Task task = next(context); - context.Cancel(out bool isCancelling); - // Cancellation is enabled by next - Assert.True(isCancelling); + Assert.False(context.IsCancellationRequested); + + context.Cancel(out bool isCancelling); + + // Cancellation is enabled by next + Assert.True(isCancelling); + + Assert.True(context.IsCancellationRequested); - await task; + await task; }) .AddCommand(command) .Build(); diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index bc0ab15891..3272b91318 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -75,7 +75,7 @@ public void Cancel(out bool isCancelling) } } - public bool IsCancelled => + public bool IsCancellationRequested => _cts?.Token.IsCancellationRequested == true; public void Dispose() From 0ec7e8b1e3f7d3cef8c7330d796223e02696f4bc Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 20 Nov 2018 17:30:45 +0100 Subject: [PATCH 29/42] Revert back to 2.1.400 sdk --- build/Versions.props | 2 +- global.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/Versions.props b/build/Versions.props index 5c6c77ec8d..556820f9a7 100644 --- a/build/Versions.props +++ b/build/Versions.props @@ -19,7 +19,7 @@ - 2.1.500 + 2.1.400 2.1.400 1.0.0-beta2-63127-01 1.0.0-beta-62503-02 diff --git a/global.json b/global.json index b62f6cc57f..71273c22ff 100644 --- a/global.json +++ b/global.json @@ -1,5 +1,5 @@ { "sdk": { - "version": "2.1.500" + "version": "2.1.400" } } From be385fa9ee9fc2c841b8e11a0ea656165bab08d0 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 20 Nov 2018 19:50:52 +0100 Subject: [PATCH 30/42] Remove duplicate DotNetCliVersion --- build/Versions.props | 1 - 1 file changed, 1 deletion(-) diff --git a/build/Versions.props b/build/Versions.props index 556820f9a7..d6d40a3581 100644 --- a/build/Versions.props +++ b/build/Versions.props @@ -19,7 +19,6 @@ - 2.1.400 2.1.400 1.0.0-beta2-63127-01 1.0.0-beta-62503-02 From fd362ac70309b41929dc6544a82607ddeeb5f657 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 21 Nov 2018 10:20:15 +0100 Subject: [PATCH 31/42] Add description for the wiki --- wiki_feature_description.md | 39 +++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 wiki_feature_description.md diff --git a/wiki_feature_description.md b/wiki_feature_description.md new file mode 100644 index 0000000000..aab1f863f6 --- /dev/null +++ b/wiki_feature_description.md @@ -0,0 +1,39 @@ +# General + +`System.CommandLine` supports performing actions when the invocation is being stopped due to process termination. For example, aborting an ongoing transaction, or flushing some data to disk. + +Process termination can be forcefull, which means the process is terminated by the OS and it doesn't get a chance to cleanup. This is _killing_ a process. + +Process termination can also be requested. For example, the user presses Control-C on an interactive application, or the system asks a service to terminate. + +# Implementing termination handling + +To add termination handling to a command, you must add a `CancellationToken` argument to the command handler. This token can be passed to async APIs. Cancellation actions can also be added directly using the `CancellationToken.Register` method. + +```c# +CommandHandler.Create(async (IConsole console, CancellationToken ct) => +{ + try + { + using (var httpClient = new HttpClient()) + { + await httpClient.GetAsync("http://www.example.com", ct); + } + return 0; + } + catch (OperationCanceledException) + { + console.Error.WriteLine("The operation was aborted"); + return 1; + } +}); +``` + +To trigger cancellation, the `CancelOnProcessTermination` middleware must be added to the `CommandLineBuilder`. + +```c# +var builder = new CommandLineBuilder() + . ... + .CancelOnProcessTermination() + . ... +``` \ No newline at end of file From 963c30d172a770b7adae0317d35d2daf73b3fda9 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 28 Nov 2018 11:09:59 +0100 Subject: [PATCH 32/42] Reword wiki feature description --- wiki_feature_description.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wiki_feature_description.md b/wiki_feature_description.md index aab1f863f6..6fd5dec7c4 100644 --- a/wiki_feature_description.md +++ b/wiki_feature_description.md @@ -29,7 +29,7 @@ CommandHandler.Create(async (IConsole console, CancellationToken ct) => }); ``` -To trigger cancellation, the `CancelOnProcessTermination` middleware must be added to the `CommandLineBuilder`. +To enabel cancellation, the `CancelOnProcessTermination` middleware must be added to the `CommandLineBuilder`. ```c# var builder = new CommandLineBuilder() From 87d57eca1d2bf772818e97dbf388bcc5dfb58014 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 28 Nov 2018 11:19:51 +0100 Subject: [PATCH 33/42] Move child process timeouts to the test process --- .../CancelOnProcessTerminationTests.cs | 38 +++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs index 5df02ffc6a..4344251507 100644 --- a/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs +++ b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs @@ -30,12 +30,10 @@ public async Task CancelOnProcessTermination_cancels_on_process_termination(int .AddCommand(new Command("the-command", handler: CommandHandler.Create(async ct => { - const int FailTimeoutMilliseconds = 10000; - try { Console.WriteLine(ChildProcessWaiting); - await Task.Delay(FailTimeoutMilliseconds, ct); + await Task.Delay(int.MaxValue, ct); } catch (OperationCanceledException) { @@ -46,7 +44,6 @@ public async Task CancelOnProcessTermination_cancels_on_process_termination(int return ExpectedExitCode; } - Assert.True(false, "The operation was not cancelled."); return 1; }))) .CancelOnProcessTermination() @@ -57,15 +54,27 @@ public async Task CancelOnProcessTermination_cancels_on_process_termination(int { System.Diagnostics.Process process = program.Process; + // Wait for the child to be in the command handler. string childState = await process.StandardOutput.ReadLineAsync(); Assert.Equal(ChildProcessWaiting, childState); + // Request termination Assert.Equal(0, kill(process.Id, signo)); + // Verify the CancellationToken is tripped childState = await process.StandardOutput.ReadLineAsync(); Assert.Equal(ChildProcessCancelling, childState); - process.WaitForExit(); + // Verify the process terminates timely + bool processExited = process.WaitForExit(10000); + if (!processExited) + { + process.Kill(); + process.WaitForExit(); + } + Assert.True(processExited); + + // Verify the process exit code Assert.Equal(ExpectedExitCode, process.ExitCode); } } @@ -82,13 +91,9 @@ public async Task CancelOnProcessTermination_non_cancellable_invocation_doesnt_b .AddCommand(new Command("the-command", handler: CommandHandler.Create(() => { - const int FailTimeoutMilliseconds = 10000; - Console.WriteLine(ChildProcessSleeping); - Thread.Sleep(FailTimeoutMilliseconds); - Assert.True(false, "The operation was not cancelled."); - return 1; + Thread.Sleep(int.MaxValue); }))) .CancelOnProcessTermination() .Build() @@ -98,12 +103,23 @@ public async Task CancelOnProcessTermination_non_cancellable_invocation_doesnt_b { System.Diagnostics.Process process = program.Process; + // Wait for the child to be in the command handler. string childState = await process.StandardOutput.ReadLineAsync(); Assert.Equal(ChildProcessSleeping, childState); + // Request termination Assert.Equal(0, kill(process.Id, signo)); - process.WaitForExit(); + // Verify the process terminates timely + bool processExited = process.WaitForExit(10000); + if (!processExited) + { + process.Kill(); + process.WaitForExit(); + } + Assert.True(processExited); + + // Verify the process exit code Assert.NotEqual(0, process.ExitCode); } } From c5fe41b6b3efcc5367fe2302a73ceb84b353c17b Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 28 Nov 2018 11:46:26 +0100 Subject: [PATCH 34/42] Simplify tests --- .../CancelOnProcessTerminationTests.cs | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs index 4344251507..21637cb97e 100644 --- a/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs +++ b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs @@ -22,8 +22,7 @@ public class CancelOnProcessTerminationTests public async Task CancelOnProcessTermination_cancels_on_process_termination(int signo) { const string ChildProcessWaiting = "Waiting for the command to be cancelled"; - const string ChildProcessCancelling = "Gracefully handling cancellation"; - const int ExpectedExitCode = 42; + const int CancelledExitCode = 42; Func> childProgram = (string[] args) => new CommandLineBuilder() @@ -37,11 +36,17 @@ public async Task CancelOnProcessTermination_cancels_on_process_termination(int } catch (OperationCanceledException) { - // Sleep a little to ensure we are really blocking the ProcessExit event. - Thread.Sleep(500); - Console.WriteLine(ChildProcessCancelling); - - return ExpectedExitCode; + // For Process.Exit handling the event must remain blocked as long as the + // command is executed. + // We are currently blocking that event because CancellationToken.Cancel + // is called from the event handler. + // We'll do an async Yield now. This means the Cancel call will return + // and we're no longer actively blocking the event. + // The event handler is responsible to continue blocking until the command + // has finished executing. If it doesn't we won't get the CancelledExitCode. + await Task.Yield(); + + return CancelledExitCode; } return 1; @@ -61,10 +66,6 @@ public async Task CancelOnProcessTermination_cancels_on_process_termination(int // Request termination Assert.Equal(0, kill(process.Id, signo)); - // Verify the CancellationToken is tripped - childState = await process.StandardOutput.ReadLineAsync(); - Assert.Equal(ChildProcessCancelling, childState); - // Verify the process terminates timely bool processExited = process.WaitForExit(10000); if (!processExited) @@ -75,7 +76,7 @@ public async Task CancelOnProcessTermination_cancels_on_process_termination(int Assert.True(processExited); // Verify the process exit code - Assert.Equal(ExpectedExitCode, process.ExitCode); + Assert.Equal(CancelledExitCode, process.ExitCode); } } @@ -84,14 +85,14 @@ public async Task CancelOnProcessTermination_cancels_on_process_termination(int [InlineData(SIGTERM)] // AppDomain.CurrentDomain.ProcessExit public async Task CancelOnProcessTermination_non_cancellable_invocation_doesnt_block_termination_and_returns_non_zero_exit_code(int signo) { - const string ChildProcessSleeping = "Sleeping"; + const string ChildProcessWaiting = "Waiting for the command to be terminated"; Func> childProgram = (string[] args) => new CommandLineBuilder() .AddCommand(new Command("the-command", handler: CommandHandler.Create(() => { - Console.WriteLine(ChildProcessSleeping); + Console.WriteLine(ChildProcessWaiting); Thread.Sleep(int.MaxValue); }))) @@ -105,7 +106,7 @@ public async Task CancelOnProcessTermination_non_cancellable_invocation_doesnt_b // Wait for the child to be in the command handler. string childState = await process.StandardOutput.ReadLineAsync(); - Assert.Equal(ChildProcessSleeping, childState); + Assert.Equal(ChildProcessWaiting, childState); // Request termination Assert.Equal(0, kill(process.Id, signo)); @@ -119,7 +120,7 @@ public async Task CancelOnProcessTermination_non_cancellable_invocation_doesnt_b } Assert.True(processExited); - // Verify the process exit code + // Verify non-zero exit code Assert.NotEqual(0, process.ExitCode); } } From 2757674d020726910cca483773bae3d790505df1 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 28 Nov 2018 14:26:17 +0100 Subject: [PATCH 35/42] Fix race between producers and consumers of cancellation --- .../CancelOnProcessTerminationTests.cs | 47 +------------ .../InvocationPipelineTests.cs | 69 ------------------- .../Invocation/InvocationContext.cs | 37 ++-------- .../Invocation/InvocationExtensions.cs | 50 +++++++------- 4 files changed, 34 insertions(+), 169 deletions(-) diff --git a/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs index 21637cb97e..eccb694295 100644 --- a/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs +++ b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs @@ -38,7 +38,7 @@ public async Task CancelOnProcessTermination_cancels_on_process_termination(int { // For Process.Exit handling the event must remain blocked as long as the // command is executed. - // We are currently blocking that event because CancellationToken.Cancel + // We are currently blocking that event because CancellationTokenSource.Cancel // is called from the event handler. // We'll do an async Yield now. This means the Cancel call will return // and we're no longer actively blocking the event. @@ -80,51 +80,6 @@ public async Task CancelOnProcessTermination_cancels_on_process_termination(int } } - [LinuxOnlyTheory] - [InlineData(SIGINT)] // Console.CancelKeyPress - [InlineData(SIGTERM)] // AppDomain.CurrentDomain.ProcessExit - public async Task CancelOnProcessTermination_non_cancellable_invocation_doesnt_block_termination_and_returns_non_zero_exit_code(int signo) - { - const string ChildProcessWaiting = "Waiting for the command to be terminated"; - - Func> childProgram = (string[] args) => - new CommandLineBuilder() - .AddCommand(new Command("the-command", - handler: CommandHandler.Create(() => - { - Console.WriteLine(ChildProcessWaiting); - - Thread.Sleep(int.MaxValue); - }))) - .CancelOnProcessTermination() - .Build() - .InvokeAsync("the-command"); - - using (RemoteExecution program = RemoteExecutor.Execute(childProgram, psi: new ProcessStartInfo { RedirectStandardOutput = true })) - { - System.Diagnostics.Process process = program.Process; - - // Wait for the child to be in the command handler. - string childState = await process.StandardOutput.ReadLineAsync(); - Assert.Equal(ChildProcessWaiting, childState); - - // Request termination - Assert.Equal(0, kill(process.Id, signo)); - - // Verify the process terminates timely - bool processExited = process.WaitForExit(10000); - if (!processExited) - { - process.Kill(); - process.WaitForExit(); - } - Assert.True(processExited); - - // Verify non-zero exit code - Assert.NotEqual(0, process.ExitCode); - } - } - [DllImport("libc", SetLastError = true)] private static extern int kill(int pid, int sig); } diff --git a/src/System.CommandLine.Tests/InvocationPipelineTests.cs b/src/System.CommandLine.Tests/InvocationPipelineTests.cs index 6ebad4b29c..32b903642b 100644 --- a/src/System.CommandLine.Tests/InvocationPipelineTests.cs +++ b/src/System.CommandLine.Tests/InvocationPipelineTests.cs @@ -139,74 +139,5 @@ public async Task Invocation_can_be_short_circuited_by_middleware_by_not_calling middlewareWasCalled.Should().BeTrue(); handlerWasCalled.Should().BeFalse(); } - - [Fact] - public async Task Invocation_can_be_cancelled_by_middleware_before_cancellation_enabled() - { - var command = new Command("the-command"); - command.Handler = CommandHandler.Create((InvocationContext context) => - { - CancellationToken ct = context.AddCancellationHandling(); - // The middleware canceled the request - Assert.True(ct.IsCancellationRequested); - }); - - var parser = new CommandLineBuilder() - .UseMiddleware(async (context, next) => - { - Assert.False(context.IsCancellationRequested); - - context.Cancel(out bool isCancelling); - - // Cancellation is not yet enabled - Assert.False(isCancelling); - - Assert.True(context.IsCancellationRequested); - - await next(context); - }) - .AddCommand(command) - .Build(); - - await parser.InvokeAsync("the-command", new TestConsole()); - } - - [Fact] - public async Task Invocation_can_be_cancelled_by_middleware_after_cancellation_enabled() - { - const int timeout = 5000; - - var command = new Command("the-command"); - command.Handler = CommandHandler.Create(async (InvocationContext context) => - { - CancellationToken ct = context.AddCancellationHandling(); - // The middleware hasn't cancelled our request yet. - Assert.False(ct.IsCancellationRequested); - - await Task.Delay(timeout, ct); - }); - - var parser = new CommandLineBuilder() - .UseMiddleware(async (context, next) => - { - Task task = next(context); - - Assert.False(context.IsCancellationRequested); - - context.Cancel(out bool isCancelling); - - // Cancellation is enabled by next - Assert.True(isCancelling); - - Assert.True(context.IsCancellationRequested); - - await task; - }) - .AddCommand(command) - .Build(); - - var invocation = parser.InvokeAsync("the-command", new TestConsole()); - await Assert.ThrowsAnyAsync(() => invocation); - } } } diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index 3272b91318..0e8bd9abac 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -9,8 +9,6 @@ public sealed class InvocationContext : IDisposable { private readonly IDisposable _onDispose; private CancellationTokenSource _cts; - private bool _isCancellationSupported; - private readonly object _gate = new object(); public InvocationContext( ParseResult parseResult, @@ -41,43 +39,22 @@ public InvocationContext( public IInvocationResult InvocationResult { get; set; } + internal event Action CancellationHandlingAdded; + /// /// Indicates the invocation can be cancelled. /// /// Token used by the caller to implement cancellation handling. - public CancellationToken AddCancellationHandling() + internal CancellationToken AddCancellationHandling() { - lock (_gate) + if (_cts == null) { - _isCancellationSupported = true; - if (_cts == null) - { - _cts = new CancellationTokenSource(); - } - return _cts.Token; + _cts = new CancellationTokenSource(); } + CancellationHandlingAdded?.Invoke(_cts); + return _cts.Token; } - /// - /// Cancels the invocation. - /// - /// returns whether the invocation is being cancelled. - public void Cancel(out bool isCancelling) - { - lock (_gate) - { - if (_cts == null) - { - _cts = new CancellationTokenSource(); - } - _cts.Cancel(); - isCancelling = _isCancellationSupported; - } - } - - public bool IsCancellationRequested => - _cts?.Token.IsCancellationRequested == true; - public void Dispose() { _onDispose?.Dispose(); diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs index 9df8f4f3c9..a82aa1cbcd 100644 --- a/src/System.CommandLine/Invocation/InvocationExtensions.cs +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -19,47 +19,49 @@ public static CommandLineBuilder CancelOnProcessTermination(this CommandLineBuil { builder.AddMiddleware(async (context, next) => { - ConsoleCancelEventHandler consoleHandler = (_, args) => + bool cancellationHandlingAdded = false; + ManualResetEventSlim blockProcessExit = null; + ConsoleCancelEventHandler consoleHandler = null; + EventHandler processExitHandler = null; + + context.CancellationHandlingAdded += (CancellationTokenSource cts) => { - context.Cancel(out bool isCancelling); - if (isCancelling) + cancellationHandlingAdded = true; + blockProcessExit = new ManualResetEventSlim(initialState: false); + consoleHandler = (_, args) => { + cts.Cancel(); // Stop the process from terminating. // Since the context was cancelled, the invocation should // finish and Main will return. args.Cancel = true; - } - }; - var blockProcessExit = new ManualResetEventSlim(initialState: false); - EventHandler processExitHandler = (_1, _2) => - { - // The process exits as soon as the event handler returns. - // We provide a return value using Environment.ExitCode - // because Main will not finish executing. - context.Cancel(out bool isCancelling); - if (isCancelling) + }; + processExitHandler = (_1, _2) => { + cts.Cancel(); + // The process exits as soon as the event handler returns. + // We provide a return value using Environment.ExitCode + // because Main will not finish executing. // Wait for the invocation to finish. blockProcessExit.Wait(); - Environment.ExitCode = context.ResultCode; - } - else - { - Environment.ExitCode = 1; - } + }; + Console.CancelKeyPress += consoleHandler; + AppDomain.CurrentDomain.ProcessExit += processExitHandler; }; + try { - Console.CancelKeyPress += consoleHandler; - AppDomain.CurrentDomain.ProcessExit += processExitHandler; await next(context); } finally { - Console.CancelKeyPress -= consoleHandler; - AppDomain.CurrentDomain.ProcessExit -= processExitHandler; - blockProcessExit.Set(); + if (cancellationHandlingAdded) + { + Console.CancelKeyPress -= consoleHandler; + AppDomain.CurrentDomain.ProcessExit -= processExitHandler; + blockProcessExit.Set(); + } } }, CommandLineBuilder.MiddlewareOrder.ProcessExit); From e0115d0c6dc95ddf3285b2f1ed28742a3a0266e6 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 29 Nov 2018 09:25:44 +0100 Subject: [PATCH 36/42] Tests: help compiler pick a CommandHandler.Create overload --- .../InvocationPipelineTests.cs | 9 ++++++++- .../UseExceptionHandlerTests.cs | 18 ++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine.Tests/InvocationPipelineTests.cs b/src/System.CommandLine.Tests/InvocationPipelineTests.cs index 1d05e2c68f..24948b7c9d 100644 --- a/src/System.CommandLine.Tests/InvocationPipelineTests.cs +++ b/src/System.CommandLine.Tests/InvocationPipelineTests.cs @@ -74,7 +74,14 @@ public void When_middleware_throws_then_InvokeAsync_does_not_handle_the_exceptio public void When_command_handler_throws_then_InvokeAsync_does_not_handle_the_exception() { var command = new Command("the-command"); - command.Handler = CommandHandler.Create(() => throw new Exception("oops!")); + command.Handler = CommandHandler.Create(() => + { + throw new Exception("oops!"); + // Help the compiler pick a CommandHandler.Create overload. +#pragma warning disable CS0162 // Unreachable code detected + return 0; +#pragma warning restore CS0162 + }); var parser = new CommandLineBuilder() .AddCommand(command) diff --git a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs index 16cf64001e..7b39041c92 100644 --- a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs +++ b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs @@ -49,7 +49,14 @@ public async Task UseExceptionHandler_catches_middleware_exceptions_and_writes_d public async Task UseExceptionHandler_catches_command_handler_exceptions_and_sets_result_code_to_1() { var command = new Command("the-command"); - command.Handler = CommandHandler.Create(() => throw new Exception("oops!")); + command.Handler = CommandHandler.Create(() => + { + throw new Exception("oops!"); + // Help the compiler pick a CommandHandler.Create overload. +#pragma warning disable CS0162 // Unreachable code detected + return 0; +#pragma warning restore CS0162 + }); var parser = new CommandLineBuilder() .AddCommand(command) @@ -65,7 +72,14 @@ public async Task UseExceptionHandler_catches_command_handler_exceptions_and_set public async Task UseExceptionHandler_catches_command_handler_exceptions_and_writes_details_to_standard_error() { var command = new Command("the-command"); - command.Handler = CommandHandler.Create(() => throw new Exception("oops!")); + command.Handler = CommandHandler.Create(() => + { + throw new Exception("oops!"); + // Help the compiler pick a CommandHandler.Create overload. +#pragma warning disable CS0162 // Unreachable code detected + return 0; +#pragma warning restore CS0162 + }); var parser = new CommandLineBuilder() .AddCommand(command) From 840f267ff45ad1b8f1cf544431186b3c6f638542 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 29 Nov 2018 09:38:57 +0100 Subject: [PATCH 37/42] InvocationContext: ensure order of CancellationHandlerAdded event and AddCancellationHandling method --- .../Invocation/InvocationContext.cs | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index 0e8bd9abac..c00ce68f42 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -9,6 +9,7 @@ public sealed class InvocationContext : IDisposable { private readonly IDisposable _onDispose; private CancellationTokenSource _cts; + private Action _cancellationHandlingAddedEvent; public InvocationContext( ParseResult parseResult, @@ -39,7 +40,18 @@ public InvocationContext( public IInvocationResult InvocationResult { get; set; } - internal event Action CancellationHandlingAdded; + internal event Action CancellationHandlingAdded + { + add + { + if (_cts != null) + { + throw new InvalidOperationException($"Handlers must be added before adding cancellation handling."); + } + _cancellationHandlingAddedEvent += value; + } + remove => _cancellationHandlingAddedEvent -= value; + } /// /// Indicates the invocation can be cancelled. @@ -47,11 +59,12 @@ public InvocationContext( /// Token used by the caller to implement cancellation handling. internal CancellationToken AddCancellationHandling() { - if (_cts == null) + if (_cts != null) { - _cts = new CancellationTokenSource(); + throw new InvalidOperationException("Cancellation handing was already added."); } - CancellationHandlingAdded?.Invoke(_cts); + _cts = new CancellationTokenSource(); + _cancellationHandlingAddedEvent?.Invoke(_cts); return _cts.Token; } From 610ca214d9b7c1eab3a4a4cfda1408f8f92e5593 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 29 Nov 2018 09:42:01 +0100 Subject: [PATCH 38/42] Use FluentAssertion style --- .../CancelOnProcessTerminationTests.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs index eccb694295..488fe6f255 100644 --- a/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs +++ b/src/System.CommandLine.Tests/CancelOnProcessTerminationTests.cs @@ -7,6 +7,7 @@ using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; +using FluentAssertions; using Xunit; namespace System.CommandLine.Tests @@ -61,10 +62,10 @@ public async Task CancelOnProcessTermination_cancels_on_process_termination(int // Wait for the child to be in the command handler. string childState = await process.StandardOutput.ReadLineAsync(); - Assert.Equal(ChildProcessWaiting, childState); + childState.Should().Be(ChildProcessWaiting); // Request termination - Assert.Equal(0, kill(process.Id, signo)); + kill(process.Id, signo).Should().Be(0); // Verify the process terminates timely bool processExited = process.WaitForExit(10000); @@ -73,10 +74,10 @@ public async Task CancelOnProcessTermination_cancels_on_process_termination(int process.Kill(); process.WaitForExit(); } - Assert.True(processExited); + processExited.Should().Be(true); // Verify the process exit code - Assert.Equal(CancelledExitCode, process.ExitCode); + process.ExitCode.Should().Be(CancelledExitCode); } } From 6aa5bd1f1d5d7e1937a7354fc9f148f0b925ba27 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 29 Nov 2018 11:31:58 +0100 Subject: [PATCH 39/42] Fix typo --- src/System.CommandLine/Invocation/InvocationContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index c00ce68f42..0e802d5a6b 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -61,7 +61,7 @@ internal CancellationToken AddCancellationHandling() { if (_cts != null) { - throw new InvalidOperationException("Cancellation handing was already added."); + throw new InvalidOperationException("Cancellation handling was already added."); } _cts = new CancellationTokenSource(); _cancellationHandlingAddedEvent?.Invoke(_cts); From 269bb1ea80fd12031b0e006524e95a795dde59b0 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 30 Nov 2018 15:48:22 +0100 Subject: [PATCH 40/42] Make RemoteExceptionException directly derive from Exception --- src/System.CommandLine.Tests/RemoteExecution.cs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/System.CommandLine.Tests/RemoteExecution.cs b/src/System.CommandLine.Tests/RemoteExecution.cs index 1f3c168dec..7d87d6dc31 100644 --- a/src/System.CommandLine.Tests/RemoteExecution.cs +++ b/src/System.CommandLine.Tests/RemoteExecution.cs @@ -66,9 +66,20 @@ private void Dispose(bool disposing) } } - private sealed class RemoteExecutionException : XunitException + private sealed class RemoteExecutionException : Exception { - internal RemoteExecutionException(string stackTrace) : base("Remote process failed with an unhandled exception.", stackTrace) { } + private readonly string _stackTrace; + + internal RemoteExecutionException(string stackTrace) + : base("Remote process failed with an unhandled exception.") + { + _stackTrace = stackTrace; + } + + public override string StackTrace + { + get => _stackTrace ?? base.StackTrace; + } } } } \ No newline at end of file From a03dcdbef89ae28f6948be0bf7fa2183f6e9446d Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 4 Dec 2018 16:58:03 +0100 Subject: [PATCH 41/42] UseDefaults: include CancelOnProcessTermination --- src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs index edfa006cdc..1cf3d7c8e8 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs @@ -145,7 +145,8 @@ public static CommandLineBuilder UseDefaults(this CommandLineBuilder builder) .UseSuggestDirective() .RegisterWithDotnetSuggest() .UseParseErrorReporting() - .UseExceptionHandler(); + .UseExceptionHandler() + .CancelOnProcessTermination(); } public static TBuilder UsePrefixes(this TBuilder builder, IReadOnlyCollection prefixes) From 6116ef9c0cd081a8b16098635f82c66336a7805a Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 5 Dec 2018 08:51:39 +0100 Subject: [PATCH 42/42] Remove wiki feature description --- wiki_feature_description.md | 39 ------------------------------------- 1 file changed, 39 deletions(-) delete mode 100644 wiki_feature_description.md diff --git a/wiki_feature_description.md b/wiki_feature_description.md deleted file mode 100644 index 6fd5dec7c4..0000000000 --- a/wiki_feature_description.md +++ /dev/null @@ -1,39 +0,0 @@ -# General - -`System.CommandLine` supports performing actions when the invocation is being stopped due to process termination. For example, aborting an ongoing transaction, or flushing some data to disk. - -Process termination can be forcefull, which means the process is terminated by the OS and it doesn't get a chance to cleanup. This is _killing_ a process. - -Process termination can also be requested. For example, the user presses Control-C on an interactive application, or the system asks a service to terminate. - -# Implementing termination handling - -To add termination handling to a command, you must add a `CancellationToken` argument to the command handler. This token can be passed to async APIs. Cancellation actions can also be added directly using the `CancellationToken.Register` method. - -```c# -CommandHandler.Create(async (IConsole console, CancellationToken ct) => -{ - try - { - using (var httpClient = new HttpClient()) - { - await httpClient.GetAsync("http://www.example.com", ct); - } - return 0; - } - catch (OperationCanceledException) - { - console.Error.WriteLine("The operation was aborted"); - return 1; - } -}); -``` - -To enabel cancellation, the `CancelOnProcessTermination` middleware must be added to the `CommandLineBuilder`. - -```c# -var builder = new CommandLineBuilder() - . ... - .CancelOnProcessTermination() - . ... -``` \ No newline at end of file