Skip to content

Commit c145a08

Browse files
tmattmat
committed
Send Ctrl+C to launched process before killing it on Windows
1 parent 1e67fcc commit c145a08

File tree

16 files changed

+259
-108
lines changed

16 files changed

+259
-108
lines changed

‎src/BuiltInTools/DotNetDeltaApplier/Microsoft.Extensions.DotNetDeltaApplier.csproj

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
<ItemGroup Condition="'$(DotNetBuildSourceOnly)' == 'true' and '$(TargetFramework)' == 'net6.0'">
2626
<FrameworkReference Update="Microsoft.NETCore.App" TargetingPackVersion="6.0.0" />
2727
</ItemGroup>
28+
<ItemGroup>
29+
<Compile Include="..\dotnet-watch\Utilities\ProcessUtilities.cs" Link="ProcessUtilities.cs" />
30+
</ItemGroup>
2831

2932
<ItemGroup>
3033
<InternalsVisibleTo Include="Microsoft.Extensions.DotNetDeltaApplier.Tests" />

‎src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Diagnostics;
55
using System.IO.Pipes;
66
using Microsoft.DotNet.HotReload;
7+
using Microsoft.DotNet.Watch;
78

89
/// <summary>
910
/// The runtime startup hook looks for top-level type named "StartupHook".
@@ -58,7 +59,7 @@ public static void Initialize()
5859
return;
5960
}
6061

61-
RegisterPosixSignalHandlers();
62+
RegisterSignalHandlers();
6263

6364
var agent = new HotReloadAgent();
6465
try
@@ -79,27 +80,34 @@ public static void Initialize()
7980
}
8081
}
8182

82-
private static void RegisterPosixSignalHandlers()
83+
private static void RegisterSignalHandlers()
8384
{
85+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
86+
{
87+
ProcessUtilities.EnableWindowsCtrlCHandling(Log);
88+
}
89+
else
90+
{
8491
#if NET10_0_OR_GREATER
85-
// Register a handler for SIGTERM to allow graceful shutdown of the application on Unix.
86-
// See https://github.com/dotnet/docs/issues/46226.
92+
// Register a handler for SIGTERM to allow graceful shutdown of the application on Unix.
93+
// See https://github.com/dotnet/docs/issues/46226.
8794

88-
// Note: registered handlers are executed in reverse order of their registration.
89-
// Since the startup hook is executed before any code of the application, it is the first handler registered and thus the last to run.
90-
91-
s_signalRegistration = PosixSignalRegistration.Create(PosixSignal.SIGTERM, context =>
92-
{
93-
Log($"SIGTERM received. Cancel={context.Cancel}");
95+
// Note: registered handlers are executed in reverse order of their registration.
96+
// Since the startup hook is executed before any code of the application, it is the first handler registered and thus the last to run.
9497

95-
if (!context.Cancel)
98+
s_signalRegistration = PosixSignalRegistration.Create(PosixSignal.SIGTERM, context =>
9699
{
97-
Environment.Exit(0);
98-
}
99-
});
100+
Log($"SIGTERM received. Cancel={context.Cancel}");
100101

101-
Log("Posix signal handlers registered.");
102+
if (!context.Cancel)
103+
{
104+
Environment.Exit(0);
105+
}
106+
});
107+
108+
Log("Posix signal handlers registered.");
102109
#endif
110+
}
103111
}
104112

105113
private static async ValueTask InitializeAsync(NamedPipeClientStream pipeClient, HotReloadAgent agent, CancellationToken cancellationToken)

‎src/BuiltInTools/dotnet-watch/CommandLine/EnvironmentOptions.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ internal enum TestFlags
2222
/// This allows tests to trigger key based events.
2323
/// </summary>
2424
ReadKeyFromStdin = 1 << 3,
25+
26+
/// <summary>
27+
/// Set if dotnet-watch should re-enable Ctrl+C handling on Windows.
28+
/// </summary>
29+
EnableWindowsCtrlCHandling = 1 << 4,
2530
}
2631

2732
internal sealed record EnvironmentOptions(

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ public async ValueTask StartSessionAsync(CancellationToken cancellationToken)
130130
};
131131

