Skip to content

Commit baaad19

Browse files
authored
Fix #2743 - allow Argument<T>.CustomParser to be set to null (#2784)
* fix #2743 * fix intermittent race condition in InvocationHPipeline
1 parent 0b48ccb commit baaad19

4 files changed

Lines changed: 31 additions & 8 deletions

File tree

src/System.CommandLine.Tests/CustomParsingTests.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,23 @@ public void Custom_parser_is_called_once_per_parse_operation_when_input_is_provi
561561
i.Should().Be(2);
562562
}
563563

564+
[Fact] // https://github.com/dotnet/command-line-api/issues/2743
565+
public void Setting_CustomParser_to_null_reverts_to_default_parsing()
566+
{
567+
Argument<int> argument = new("int")
568+
{
569+
CustomParser = (_) => 0
570+
};
571+
572+
argument.CustomParser = null;
573+
574+
var command = new RootCommand { argument };
575+
576+
var result = command.Parse("123");
577+
578+
result.GetValue(argument).Should().Be(123);
579+
}
580+
564581
[Fact]
565582
public void Default_value_factory_is_called_once_per_parse_operation_when_no_input_is_provided()
566583
{

src/System.CommandLine/Argument{T}.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ public Func<ArgumentResult, T>? DefaultValueFactory
7878
}
7979
};
8080
}
81+
else
82+
{
83+
ConvertArguments = null;
84+
}
8185
}
8286
}
8387

src/System.CommandLine/Invocation/InvocationPipeline.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,21 +44,22 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
4444
return syncAction.Invoke(parseResult);
4545

4646
case AsynchronousCommandLineAction asyncAction:
47-
var startedInvocation = asyncAction.InvokeAsync(parseResult, cts.Token);
48-
4947
var timeout = parseResult.InvocationConfiguration.ProcessTerminationTimeout;
5048

5149
if (timeout.HasValue)
5250
{
53-
terminationHandler = new(cts, startedInvocation, timeout.Value);
51+
terminationHandler = new(cts, timeout.Value);
5452
}
5553

54+
var startedInvocation = asyncAction.InvokeAsync(parseResult, cts.Token);
55+
5656
if (terminationHandler is null)
5757
{
5858
return await startedInvocation;
5959
}
6060
else
6161
{
62+
terminationHandler.StartedHandler = startedInvocation;
6263
// Handlers may not implement cancellation.
6364
// In such cases, when CancelOnProcessTermination is configured and user presses Ctrl+C,
6465
// ProcessTerminationCompletionSource completes first, with the result equal to native exit code for given signal.

src/System.CommandLine/Invocation/ProcessTerminationHandler.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,20 @@ internal sealed class ProcessTerminationHandler : IDisposable
1414

1515
internal readonly TaskCompletionSource<int> ProcessTerminationCompletionSource;
1616
private readonly CancellationTokenSource _handlerCancellationTokenSource;
17-
private readonly Task<int> _startedHandler;
17+
private Task<int>? _startedHandler;
1818
private readonly TimeSpan _processTerminationTimeout;
1919
#if NET7_0_OR_GREATER
2020
private readonly IDisposable? _sigIntRegistration, _sigTermRegistration;
2121
#endif
22-
22+
23+
internal Task<int> StartedHandler { set => Volatile.Write(ref _startedHandler, value); }
24+
2325
internal ProcessTerminationHandler(
2426
CancellationTokenSource handlerCancellationTokenSource,
25-
Task<int> startedHandler,
2627
TimeSpan processTerminationTimeout)
2728
{
2829
ProcessTerminationCompletionSource = new ();
2930
_handlerCancellationTokenSource = handlerCancellationTokenSource;
30-
_startedHandler = startedHandler;
3131
_processTerminationTimeout = processTerminationTimeout;
3232

3333
#if NET7_0_OR_GREATER // we prefer the new API as they allow for cancelling SIGTERM
@@ -86,8 +86,9 @@ void Cancel(int forcedTerminationExitCode)
8686

8787
try
8888
{
89+
var startedHandler = Volatile.Read(ref _startedHandler);
8990
// wait for the configured interval
90-
if (!_startedHandler.Wait(_processTerminationTimeout))
91+
if (startedHandler is null || !startedHandler.Wait(_processTerminationTimeout))
9192
{
9293
// if the handler does not finish within configured time,
9394
// use the completion source to signal forced completion (preserving native exit code)

0 commit comments

Comments
 (0)