Skip to content

Commit 821a27c

Browse files
committed
Reworking the code so it doesn't register for events without a cancellation token
1 parent 7f8e152 commit 821a27c

File tree

5 files changed

+55
-45
lines changed

5 files changed

+55
-45
lines changed

src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ System.CommandLine.Invocation
374374
public System.CommandLine.LocalizationResources LocalizationResources { get; }
375375
public System.CommandLine.Parsing.Parser Parser { get; }
376376
public System.CommandLine.Parsing.ParseResult ParseResult { get; set; }
377-
public System.Void AddLinkedCancellationToken(System.Threading.CancellationToken token)
377+
public System.Void AddLinkedCancellationToken(System.Func<System.Threading.CancellationToken> token)
378378
public System.Void Dispose()
379379
public delegate InvocationMiddleware : System.MulticastDelegate, System.ICloneable, System.Runtime.Serialization.ISerializable
380380
.ctor(System.Object object, System.IntPtr method)

src/System.CommandLine.Tests/Invocation/InvocationContextTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public void InvocationContext_with_linked_cancellation_token_can_cancel_by_passe
3535
.Build()
3636
.Parse("");
3737
using InvocationContext context = new(parseResult, cancellationToken: cts1.Token);
38-
context.AddLinkedCancellationToken(cts2.Token);
38+
context.AddLinkedCancellationToken(() => cts2.Token);
3939

4040
var token = context.CancellationToken;
4141

@@ -53,7 +53,7 @@ public void InvocationContext_with_linked_cancellation_token_can_cancel_by_linke
5353
.Build()
5454
.Parse("");
5555
using InvocationContext context = new(parseResult, cancellationToken: cts1.Token);
56-
context.AddLinkedCancellationToken(cts2.Token);
56+
context.AddLinkedCancellationToken(() => cts2.Token);
5757

5858
var token = context.CancellationToken;
5959

@@ -71,10 +71,10 @@ public void InvocationContext_adding_additional_linked_token_after_token_has_bee
7171
.Build()
7272
.Parse("");
7373
using InvocationContext context = new(parseResult, cancellationToken: cts.Token);
74-
context.AddLinkedCancellationToken(cts.Token);
74+
context.AddLinkedCancellationToken(() => cts.Token);
7575
_ = context.CancellationToken;
7676

77-
Assert.Throws<InvalidOperationException>(() => context.AddLinkedCancellationToken(cts.Token));
77+
Assert.Throws<InvalidOperationException>(() => context.AddLinkedCancellationToken(() => cts.Token));
7878
}
7979
}
8080
}

src/System.CommandLine.Tests/Invocation/InvocationPipelineTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ public async Task Command_InvokeAsync_can_cancel_from_middleware()
355355
})
356356
.AddMiddleware(async (context, next) =>
357357
{
358-
context.AddLinkedCancellationToken(cts.Token);
358+
context.AddLinkedCancellationToken(() => cts.Token);
359359
cts.Cancel();
360360
await next(context);
361361
})

