Add support for timeout in process termination handling #1756
Conversation
ac56679 to
680d466
Compare
873530b to
a98a0ba
Compare
...ility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt
Outdated
Show resolved
Hide resolved
src/System.CommandLine.Tests/Invocation/CancelOnProcessTerminationTests.cs
Outdated
Show resolved
Hide resolved
|
@jonsequitur thank you for the initial feedback (should be addressed now). Are there any further unaddressed concerns or suggestions? |
src/System.CommandLine.Tests/Invocation/CancelOnProcessTerminationTests.cs
Outdated
Show resolved
Hide resolved
src/System.CommandLine.Tests/Invocation/CancelOnProcessTerminationTests.cs
Outdated
Show resolved
Hide resolved
I suggested a couple of test renames but feel free to adjust if you have something more precise. The goal is to embed the expectations about the scenario in the test name, like a spec. |
| [LinuxOnlyTheory] | ||
| [InlineData(SIGINT)] | ||
| [InlineData(SIGTERM)] | ||
| public async Task CancelOnProcessTermination_null_timeout_on_cancel_processing(int signo) |
There was a problem hiding this comment.
Is this test redundant with the pre-existing test above?
There was a problem hiding this comment.
It is redundand with the current CancelOnProcessTermination implementation. But it guards the possibility of contract/behavior change on explict passing of null as timeout.
I can remove it if you feel it's superfluous.
|
@jonsequitur Are there any further unaddressed concerns or suggestions? |
Background:
related to dotnet/templating#4799
It would be helpfull to be able to specify optional maximum timeout for handling the process termination - in order to prevent unwanted 'unkillable' command - especially for cases where command builder and and actual implementation of the command have different maintainers (e.g. a case of dotnet sdk).
The process that don't repond to Ctrl-C can still be killed by other means - but it may create unpleasant and inconsistent experience to users. The idea is to have top level handler that gives command chance to perform cleanup in correct way, but bail out if they do not return in timely manner (e.g. due to not using the cancellation patter at all).
If the optinal timeout is not specified, the original behavior remains (no timeout enforced).