Skip to content

Commit 0bff161

Browse files
tmattmat
committed
Simplify process termination
1 parent 5b140d5 commit 0bff161

18 files changed

+173
-133
lines changed

‎src/BuiltInTools/dotnet-watch/DotNetWatchContext.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ internal sealed class DotNetWatchContext
99
public required GlobalOptions Options { get; init; }
1010
public required EnvironmentOptions EnvironmentOptions { get; init; }
1111
public required IReporter Reporter { get; init; }
12+
public required ProcessRunner ProcessRunner { get; init; }
1213

1314
public required ProjectOptions RootProjectOptions { get; init; }
1415
}

‎src/BuiltInTools/dotnet-watch/DotNetWatcher.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke
8181

8282
fileSetWatcher.WatchContainingDirectories(evaluationResult.Files.Keys);
8383

84-
var processTask = ProcessRunner.RunAsync(processSpec, Context.Reporter, isUserApplication: true, launchResult: null, combinedCancellationSource.Token);
84+
var processTask = Context.ProcessRunner.RunAsync(processSpec, Context.Reporter, isUserApplication: true, launchResult: null, combinedCancellationSource.Token);
8585

8686
Task<ChangedFile?> fileSetTask;
8787
Task finishedTask;

‎src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ internal sealed class CompilationHandler : IDisposable
1717
public readonly EnvironmentOptions EnvironmentOptions;
1818
private readonly IReporter _reporter;
1919
private readonly WatchHotReloadService _hotReloadService;
20+
private readonly ProcessRunner _processRunner;
2021

2122
/// <summary>
2223
/// Lock to synchronize:
@@ -36,17 +37,15 @@ internal sealed class CompilationHandler : IDisposable
3637
/// </summary>
3738
private ImmutableList<WatchHotReloadService.Update> _previousUpdates = [];
3839

39-
private readonly CancellationToken _shutdownCancellationToken;
40-
4140
private bool _isDisposed;
4241

43-
public CompilationHandler(IReporter reporter, EnvironmentOptions environmentOptions, CancellationToken shutdownCancellationToken)
42+
public CompilationHandler(IReporter reporter, ProcessRunner processRunner, EnvironmentOptions environmentOptions)
4443
{
4544
_reporter = reporter;
45+
_processRunner = processRunner;
4646
EnvironmentOptions = environmentOptions;
4747
Workspace = new IncrementalMSBuildWorkspace(reporter);
4848
_hotReloadService = new WatchHotReloadService(Workspace.CurrentSolution.Services, () => ValueTask.FromResult(GetAggregateCapabilities()));
49-
_shutdownCancellationToken = shutdownCancellationToken;
5049
}
5150

5251
public void Dispose()
@@ -153,7 +152,7 @@ public async ValueTask StartSessionAsync(CancellationToken cancellationToken)
153152
};
154153

