[release/9.0.2xx]: Enable Ctrl+C forwarding to child processes (#46012)

tmat · web-flow · commit ac1a9d780bee · 2025-01-16T20:17:56.000-08:00
diff --git a/src/BuiltInTools/dotnet-watch/EnvironmentOptions.cs b/src/BuiltInTools/dotnet-watch/EnvironmentOptions.cs
@@ -27,6 +27,7 @@ internal enum TestFlags
     internal sealed record EnvironmentOptions(
         string WorkingDirectory,
         string MuxerPath,
+        TimeSpan ProcessCleanupTimeout,
         bool IsPollingEnabled = false,
         bool SuppressHandlingStaticContentFiles = false,
         bool SuppressMSBuildIncrementalism = false,
@@ -40,6 +41,7 @@ internal sealed record EnvironmentOptions(
         (
             WorkingDirectory: Directory.GetCurrentDirectory(),
             MuxerPath: GetMuxerPathFromEnvironment(),
+            ProcessCleanupTimeout: EnvironmentVariables.ProcessCleanupTimeout,
             IsPollingEnabled: EnvironmentVariables.IsPollingEnabled,
             SuppressHandlingStaticContentFiles: EnvironmentVariables.SuppressHandlingStaticContentFiles,
             SuppressMSBuildIncrementalism: EnvironmentVariables.SuppressMSBuildIncrementalism,
diff --git a/src/BuiltInTools/dotnet-watch/EnvironmentVariables.cs b/src/BuiltInTools/dotnet-watch/EnvironmentVariables.cs
@@ -22,6 +22,7 @@ public static partial class Names
     public static bool IsPollingEnabled => ReadBool("DOTNET_USE_POLLING_FILE_WATCHER");
     public static bool SuppressEmojis => ReadBool("DOTNET_WATCH_SUPPRESS_EMOJIS");
     public static bool RestartOnRudeEdit => ReadBool("DOTNET_WATCH_RESTART_ON_RUDE_EDIT");
+    public static TimeSpan ProcessCleanupTimeout => ReadTimeSpan("DOTNET_WATCH_PROCESS_CLEANUP_TIMEOUT_MS");
 
     public static string SdkRootDirectory =>
 #if DEBUG
@@ -43,4 +44,7 @@ public static partial class Names
 
     private static bool ReadBool(string variableName)
         => Environment.GetEnvironmentVariable(variableName) is var value && (value == "1" || bool.TryParse(value, out var boolValue) && boolValue);
+
+    private static TimeSpan ReadTimeSpan(string variableName)
+        => Environment.GetEnvironmentVariable(variableName) is var value && long.TryParse(value, out var intValue) && intValue >= 0 ? TimeSpan.FromMilliseconds(intValue) : TimeSpan.FromSeconds(5);
 }
diff --git a/src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs b/src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs
@@ -15,6 +15,7 @@ namespace Microsoft.DotNet.Watch
     internal sealed class CompilationHandler : IDisposable
     {
         public readonly IncrementalMSBuildWorkspace Workspace;
+        public readonly EnvironmentOptions EnvironmentOptions;
 
         private readonly IReporter _reporter;
         private readonly WatchHotReloadService _hotReloadService;
@@ -43,13 +44,17 @@ internal sealed class CompilationHandler : IDisposable
         /// </summary>
         private ImmutableArray<string> _currentAggregateCapabilities;
 
+        private readonly CancellationToken _shutdownCancellationToken;
+
         private bool _isDisposed;
 
-        public CompilationHandler(IReporter reporter)
+        public CompilationHandler(IReporter reporter, EnvironmentOptions environmentOptions, CancellationToken shutdownCancellationToken)
         {
             _reporter = reporter;
+            EnvironmentOptions = environmentOptions;
             Workspace = new IncrementalMSBuildWorkspace(reporter);
             _hotReloadService = new WatchHotReloadService(Workspace.CurrentSolution.Services, GetAggregateCapabilitiesAsync);
+            _shutdownCancellationToken = shutdownCancellationToken;
         }
 
         public void Dispose()
@@ -154,6 +159,7 @@ private static DeltaApplier CreateDeltaApplier(HotReloadProfile profile, Project
             var runningProject = new RunningProject(
                 projectNode,
                 projectOptions,
+                EnvironmentOptions,
                 deltaApplier,
                 processReporter,
                 browserRefreshServer,
@@ -542,16 +548,10 @@ private void UpdateRunningProjects(Func<ImmutableDictionary<string, ImmutableArr
             }
         }
 
-        private static async ValueTask<IReadOnlyList<int>> TerminateRunningProjects(IEnumerable<RunningProject> projects, CancellationToken cancellationToken)
+        private async ValueTask<IReadOnlyList<int>> TerminateRunningProjects(IEnumerable<RunningProject> projects, CancellationToken cancellationToken)
         {
-            // cancel first, this will cause the process tasks to complete:
-            foreach (var project in projects)
-            {
-                project.ProcessTerminationSource.Cancel();
-            }
-
             // wait for all tasks to complete:
-            return await Task.WhenAll(projects.Select(p => p.RunningProcess)).WaitAsync(cancellationToken);
+            return await Task.WhenAll(projects.Select(p => p.TerminateAsync(_shutdownCancellationToken).AsTask())).WaitAsync(cancellationToken);
         }
 
         private static Task ForEachProjectAsync(ImmutableDictionary<string, ImmutableArray<RunningProject>> projects, Func<RunningProject, CancellationToken, Task> action, CancellationToken cancellationToken)
diff --git a/src/BuiltInTools/dotnet-watch/HotReload/RunningProject.cs b/src/BuiltInTools/dotnet-watch/HotReload/RunningProject.cs
@@ -12,6 +12,7 @@ namespace Microsoft.DotNet.Watch
     internal sealed class RunningProject(
         ProjectGraphNode projectNode,
         ProjectOptions options,
+        EnvironmentOptions environmentOptions,
         DeltaApplier deltaApplier,
         IReporter reporter,
         BrowserRefreshServer? browserRefreshServer,
@@ -68,5 +69,24 @@ public async ValueTask WaitForProcessRunningAsync(CancellationToken cancellation
         {
             await DeltaApplier.WaitForProcessRunningAsync(cancellationToken);
         }
+
+        public async ValueTask<int> TerminateAsync(CancellationToken shutdownCancellationToken)
+        {
+            if (shutdownCancellationToken.IsCancellationRequested)
+            {
+                // Ctrl+C sent, wait for the process to exit
+                try
+                {
+                    _ = await RunningProcess.WaitAsync(environmentOptions.ProcessCleanupTimeout, CancellationToken.None);
+                }
+                catch (TimeoutException)
+                {
+                    // nop
+                }
+            }
+
+            ProcessTerminationSource.Cancel();
+            return await RunningProcess;
+        }
     }
 }
diff --git a/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs b/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs
@@ -72,7 +72,6 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke
                 var waitForFileChangeBeforeRestarting = true;
                 EvaluationResult? evaluationResult = null;
                 RunningProject? rootRunningProject = null;
-                Task<ImmutableList<ChangedFile>>? fileWatcherTask = null;
                 IRuntimeProcessLauncher? runtimeProcessLauncher = null;
                 CompilationHandler? compilationHandler = null;
                 Action<ChangedPath>? fileChangedCallback = null;
@@ -101,7 +100,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke
 
                     await using var browserConnector = new BrowserConnector(Context);
                     var projectMap = new ProjectNodeMap(evaluationResult.ProjectGraph, Context.Reporter);
-                    compilationHandler = new CompilationHandler(Context.Reporter);
+                    compilationHandler = new CompilationHandler(Context.Reporter, Context.EnvironmentOptions, shutdownCancellationToken);
                     var staticFileHandler = new StaticFileHandler(Context.Reporter, projectMap, browserConnector);
                     var scopedCssFileHandler = new ScopedCssFileHandler(Context.Reporter, projectMap, browserConnector);
                     var projectLauncher = new ProjectLauncher(Context, projectMap, browserConnector, compilationHandler, iteration);
@@ -517,38 +516,24 @@ async Task<ImmutableList<ChangedFile>> CaptureChangedFilesSnapshot(ImmutableDict
                         await compilationHandler.TerminateNonRootProcessesAndDispose(CancellationToken.None);
                     }
 
-                    if (!rootProcessTerminationSource.IsCancellationRequested)
+                    if (rootRunningProject != null)
                     {
-                        rootProcessTerminationSource.Cancel();
+                        await rootRunningProject.TerminateAsync(shutdownCancellationToken);
                     }
 
-                    try
-                    {
-                        // Wait for the root process to exit.
-                        await Task.WhenAll(new[] { (Task?)rootRunningProject?.RunningProcess, fileWatcherTask }.Where(t => t != null)!);
-                    }
-                    catch (OperationCanceledException) when (!shutdownCancellationToken.IsCancellationRequested)
+                    if (runtimeProcessLauncher != null)
                     {
-                        // nop
+                        await runtimeProcessLauncher.DisposeAsync();
                     }
-                    finally
-                    {
-                        fileWatcherTask = null;
 
-                        if (runtimeProcessLauncher != null)
-                        {
-                            await runtimeProcessLauncher.DisposeAsync();
-                        }
-
-                        rootRunningProject?.Dispose();
+                    rootRunningProject?.Dispose();
 
-                        if (waitForFileChangeBeforeRestarting &&
-                            !shutdownCancellationToken.IsCancellationRequested &&
-                            !forceRestartCancellationSource.IsCancellationRequested)
-                        {
-                            using var shutdownOrForcedRestartSource = CancellationTokenSource.CreateLinkedTokenSource(shutdownCancellationToken, forceRestartCancellationSource.Token);
-                            await WaitForFileChangeBeforeRestarting(fileWatcher, evaluationResult, shutdownOrForcedRestartSource.Token);
-                        }
+                    if (waitForFileChangeBeforeRestarting &&
+                        !shutdownCancellationToken.IsCancellationRequested &&
+                        !forceRestartCancellationSource.IsCancellationRequested)
+                    {
+                        using var shutdownOrForcedRestartSource = CancellationTokenSource.CreateLinkedTokenSource(shutdownCancellationToken, forceRestartCancellationSource.Token);
+                        await WaitForFileChangeBeforeRestarting(fileWatcher, evaluationResult, shutdownOrForcedRestartSource.Token);
                     }
                 }
             }
diff --git a/src/BuiltInTools/dotnet-watch/Internal/PhysicalConsole.cs b/src/BuiltInTools/dotnet-watch/Internal/PhysicalConsole.cs
@@ -17,27 +17,7 @@ internal sealed class PhysicalConsole : IConsole
         public PhysicalConsole(TestFlags testFlags)
         {
             Console.OutputEncoding = Encoding.UTF8;
-
-            bool readFromStdin;
-            if (testFlags.HasFlag(TestFlags.ReadKeyFromStdin))
-            {
-                readFromStdin = true;
-            }
-            else
-            {
-                try
-                {
-                    Console.TreatControlCAsInput = true;
-                    readFromStdin = false;
-                }
-                catch
-                {
-                    // fails when stdin is redirected
-                    readFromStdin = true;
-                }
-            }
-
-            _ = readFromStdin ? ListenToStandardInputAsync() : ListenToConsoleKeyPressAsync();
+            _ = testFlags.HasFlag(TestFlags.ReadKeyFromStdin) ? ListenToStandardInputAsync() : ListenToConsoleKeyPressAsync();
         }
 
         private async Task ListenToStandardInputAsync()
@@ -73,14 +53,22 @@ private async Task ListenToStandardInputAsync()
         }
 
         private Task ListenToConsoleKeyPressAsync()
-            => Task.Factory.StartNew(() =>
+        {
+            Console.CancelKeyPress += (s, e) =>
+            {
+                e.Cancel = true;
+                KeyPressed?.Invoke(new ConsoleKeyInfo(CtrlC, ConsoleKey.C, shift: false, alt: false, control: true));
+            };
+
+            return Task.Factory.StartNew(() =>
             {
                 while (true)
                 {
                     var key = Console.ReadKey(intercept: true);
                     KeyPressed?.Invoke(key);
                 }
             }, TaskCreationOptions.LongRunning);
+        }
 
         public TextWriter Error => Console.Error;
         public TextWriter Out => Console.Out;
diff --git a/src/BuiltInTools/dotnet-watch/Properties/launchSettings.json b/src/BuiltInTools/dotnet-watch/Properties/launchSettings.json
@@ -8,7 +8,8 @@
         "DOTNET_WATCH_DEBUG_SDK_DIRECTORY": "$(RepoRoot)artifacts\\bin\\redist\\$(Configuration)\\dotnet\\sdk\\$(Version)",
         "DCP_IDE_REQUEST_TIMEOUT_SECONDS": "100000",
         "DCP_IDE_NOTIFICATION_TIMEOUT_SECONDS": "100000",
-        "DCP_IDE_NOTIFICATION_KEEPALIVE_SECONDS": "100000"
+        "DCP_IDE_NOTIFICATION_KEEPALIVE_SECONDS": "100000",
+        "DOTNET_WATCH_PROCESS_CLEANUP_TIMEOUT_MS": "0"
       }
     }
   }
diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs
@@ -697,6 +697,8 @@ public async Task Aspire()
             App.AssertOutputContains("dotnet watch 🔥 Project baselines updated.");
             App.AssertOutputContains($"dotnet watch ⭐ Starting project: {serviceProjectPath}");
 
+            // Note: sending Ctrl+C via standard input is not the same as sending real Ctrl+C.
+            // The latter terminates the processes gracefully on Windows, so exit codes -1 are actually not reported.
             App.SendControlC();
 
             await App.AssertOutputLineStartsWith("dotnet watch 🛑 Shutdown requested. Press Ctrl+C again to force exit.");
@@ -711,8 +713,8 @@ public async Task Aspire()
             {
                 // Unix process may return exit code = 128 + SIGTERM
                 // Exited with error code 143
-                await App.AssertOutputLine(line => line.Contains($"[WatchAspire.ApiService ({tfm})] Exited"), failure: _ => false);
-                await App.AssertOutputLine(line => line.Contains($"[WatchAspire.AppHost ({tfm})] Exited"), failure: _ => false);
+                await App.AssertOutputLine(line => line.Contains($"[WatchAspire.ApiService ({tfm})] Exited"));
+                await App.AssertOutputLine(line => line.Contains($"[WatchAspire.AppHost ({tfm})] Exited"));
             }
 
             await App.AssertOutputLineStartsWith("dotnet watch ⭐ Waiting for server to shutdown ...");
diff --git a/test/dotnet-watch.Tests/HotReload/CompilationHandlerTests.cs b/test/dotnet-watch.Tests/HotReload/CompilationHandlerTests.cs
@@ -20,14 +20,16 @@ public async Task ReferenceOutputAssembly_False()
         var reporter = new TestReporter(Logger);
         var options = TestOptions.GetProjectOptions(["--project", hostProject]);
 
+        var environmentOptions = TestOptions.GetEnvironmentOptions(Environment.CurrentDirectory, "dotnet");
+
         var factory = new MSBuildFileSetFactory(
             rootProjectFile: options.ProjectPath,
             buildArguments: [],
-            environmentOptions: new EnvironmentOptions(Environment.CurrentDirectory, "dotnet"),
+            environmentOptions: environmentOptions,
             reporter);
 
         var projectGraph = factory.TryLoadProjectGraph(projectGraphRequired: false);
-        var handler = new CompilationHandler(reporter);
+        var handler = new CompilationHandler(reporter, environmentOptions, CancellationToken.None);
 
         await handler.Workspace.UpdateProjectConeAsync(hostProject, CancellationToken.None);
 
diff --git a/test/dotnet-watch.Tests/HotReload/RuntimeProcessLauncherTests.cs b/test/dotnet-watch.Tests/HotReload/RuntimeProcessLauncherTests.cs
@@ -100,7 +100,10 @@ private RunningWatcher StartWatcher(TestAsset testAsset, string[] args, string w
         var program = Program.TryCreate(
            TestOptions.GetCommandLineOptions(["--verbose", ..args, "--project", projectPath]),
            console,
-           TestOptions.GetEnvironmentOptions(workingDirectory, TestContext.Current.ToolsetUnderTest.DotNetHostPath, testAsset),
+           TestOptions.GetEnvironmentOptions(workingDirectory, TestContext.Current.ToolsetUnderTest.DotNetHostPath, testAsset) with
+           {
+               ProcessCleanupTimeout = TimeSpan.FromSeconds(0),
+           },
            reporter,
            out var errorCode);
 
diff --git a/test/dotnet-watch.Tests/MsBuildFileSetFactoryTest.cs b/test/dotnet-watch.Tests/MsBuildFileSetFactoryTest.cs
@@ -325,9 +325,7 @@ public async Task ProjectReferences_Graph()
                 .Path;
             var projectA = Path.Combine(testDirectory, "A", "A.csproj");
 
-            var options = new EnvironmentOptions(
-                MuxerPath: MuxerPath,
-                WorkingDirectory: testDirectory);
+            var options = TestOptions.GetEnvironmentOptions(workingDirectory: testDirectory, muxerPath: MuxerPath);
 
             var output = new List<string>();
             _reporter.OnProcessOutput += line => output.Add(line.Content);
@@ -386,9 +384,7 @@ public async Task MsbuildOutput()
 
             var project1Path = GetTestProjectPath(project1);
 
-            var options = new EnvironmentOptions(
-                MuxerPath: MuxerPath,
-                WorkingDirectory: Path.GetDirectoryName(project1Path)!);
+            var options = TestOptions.GetEnvironmentOptions(workingDirectory: Path.GetDirectoryName(project1Path)!, muxerPath: MuxerPath);
 
             var output = new List<string>();
             _reporter.OnProcessOutput += line => output.Add($"{(line.IsError ? "[stderr]" : "[stdout]")} {line.Content}");
@@ -408,10 +404,7 @@ private Task<EvaluationResult> Evaluate(TestAsset projectPath)
 
         private async Task<EvaluationResult> Evaluate(string projectPath)
         {
-            var options = new EnvironmentOptions(
-                MuxerPath: MuxerPath,
-                WorkingDirectory: Path.GetDirectoryName(projectPath)!);
-
+            var options = TestOptions.GetEnvironmentOptions(workingDirectory: Path.GetDirectoryName(projectPath)!, muxerPath: MuxerPath);
             var factory = new MSBuildFileSetFactory(projectPath, buildArguments: [], options, _reporter);
             var result = await factory.TryCreateAsync(requireProjectGraph: null, CancellationToken.None);
             Assert.NotNull(result);
diff --git a/test/dotnet-watch.Tests/Utilities/MockFileSetFactory.cs b/test/dotnet-watch.Tests/Utilities/MockFileSetFactory.cs
@@ -6,7 +6,7 @@ namespace Microsoft.DotNet.Watch.UnitTests;
 internal class MockFileSetFactory() : MSBuildFileSetFactory(
     rootProjectFile: "test.csproj",
     buildArguments: [],
-    new EnvironmentOptions(Environment.CurrentDirectory, "dotnet"),
+    TestOptions.GetEnvironmentOptions(Environment.CurrentDirectory, "dotnet"),
     NullReporter.Singleton)
 {
     public Func<EvaluationResult> TryCreateImpl;
diff --git a/test/dotnet-watch.Tests/Utilities/TestOptions.cs b/test/dotnet-watch.Tests/Utilities/TestOptions.cs
@@ -15,7 +15,8 @@ public static int GetTestPort()
     public static readonly ProjectOptions ProjectOptions = GetProjectOptions([]);
 
     public static EnvironmentOptions GetEnvironmentOptions(string workingDirectory = "", string muxerPath = "", TestAsset? asset = null)
-        => new(workingDirectory, muxerPath, TestFlags: TestFlags.RunningAsTest, TestOutput: asset != null ? GetWatchTestOutputPath(asset) : "");
+        // 0 timeout for process cleanup in tests. We can't send Ctrl+C, so process termination must be forced.
+        => new(workingDirectory, muxerPath, ProcessCleanupTimeout: TimeSpan.FromSeconds(0), TestFlags: TestFlags.RunningAsTest, TestOutput: asset != null ? GetWatchTestOutputPath(asset) : "");
 
     public static CommandLineOptions GetCommandLineOptions(string[] args)
         => CommandLineOptions.Parse(args, NullReporter.Singleton, TextWriter.Null, out _) ?? throw new InvalidOperationException();
diff --git a/test/dotnet-watch.Tests/Watch/Utilities/WatchableApp.cs b/test/dotnet-watch.Tests/Watch/Utilities/WatchableApp.cs

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,8 @@`
`8`	`8`	`"DOTNET_WATCH_DEBUG_SDK_DIRECTORY": "$(RepoRoot)artifacts\\bin\\redist\\$(Configuration)\\dotnet\\sdk\\$(Version)",`
`9`	`9`	`"DCP_IDE_REQUEST_TIMEOUT_SECONDS": "100000",`
`10`	`10`	`"DCP_IDE_NOTIFICATION_TIMEOUT_SECONDS": "100000",`
`11`		`- "DCP_IDE_NOTIFICATION_KEEPALIVE_SECONDS": "100000"`
	`11`	`+ "DCP_IDE_NOTIFICATION_KEEPALIVE_SECONDS": "100000",`
	`12`	`+ "DOTNET_WATCH_PROCESS_CLEANUP_TIMEOUT_MS": "0"`
`12`	`13`	`}`
`13`	`14`	`}`
`14`	`15`	`}`