src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -47,32 +47,39 @@ public static CommandLineBuilder CancelOnProcessTermination(this CommandLineBuil
4747
{
4848
builder.AddMiddleware(async (context, next) =>
4949
{
50-
//TODO: This CancellationTokenSource is never disposed...
51-
CancellationTokenSource cts = new();
50+
ConsoleCancelEventHandler? consoleHandler = null;
51+
EventHandler? processExitHandler = null;
52+
ManualResetEventSlim? blockProcessExit = null;
5253

53-
context.AddLinkedCancellationToken(cts.Token);
54-
55-
ManualResetEventSlim blockProcessExit = new(initialState: false);
56-
ConsoleCancelEventHandler consoleHandler = (_, args) =>
57-
{
58-
cts.Cancel();
59-
// Stop the process from terminating.
60-
// Since the context was cancelled, the invocation should
61-
// finish and Main will return.
62-
args.Cancel = true;
63-
};
64-
EventHandler processExitHandler = (_, _) =>
54+
context.AddLinkedCancellationToken(() =>
6555
{
66-
cts.Cancel();
67-
// The process exits as soon as the event handler returns.
68-
// We provide a return value using Environment.ExitCode
69-
// because Main will not finish executing.
70-
// Wait for the invocation to finish.
71-
blockProcessExit.Wait();
72-
ExitCode = context.ExitCode;
73-
};
74-
Console.CancelKeyPress += consoleHandler;
75-
AppDomain.CurrentDomain.ProcessExit += processExitHandler;
56+
//TODO: This CancellationTokenSource is never disposed...
57+
CancellationTokenSource cts = new();
58+
blockProcessExit = new ManualResetEventSlim(initialState: false);
59+
consoleHandler = (_, args) =>
60+
{
61+
cts.Cancel();
62+
// Stop the process from terminating.
63+
// Since the context was cancelled, the invocation should
64+
// finish and Main will return.
65+
args.Cancel = true;
66+
};
67+
processExitHandler = (_, _) =>
68+
{
69+
cts.Cancel();
70+
// The process exits as soon as the event handler returns.
71+
// We provide a return value using Environment.ExitCode
72+
// because Main will not finish executing.
73+
// Wait for the invocation to finish.
74+
blockProcessExit.Wait();
75+
ExitCode = context.ExitCode;
76+
};
77+
Console.CancelKeyPress += consoleHandler;
78+
AppDomain.CurrentDomain.ProcessExit += processExitHandler;
79+
80+
return cts.Token;
81+
});
82+
7683

7784
try
7885
{
@@ -82,7 +89,7 @@ public static CommandLineBuilder CancelOnProcessTermination(this CommandLineBuil
8289
{
8390
Console.CancelKeyPress -= consoleHandler;
8491
AppDomain.CurrentDomain.ProcessExit -= processExitHandler;
85-
blockProcessExit.Set();
92+
blockProcessExit?.Set();
8693
}
8794
}, MiddlewareOrderInternal.Startup);
8895

@@ -439,7 +446,7 @@ public static CommandLineBuilder UseHelp(
439446
int? maxWidth = null)
440447
{
441448
builder.CustomizeHelpLayout(customize);
442-
449+
443450
if (builder.HelpOption is null)
444451
{
445452
builder.UseHelp(new HelpOption(() => builder.LocalizationResources), maxWidth);

src/System.CommandLine/Invocation/InvocationContext.cs

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ public sealed class InvocationContext : IDisposable
1616
{
1717
private HelpBuilder? _helpBuilder;
1818
private CancellationTokenSource? _linkedTokensSource;
19-
private List<CancellationToken>? _linkedTokens;
20-
private CancellationToken _cancellationToken;
19+
private List<Func<CancellationToken>> _cancellationTokens = new(3);
2120
private Lazy<CancellationToken> _lazyCancellationToken;
2221

2322
/// <param name="parseResult">The result of the current parse operation.</param>
@@ -32,7 +31,7 @@ public InvocationContext(
3231
BindingContext.ServiceProvider.AddService(_ => CancellationToken);
3332
BindingContext.ServiceProvider.AddService(_ => this);
3433

35-
_cancellationToken = cancellationToken;
34+
_cancellationTokens.Add(() => cancellationToken);
3635
_lazyCancellationToken = new Lazy<CancellationToken>(BuildCancellationToken);
3736
}
3837

@@ -88,15 +87,19 @@ public ParseResult ParseResult
8887

8988
private CancellationToken BuildCancellationToken()
9089
{
91-
if (_linkedTokens is not null)
90+
switch(_cancellationTokens.Count)
9291
{
93-
var tokens = new List<CancellationToken>(_linkedTokens.Count + 1);
94-
tokens.Add(_cancellationToken);
95-
tokens.AddRange(_linkedTokens);
96-
_linkedTokensSource = CancellationTokenSource.CreateLinkedTokenSource(tokens.ToArray());
97-
return _linkedTokensSource.Token;
98-
}
99-
return _cancellationToken;
92+
case 0: return CancellationToken.None;
93+
case 1: return _cancellationTokens[0]();
94+
default:
95+
CancellationToken[] tokens = new CancellationToken[_cancellationTokens.Count];
96+
for(int i = 0; i < _cancellationTokens.Count; i++)
97+
{
98+
tokens[i] = _cancellationTokens[i]();
99+
}
100+
_linkedTokensSource = CancellationTokenSource.CreateLinkedTokenSource(tokens);
101+
return _linkedTokensSource.Token;
102+
};
100103
}
101104

102105

@@ -108,13 +111,13 @@ public void Dispose()
108111
(Console as IDisposable)?.Dispose();
109112
}
110113

111-
public void AddLinkedCancellationToken(CancellationToken token)
114+
public void AddLinkedCancellationToken(Func<CancellationToken> token)
112115
{
113116
if (_lazyCancellationToken.IsValueCreated)
114117
{
115118
throw new InvalidOperationException($"Cannot add additional linked cancellation tokens once {nameof(InvocationContext)}.{nameof(CancellationToken)} has been invoked");
116119
}
117-
(_linkedTokens ??= new()).Add(token);
120+
_cancellationTokens.Add(token);
118121
}
119122
}
120123
}

0 commit comments

Comments
 (0)