132132
var launchResult = new ProcessLaunchResult();
133-
var runningProcess = _processRunner.RunAsync(processSpec, processReporter, isUserApplication: true, launchResult, processTerminationSource.Token);
133+
var runningProcess = _processRunner.RunAsync(processSpec, processReporter, launchResult, processTerminationSource.Token);
134134
if (launchResult.ProcessId == null)
135135
{
136136
// error already reported

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,7 @@ await FileWatcher.WaitForFileChangeAsync(
796796
{
797797
Executable = _context.EnvironmentOptions.MuxerPath,
798798
WorkingDirectory = Path.GetDirectoryName(projectPath)!,
799+
IsUserApplication = false,
799800
OnOutput = line =>
800801
{
801802
lock (buildOutput)
@@ -804,12 +805,12 @@ await FileWatcher.WaitForFileChangeAsync(
804805
}
805806
},
806807
// pass user-specified build arguments last to override defaults:
807-
Arguments = ["build", projectPath, "-consoleLoggerParameters:NoSummary;Verbosity=minimal", .. binLogArguments, .. buildArguments]
808+
Arguments = ["build", projectPath, "-consoleLoggerParameters:NoSummary;Verbosity=minimal", .. binLogArguments, .. buildArguments],
808809
};
809810

810811
_context.Reporter.Output($"Building {projectPath} ...");
811812

812-
var exitCode = await _context.ProcessRunner.RunAsync(processSpec, _context.Reporter, isUserApplication: false, launchResult: null, cancellationToken);
813+
var exitCode = await _context.ProcessRunner.RunAsync(processSpec, _context.Reporter, launchResult: null, cancellationToken);
813814
return (exitCode == 0, buildOutput.ToImmutableArray(), projectPath);
814815
}
815816

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

Lines changed: 51 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,27 @@ internal sealed class ProcessRunner(
1010
TimeSpan processCleanupTimeout,
1111
CancellationToken shutdownCancellationToken)
1212
{
13-
private const int SIGKILL = 9;
14-
private const int SIGTERM = 15;
15-
1613
private sealed class ProcessState
1714
{
1815
public int ProcessId;
1916
public bool HasExited;
2017
}
2118

19+
// For testing purposes only, lock on access.
20+
private static readonly HashSet<int> s_runningApplicationProcesses = [];
21+
22+
public static IReadOnlyCollection<int> GetRunningApplicationProcesses()
23+
{
24+
lock (s_runningApplicationProcesses)
25+
{
26+
return [.. s_runningApplicationProcesses];
27+
}
28+
}
29+
2230
/// <summary>
2331
/// Launches a process.
2432
/// </summary>
25-
/// <param name="isUserApplication">True if the process is a user application, false if it is a helper process (e.g. msbuild).</param>
26-
public async Task<int> RunAsync(ProcessSpec processSpec, IReporter reporter, bool isUserApplication, ProcessLaunchResult? launchResult, CancellationToken processTerminationToken)
33+
public async Task<int> RunAsync(ProcessSpec processSpec, IReporter reporter, ProcessLaunchResult? launchResult, CancellationToken processTerminationToken)
2734
{
2835
var state = new ProcessState();
2936
var stopwatch = new Stopwatch();
@@ -49,6 +56,14 @@ public async Task<int> RunAsync(ProcessSpec processSpec, IReporter reporter, boo
4956

5057
state.ProcessId = process.Id;
5158

59+
if (processSpec.IsUserApplication)
60+
{
61+
lock (s_runningApplicationProcesses)
62+
{
63+
s_runningApplicationProcesses.Add(state.ProcessId);
64+
}
65+
}
66+
5267
if (onOutput != null)
5368
{
5469
process.BeginOutputReadLine();
@@ -90,12 +105,12 @@ public async Task<int> RunAsync(ProcessSpec processSpec, IReporter reporter, boo
90105
// Either Ctrl+C was pressed or the process is being restarted.
91106

92107
// Non-cancellable to not leave orphaned processes around blocking resources:
93-
await TerminateProcessAsync(process, state, reporter, CancellationToken.None);
108+
await TerminateProcessAsync(process, processSpec, state, reporter, CancellationToken.None);
94109
}
95110
}
96111
catch (Exception e)
97112
{
98-
if (isUserApplication)
113+
if (processSpec.IsUserApplication)
99114
{
100115
reporter.Error($"Application failed: {e.Message}");
101116
}
@@ -104,6 +119,14 @@ public async Task<int> RunAsync(ProcessSpec processSpec, IReporter reporter, boo
104119
{
105120
stopwatch.Stop();
106121

122+
if (processSpec.IsUserApplication)
123+
{
124+
lock (s_runningApplicationProcesses)
125+
{
126+
s_runningApplicationProcesses.Remove(state.ProcessId);
127+
}
128+
}
129+
107130
state.HasExited = true;
108131

109132
try
@@ -117,7 +140,7 @@ public async Task<int> RunAsync(ProcessSpec processSpec, IReporter reporter, boo
117140

118141
reporter.Verbose($"Process id {process.Id} ran for {stopwatch.ElapsedMilliseconds}ms and exited with exit code {exitCode}.");
119142

120-
if (isUserApplication)
143+
if (processSpec.IsUserApplication)
121144
{
122145
if (exitCode == 0)
123146
{
@@ -157,6 +180,11 @@ private static Process CreateProcess(ProcessSpec processSpec, Action<OutputLine>
157180
}
158181
};
159182

183+
if (processSpec.IsUserApplication && RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
184+
{
185+
process.StartInfo.CreateNewProcessGroup = true;
186+
}
187+
160188
if (processSpec.EscapedArguments is not null)
161189
{
162190
process.StartInfo.Arguments = processSpec.EscapedArguments;
@@ -210,28 +238,24 @@ private static Process CreateProcess(ProcessSpec processSpec, Action<OutputLine>
210238
return process;
211239
}
212240

213-
private async ValueTask TerminateProcessAsync(Process process, ProcessState state, IReporter reporter, CancellationToken cancellationToken)
241+
private async ValueTask TerminateProcessAsync(Process process, ProcessSpec processSpec, ProcessState state, IReporter reporter, CancellationToken cancellationToken)
214242
{
215243
if (!shutdownCancellationToken.IsCancellationRequested)
216244
{
217-
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
245+
var forceOnly = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && !processSpec.IsUserApplication;
246+
247+
// Ctrl+C hasn't been sent.
248+
TerminateProcess(process, state, reporter, forceOnly);
249+
250+
if (forceOnly)
218251
{
219-
// Ctrl+C hasn't been sent, force termination.
220-
// We don't have means to terminate gracefully on Windows (https://github.com/dotnet/runtime/issues/109432)
221-
TerminateProcess(process, state, reporter, force: true);
222252
_ = await WaitForExitAsync(process, state, timeout: null, reporter, cancellationToken);
223-
224253
return;
225254
}
226-
else
227-
{
228-
// Ctrl+C hasn't been sent, send SIGTERM now:
229-
TerminateProcess(process, state, reporter, force: false);
230-
}
231255
}
232256

233257
// Ctlr+C/SIGTERM has been sent, wait for the process to exit gracefully.
234-
if (processCleanupTimeout.Milliseconds == 0 ||
258+
if (processCleanupTimeout.TotalMilliseconds == 0 ||
235259
!await WaitForExitAsync(process, state, processCleanupTimeout, reporter, cancellationToken))
236260
{
237261
// Force termination if the process is still running after the timeout.
@@ -327,55 +351,28 @@ private static void TerminateProcess(Process process, ProcessState state, IRepor
327351

328352
private static void TerminateWindowsProcess(Process process, ProcessState state, IReporter reporter, bool force)
329353
{
330-
// Needs API: https://github.com/dotnet/runtime/issues/109432
331-
// Code below does not work because the process creation needs CREATE_NEW_PROCESS_GROUP flag.
354+
var processId = state.ProcessId;
332355

333-
reporter.Verbose($"Terminating process {state.ProcessId}.");
356+
reporter.Verbose($"Terminating process {processId} ({(force ? "Kill" : "Ctrl+C")}).");
334357

335358
if (force)
336359
{
337360
process.Kill();
338361
}
339-
#if TODO
340362
else
341363
{
342-
const uint CTRL_C_EVENT = 0;
343-
344-
[DllImport("kernel32.dll", SetLastError = true)]
345-
static extern bool GenerateConsoleCtrlEvent(uint dwCtrlEvent, uint dwProcessGroupId);
346-
347-
[DllImport("kernel32.dll", SetLastError = true)]
348-
static extern bool AttachConsole(uint dwProcessId);
349-
350-
[DllImport("kernel32.dll", SetLastError = true)]
351-
static extern bool FreeConsole();
352-
353-
if (AttachConsole((uint)state.ProcessId) &&
354-
GenerateConsoleCtrlEvent(CTRL_C_EVENT, 0) &&
355-
FreeConsole())
356-
{
357-
return;
358-
}
359-
360-
var error = Marshal.GetLastPInvokeError();
361-
reporter.Verbose($"Failed to send Ctrl+C to process {state.ProcessId}: {Marshal.GetPInvokeErrorMessage(error)} (code {error})");
364+
ProcessUtilities.SendWindowsCtrlCEvent(processId, m => reporter.Verbose(m));
362365
}
363-
#endif
364366
}
365367

366368
private static void TerminateUnixProcess(ProcessState state, IReporter reporter, bool force)
367369
{
368-
[DllImport("libc", SetLastError = true, EntryPoint = "kill")]
369-
static extern int sys_kill(int pid, int sig);
370-
371370
reporter.Verbose($"Terminating process {state.ProcessId} ({(force ? "SIGKILL" : "SIGTERM")}).");
372371

373-
var result = sys_kill(state.ProcessId, force ? SIGKILL : SIGTERM);
374-
if (result != 0)
375-
{
376-
var error = Marshal.GetLastPInvokeError();
377-
reporter.Verbose($"Error while sending SIGTERM to process {state.ProcessId}: {Marshal.GetPInvokeErrorMessage(error)} (code {error}).");
378-
}
372+
ProcessUtilities.SendPosixSignal(
373+
state.ProcessId,
374+
signal: force ? ProcessUtilities.SIGKILL : ProcessUtilities.SIGTERM,
375+
log: m => reporter.Verbose(m));
379376
}
380377
}
381378
}

‎src/BuiltInTools/dotnet-watch/Process/ProcessSpec.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ internal sealed class ProcessSpec
1515
public ProcessExitAction? OnExit { get; set; }
1616
public CancellationToken CancelOutputCapture { get; set; }
1717

18+
/// <summary>
19+
/// True if the process is a user application, false if it is a helper process (e.g. dotnet build).</param>
20+
/// </summary>
21+
public bool IsUserApplication { get; set; }
22+
1823
public string? ShortDisplayName()
1924
=> Path.GetFileNameWithoutExtension(Executable);
2025

‎src/BuiltInTools/dotnet-watch/Process/ProjectLauncher.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ public EnvironmentOptions EnvironmentOptions
4747
var processSpec = new ProcessSpec
4848
{
4949
Executable = EnvironmentOptions.MuxerPath,
50+
IsUserApplication = true,
5051
WorkingDirectory = projectOptions.WorkingDirectory,
51-
OnOutput = onOutput
52+
OnOutput = onOutput,
5253
};
5354

5455
var environmentBuilder = EnvironmentVariablesBuilder.FromCurrentEnvironment();

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ public static async Task<int> Main(string[] args)
7676
if (environmentOptions.TestFlags != TestFlags.None)
7777
{
7878
reporter.Verbose($"Test flags: {environmentOptions.TestFlags}");
79+
80+
if (environmentOptions.TestFlags.HasFlag(TestFlags.EnableWindowsCtrlCHandling) &&
81+
RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
82+
{
83+
ProcessUtilities.EnableWindowsCtrlCHandling(m => reporter.Verbose($"Test: {m}"));
84+
}
7985
}
8086

8187
if (!TryFindProject(workingDirectory, options, reporter, out var projectPath))

‎src/BuiltInTools/dotnet-watch/Properties/launchSettings.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"DCP_IDE_REQUEST_TIMEOUT_SECONDS": "100000",
1010
"DCP_IDE_NOTIFICATION_TIMEOUT_SECONDS": "100000",
1111
"DCP_IDE_NOTIFICATION_KEEPALIVE_SECONDS": "100000",
12-
"DOTNET_WATCH_PROCESS_CLEANUP_TIMEOUT_MS": "0",
12+
"DOTNET_WATCH_PROCESS_CLEANUP_TIMEOUT_MS": "100000",
1313
"ASPIRE_ALLOW_UNSECURED_TRANSPORT": "1",
1414
"__DOTNET_WATCH_TEST_FLAGS": "RunningAsTest",
1515
"__DOTNET_WATCH_TEST_OUTPUT_DIR": "$(RepoRoot)\\artifacts\\tmp\\Debug"

0 commit comments

Comments
 (0)