155154
var launchResult = new ProcessLaunchResult();
156-
var runningProcess = ProcessRunner.RunAsync(processSpec, processReporter, isUserApplication: true, launchResult, processTerminationSource.Token);
155+
var runningProcess = _processRunner.RunAsync(processSpec, processReporter, isUserApplication: true, launchResult, processTerminationSource.Token);
157156
if (launchResult.ProcessId == null)
158157
{
159158
// error already reported
@@ -167,7 +166,6 @@ public async ValueTask StartSessionAsync(CancellationToken cancellationToken)
167166
var runningProject = new RunningProject(
168167
projectNode,
169168
projectOptions,
170-
EnvironmentOptions,
171169
deltaApplier,
172170
processReporter,
173171
browserRefreshServer,
@@ -674,7 +672,7 @@ public bool TryGetRunningProject(string projectPath, out ImmutableArray<RunningP
674672
private async ValueTask<IReadOnlyList<int>> TerminateRunningProjects(IEnumerable<RunningProject> projects, CancellationToken cancellationToken)
675673
{
676674
// wait for all tasks to complete:
677-
return await Task.WhenAll(projects.Select(p => p.TerminateAsync(_shutdownCancellationToken).AsTask())).WaitAsync(cancellationToken);
675+
return await Task.WhenAll(projects.Select(p => p.TerminateAsync().AsTask())).WaitAsync(cancellationToken);
678676
}
679677

680678
private static Task ForEachProjectAsync(ImmutableDictionary<string, ImmutableArray<RunningProject>> projects, Func<RunningProject, CancellationToken, Task> action, CancellationToken cancellationToken)

‎src/BuiltInTools/dotnet-watch/HotReload/RunningProject.cs‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ namespace Microsoft.DotNet.Watch
1212
internal sealed class RunningProject(
1313
ProjectGraphNode projectNode,
1414
ProjectOptions options,
15-
EnvironmentOptions environmentOptions,
1615
DeltaApplier deltaApplier,
1716
IReporter reporter,
1817
BrowserRefreshServer? browserRefreshServer,
@@ -70,21 +69,8 @@ public async ValueTask WaitForProcessRunningAsync(CancellationToken cancellation
7069
await DeltaApplier.WaitForProcessRunningAsync(cancellationToken);
7170
}
7271

73-
public async ValueTask<int> TerminateAsync(CancellationToken shutdownCancellationToken)
72+
public async ValueTask<int> TerminateAsync()
7473
{
75-
if (shutdownCancellationToken.IsCancellationRequested)
76-
{
77-
// Ctrl+C sent, wait for the process to exit
78-
try
79-
{
80-
_ = await RunningProcess.WaitAsync(environmentOptions.ProcessCleanupTimeout, CancellationToken.None);
81-
}
82-
catch (TimeoutException)
83-
{
84-
// nop
85-
}
86-
}
87-
8874
ProcessTerminationSource.Cancel();
8975
return await RunningProcess;
9076
}

‎src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke
100100
}
101101

102102
var projectMap = new ProjectNodeMap(evaluationResult.ProjectGraph, Context.Reporter);
103-
compilationHandler = new CompilationHandler(Context.Reporter, Context.EnvironmentOptions, shutdownCancellationToken);
103+
compilationHandler = new CompilationHandler(Context.Reporter, Context.ProcessRunner, Context.EnvironmentOptions);
104104
var scopedCssFileHandler = new ScopedCssFileHandler(Context.Reporter, projectMap, browserConnector);
105105
var projectLauncher = new ProjectLauncher(Context, projectMap, browserConnector, compilationHandler, iteration);
106106
var outputDirectories = GetProjectOutputDirectories(evaluationResult.ProjectGraph);
@@ -459,6 +459,8 @@ async Task<ImmutableList<ChangedFile>> CaptureChangedFilesSnapshot(ImmutableDict
459459
changedFiles = changedFiles
460460
.Select(f => evaluationResult.Files.TryGetValue(f.Item.FilePath, out var evaluatedFile) ? f with { Item = evaluatedFile } : f)
461461
.ToImmutableList();
462+
463+
Context.Reporter.Report(MessageDescriptor.ReEvaluationCompleted);
462464
}
463465

464466
if (rebuiltProjects != null)
@@ -531,7 +533,7 @@ async Task<ImmutableList<ChangedFile>> CaptureChangedFilesSnapshot(ImmutableDict
531533

532534
if (rootRunningProject != null)
533535
{
534-
await rootRunningProject.TerminateAsync(shutdownCancellationToken);
536+
await rootRunningProject.TerminateAsync();
535537
}
536538

537539
if (runtimeProcessLauncher != null)
@@ -841,7 +843,7 @@ await FileWatcher.WaitForFileChangeAsync(
841843

842844
Context.Reporter.Output($"Building {projectPath} ...");
843845

844-
var exitCode = await ProcessRunner.RunAsync(processSpec, Context.Reporter, isUserApplication: false, launchResult: null, cancellationToken);
846+
var exitCode = await Context.ProcessRunner.RunAsync(processSpec, Context.Reporter, isUserApplication: false, launchResult: null, cancellationToken);
845847
return (exitCode == 0, buildOutput.ToImmutableArray(), projectPath);
846848
}
847849

‎src/BuiltInTools/dotnet-watch/Internal/IReporter.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ public MessageDescriptor ToErrorWhen(bool condition)
6868
public static readonly MessageDescriptor FixBuildError = new("Fix the error to continue or press Ctrl+C to exit.", WatchEmoji, MessageSeverity.Warning, s_id++);
6969
public static readonly MessageDescriptor WaitingForChanges = new("Waiting for changes", WatchEmoji, MessageSeverity.Verbose, s_id++);
7070
public static readonly MessageDescriptor LaunchedProcess = new("Launched '{0}' with arguments '{1}': process id {2}", LaunchEmoji, MessageSeverity.Verbose, s_id++);
71-
public static readonly MessageDescriptor KillingProcess = new("Killing process {0}", WatchEmoji, MessageSeverity.Verbose, s_id++);
7271
public static readonly MessageDescriptor HotReloadChangeHandled = new("Hot reload change handled in {0}ms.", HotReloadEmoji, MessageSeverity.Verbose, s_id++);
7372
public static readonly MessageDescriptor HotReloadSucceeded = new("Hot reload succeeded.", HotReloadEmoji, MessageSeverity.Output, s_id++);
7473
public static readonly MessageDescriptor UpdatesApplied = new("Updates applied: {0} out of {1}.", HotReloadEmoji, MessageSeverity.Verbose, s_id++);
@@ -87,6 +86,7 @@ public MessageDescriptor ToErrorWhen(bool condition)
8786
public static readonly MessageDescriptor IgnoringChangeInHiddenDirectory = new("Ignoring change in hidden directory '{0}': {1} '{2}'", WatchEmoji, MessageSeverity.Verbose, s_id++);
8887
public static readonly MessageDescriptor IgnoringChangeInOutputDirectory = new("Ignoring change in output directory: {0} '{1}'", WatchEmoji, MessageSeverity.Verbose, s_id++);
8988
public static readonly MessageDescriptor FileAdditionTriggeredReEvaluation = new("File addition triggered re-evaluation.", WatchEmoji, MessageSeverity.Verbose, s_id++);
89+
public static readonly MessageDescriptor ReEvaluationCompleted = new("Re-evaluation completed.", WatchEmoji, MessageSeverity.Verbose, s_id++);
9090
public static readonly MessageDescriptor NoCSharpChangesToApply = new("No C# changes to apply.", WatchEmoji, MessageSeverity.Output, s_id++);
9191
public static readonly MessageDescriptor Exited = new("Exited", WatchEmoji, MessageSeverity.Output, s_id++);
9292
public static readonly MessageDescriptor ExitedWithUnknownErrorCode = new("Exited with unknown error code", ErrorEmoji, MessageSeverity.Error, s_id++);

‎src/BuiltInTools/dotnet-watch/Internal/MsBuildFileSetFactory.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ internal class MSBuildFileSetFactory(
2020
string rootProjectFile,
2121
IEnumerable<string> buildArguments,
2222
EnvironmentOptions environmentOptions,
23+
ProcessRunner processRunner,
2324
IReporter reporter)
2425
{
2526
private const string TargetName = "GenerateWatchList";
@@ -53,7 +54,7 @@ internal class MSBuildFileSetFactory(
5354

5455
reporter.Verbose($"Running MSBuild target '{TargetName}' on '{rootProjectFile}'");
5556

56-
var exitCode = await ProcessRunner.RunAsync(processSpec, reporter, isUserApplication: false, launchResult: null, cancellationToken);
57+
var exitCode = await processRunner.RunAsync(processSpec, reporter, isUserApplication: false, launchResult: null, cancellationToken);
5758

5859
var success = exitCode == 0 && File.Exists(watchList);
5960

‎src/BuiltInTools/dotnet-watch/Internal/ProcessRunner.cs‎

Lines changed: 87 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66

77
namespace Microsoft.DotNet.Watch
88
{
9-
internal sealed class ProcessRunner
9+
internal sealed class ProcessRunner(
10+
TimeSpan processCleanupTimeout,
11+
CancellationToken shutdownCancellationToken)
1012
{
1113
private const int SIGKILL = 9;
1214
private const int SIGTERM = 15;
@@ -15,14 +17,13 @@ private sealed class ProcessState
1517
{
1618
public int ProcessId;
1719
public bool HasExited;
18-
public bool ForceExit;
1920
}
2021

2122
/// <summary>
2223
/// Launches a process.
2324
/// </summary>
2425
/// <param name="isUserApplication">True if the process is a user application, false if it is a helper process (e.g. msbuild).</param>
25-
public static async Task<int> RunAsync(ProcessSpec processSpec, IReporter reporter, bool isUserApplication, ProcessLaunchResult? launchResult, CancellationToken processTerminationToken)
26+
public async Task<int> RunAsync(ProcessSpec processSpec, IReporter reporter, bool isUserApplication, ProcessLaunchResult? launchResult, CancellationToken processTerminationToken)
2627
{
2728
Ensure.NotNull(processSpec, nameof(processSpec));
2829

@@ -38,8 +39,6 @@ public static async Task<int> RunAsync(ProcessSpec processSpec, IReporter report
3839

3940
using var process = CreateProcess(processSpec, onOutput, state, reporter);
4041

41-
processTerminationToken.Register(() => TerminateProcess(process, state, reporter));
42-
4342
stopwatch.Start();
4443

4544
Exception? launchException = null;
@@ -85,45 +84,15 @@ public static async Task<int> RunAsync(ProcessSpec processSpec, IReporter report
8584
{
8685
try
8786
{
88-
await process.WaitForExitAsync(processTerminationToken);
89-
90-
// ensures that all process output has been reported:
91-
try
92-
{
93-
process.WaitForExit();
94-
}
95-
catch
96-
{
97-
}
87+
_ = await WaitForExitAsync(process, timeout: null, processTerminationToken);
9888
}
9989
catch (OperationCanceledException)
10090
{
10191
// Process termination requested via cancellation token.
102-
// Wait for the actual process exit.
103-
while (true)
104-
{
105-
try
106-
{
107-
// non-cancellable to not leave orphaned processes around blocking resources:
108-
await process.WaitForExitAsync(CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(5), CancellationToken.None);
109-
break;
110-
}
111-
catch (TimeoutException)
112-
{
113-
// nop
114-
}
115-
116-
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || state.ForceExit)
117-
{
118-
reporter.Output($"Waiting for process {state.ProcessId} to exit ...");
119-
}
120-
else
121-
{
122-
reporter.Output($"Forcing process {state.ProcessId} to exit ...");
123-
}
92+
// Either Ctrl+C was pressed or the process is being restarted.
12493

125-
state.ForceExit = true;
126-
}
94+
// Non-cancellable to not leave orphaned processes around blocking resources:
95+
await TerminateProcessAsync(process, state, reporter, CancellationToken.None);
12796
}
12897
}
12998
catch (Exception e)
@@ -243,24 +212,82 @@ private static Process CreateProcess(ProcessSpec processSpec, Action<OutputLine>
243212
return process;
244213
}
245214

246-
private static void TerminateProcess(Process process, ProcessState state, IReporter reporter)
215+
private async ValueTask TerminateProcessAsync(Process process, ProcessState state, IReporter reporter, CancellationToken cancellationToken)
216+
{
217+
if (!shutdownCancellationToken.IsCancellationRequested)
218+
{
219+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
220+
{
221+
// Ctrl+C hasn't been sent, force termination.
222+
// We don't have means to terminate gracefully on Windows (https://github.com/dotnet/runtime/issues/109432)
223+
TerminateProcess(process, state, reporter, force: true);
224+
_ = await WaitForExitAsync(process, timeout: null, cancellationToken);
225+
226+
return;
227+
}
228+
else
229+
{
230+
// Ctrl+C hasn't been sent, send SIGTERM now:
231+
TerminateProcess(process, state, reporter, force: false);
232+
}
233+
}
234+
235+
// Ctlr+C/SIGTERM has been sent, wait for the process to exit gracefully.
236+
if (!await WaitForExitAsync(process, processCleanupTimeout, cancellationToken))
237+
{
238+
// Force termination if the process is still running after the timeout.
239+
TerminateProcess(process, state, reporter, force: true);
240+
241+
_ = await WaitForExitAsync(process, timeout: null, cancellationToken);
242+
}
243+
}
244+
245+
private static async ValueTask<bool> WaitForExitAsync(Process process, TimeSpan? timeout, CancellationToken cancellationToken)
246+
{
247+
var task = process.WaitForExitAsync(cancellationToken);
248+
249+
if (timeout.HasValue)
250+
{
251+
try
252+
{
253+
await task.WaitAsync(timeout.Value, cancellationToken);
254+
}
255+
catch (TimeoutException)
256+
{
257+
return false;
258+
}
259+
}
260+
else
261+
{
262+
await task;
263+
}
264+
265+
// ensures that all process output has been reported:
266+
try
267+
{
268+
process.WaitForExit();
269+
}
270+
catch
271+
{
272+
}
273+
274+
return true;
275+
}
276+
277+
private static void TerminateProcess(Process process, ProcessState state, IReporter reporter, bool force)
247278
{
248279
try
249280
{
250281
if (!state.HasExited && !process.HasExited)
251282
{
252-
reporter.Report(MessageDescriptor.KillingProcess, state.ProcessId.ToString());
253-
254283
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
255284
{
256-
TerminateWindowsProcess(process, state, reporter);
285+
TerminateWindowsProcess(process, state, reporter, force);
257286
}
258287
else
259288
{
260-
TerminateUnixProcess(state, reporter);
289+
TerminateUnixProcess(state, reporter, force);
261290
}
262-
263-
reporter.Verbose($"Process {state.ProcessId} killed.");
264291
}
265292
}
266293
catch (Exception ex)
@@ -272,12 +299,19 @@ private static void TerminateProcess(Process process, ProcessState state, IRepor
272299
}
273300
}
274301

275-
private static void TerminateWindowsProcess(Process process, ProcessState state, IReporter reporter)
302+
private static void TerminateWindowsProcess(Process process, ProcessState state, IReporter reporter, bool force)
276303
{
277304
// Needs API: https://github.com/dotnet/runtime/issues/109432
278305
// Code below does not work because the process creation needs CREATE_NEW_PROCESS_GROUP flag.
279-
#if TODO
280-
if (!state.ForceExit)
306+
307+
reporter.Verbose($"Terminating process {state.ProcessId}.");
308+
309+
if (force)
310+
{
311+
process.Kill();
312+
}
313+
#if TODO
314+
else
281315
{
282316
const uint CTRL_C_EVENT = 0;
283317

@@ -301,16 +335,16 @@ private static void TerminateWindowsProcess(Process process, ProcessState state,
301335
reporter.Verbose($"Failed to send Ctrl+C to process {state.ProcessId}: {Marshal.GetPInvokeErrorMessage(error)} (code {error})");
302336
}
303337
#endif
304-
305-
process.Kill();
306338
}
307339

308-
private static void TerminateUnixProcess(ProcessState state, IReporter reporter)
340+
private static void TerminateUnixProcess(ProcessState state, IReporter reporter, bool force)
309341
{
310342
[DllImport("libc", SetLastError = true, EntryPoint = "kill")]
311343
static extern int sys_kill(int pid, int sig);
312344

313-
var result = sys_kill(state.ProcessId, state.ForceExit ? SIGKILL : SIGTERM);
345+
reporter.Verbose($"Terminating process {state.ProcessId} ({(force ? "SIGKILL" : "SIGTERM")}).");
346+
347+
var result = sys_kill(state.ProcessId, force ? SIGKILL : SIGTERM);
314348
if (result != 0)
315349
{
316350
var error = Marshal.GetLastPInvokeError();

0 commit comments

Comments
 (0)