Skip to content

Commit

Permalink
Prevent propagation of Ctrl+C to child processes
Browse files Browse the repository at this point in the history
  • Loading branch information
tmat committed Oct 31, 2024
1 parent d26558d commit 76877d6
Show file tree
Hide file tree
Showing 10 changed files with 144 additions and 99 deletions.
6 changes: 6 additions & 0 deletions src/BuiltInTools/dotnet-watch/EnvironmentOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ internal enum TestFlags
/// Elevates the severity of <see cref="MessageDescriptor.WaitingForChanges"/> from <see cref="MessageSeverity.Output"/>.
/// </summary>
ElevateWaitingForChangesMessageSeverity = 1 << 2,

/// <summary>
/// Instead of using <see cref="Console.ReadKey()"/> to watch for Ctrl+C, Ctlr+R, and other keys, read from standard input.
/// This allows tests to trigger key based events.
/// </summary>
ReadKeyFromStdin = 1 << 3,
}

internal sealed record EnvironmentOptions(
Expand Down
13 changes: 6 additions & 7 deletions src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke

_console.KeyPressed += (key) =>
{
var modifiers = ConsoleModifiers.Control;
if ((key.Modifiers & modifiers) == modifiers && key.Key == ConsoleKey.R && forceRestartCancellationSource is { } source)
if (key.Modifiers.HasFlag(ConsoleModifiers.Control) && key.Key == ConsoleKey.R && forceRestartCancellationSource is { } source)
{
// provide immediate feedback to the user:
Context.Reporter.Report(source.IsCancellationRequested ? MessageDescriptor.RestartInProgress : MessageDescriptor.RestartRequested);
Expand Down Expand Up @@ -327,11 +326,6 @@ await Task.WhenAll(
}
finally
{
if (!rootProcessTerminationSource.IsCancellationRequested)
{
rootProcessTerminationSource.Cancel();
}

if (runtimeProcessLauncher != null)
{
// Request cleanup of all processes created by the launcher before we terminate the root process.
Expand All @@ -345,6 +339,11 @@ await Task.WhenAll(
await compilationHandler.TerminateNonRootProcessesAndDispose(CancellationToken.None);
}

if (!rootProcessTerminationSource.IsCancellationRequested)
{
rootProcessTerminationSource.Cancel();
}

try
{
// Wait for the root process to exit.
Expand Down
5 changes: 0 additions & 5 deletions src/BuiltInTools/dotnet-watch/Internal/IConsole.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,9 @@ namespace Microsoft.Extensions.Tools.Internal
/// </summary>
internal interface IConsole
{
event ConsoleCancelEventHandler CancelKeyPress;
event Action<ConsoleKeyInfo> KeyPressed;
TextWriter Out { get; }
TextWriter Error { get; }
TextReader In { get; }
bool IsInputRedirected { get; }
bool IsOutputRedirected { get; }
bool IsErrorRedirected { get; }
ConsoleColor ForegroundColor { get; set; }
void ResetColor();
void Clear();
Expand Down
83 changes: 57 additions & 26 deletions src/BuiltInTools/dotnet-watch/Internal/PhysicalConsole.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.DotNet.Watcher;

namespace Microsoft.Extensions.Tools.Internal
{
/// <summary>
Expand All @@ -9,52 +11,81 @@ namespace Microsoft.Extensions.Tools.Internal
/// </summary>
internal sealed class PhysicalConsole : IConsole
{
private readonly List<Action<ConsoleKeyInfo>> _keyPressedListeners = new();
public const char CtrlC = '\x03';
public const char CtrlR = '\x12';

public event Action<ConsoleKeyInfo>? KeyPressed;

private PhysicalConsole()
public PhysicalConsole(TestFlags testFlags)
{
Console.OutputEncoding = Encoding.UTF8;
Console.CancelKeyPress += (o, e) =>

bool readFromStdin;
if (testFlags.HasFlag(TestFlags.ReadKeyFromStdin))
{
CancelKeyPress?.Invoke(o, e);
};
readFromStdin = true;
}
else
{
try
{
Console.TreatControlCAsInput = true;
readFromStdin = false;
}
catch
{
// fails when stdin is redirected
readFromStdin = true;
}
}

_ = readFromStdin ? ListenToStandardInputAsync() : ListenToConsoleKeyPressAsync();
}

public event Action<ConsoleKeyInfo> KeyPressed
private async Task ListenToStandardInputAsync()
{
add
using var stream = Console.OpenStandardInput();
var buffer = new byte[1];

while (true)
{
_keyPressedListeners.Add(value);
ListenToConsoleKeyPress();
}
var bytesRead = await stream.ReadAsync(buffer, CancellationToken.None);
if (bytesRead != 1)
{
break;
}

var c = (char)buffer[0];

remove => _keyPressedListeners.Remove(value);
// handle all input keys that watcher might consume:
var key = c switch
{
CtrlC => new ConsoleKeyInfo('C', ConsoleKey.C, shift: false, alt: false, control: true),
CtrlR => new ConsoleKeyInfo('R', ConsoleKey.R, shift: false, alt: false, control: true),
>= 'A' and <= 'Z' => new ConsoleKeyInfo(c, ConsoleKey.A + (c - 'A'), shift: false, alt: false, control: false),
_ => default
};

if (key.Key != ConsoleKey.None)
{
KeyPressed?.Invoke(key);
}
}
}

private void ListenToConsoleKeyPress()
{
Task.Factory.StartNew(() =>
private Task ListenToConsoleKeyPressAsync()
=> Task.Factory.StartNew(() =>
{
while (true)
{
var key = Console.ReadKey(intercept: true);
for (var i = 0; i < _keyPressedListeners.Count; i++)
{
_keyPressedListeners[i](key);
}
KeyPressed?.Invoke(key);
}
}, TaskCreationOptions.LongRunning);
}

public static IConsole Singleton { get; } = new PhysicalConsole();

public event ConsoleCancelEventHandler? CancelKeyPress;
public TextWriter Error => Console.Error;
public TextReader In => Console.In;
public TextWriter Out => Console.Out;
public bool IsInputRedirected => Console.IsInputRedirected;
public bool IsOutputRedirected => Console.IsOutputRedirected;
public bool IsErrorRedirected => Console.IsErrorRedirected;

public ConsoleColor ForegroundColor
{
get => Console.ForegroundColor;
Expand Down
52 changes: 31 additions & 21 deletions src/BuiltInTools/dotnet-watch/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.


using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.Loader;
using Microsoft.Build.Graph;
Expand Down Expand Up @@ -36,10 +37,12 @@ public static async Task<int> Main(string[] args)
// Register listeners that load Roslyn-related assemblies from the `Roslyn/bincore` directory.
RegisterAssemblyResolutionEvents(sdkRootDirectory);

var environmentOptions = EnvironmentOptions.FromEnvironment();

var program = TryCreate(
args,
PhysicalConsole.Singleton,
EnvironmentOptions.FromEnvironment(),
new PhysicalConsole(environmentOptions.TestFlags),
environmentOptions,
EnvironmentVariables.VerboseCliOutput,
out var exitCode);

Expand Down Expand Up @@ -77,6 +80,11 @@ public static async Task<int> Main(string[] args)
var workingDirectory = environmentOptions.WorkingDirectory;
reporter.Verbose($"Working directory: '{workingDirectory}'");

if (environmentOptions.TestFlags != TestFlags.None)
{
reporter.Verbose($"Test flags: {environmentOptions.TestFlags}");
}

string projectPath;
try
{
Expand All @@ -97,9 +105,28 @@ public static async Task<int> Main(string[] args)
// internal for testing
internal async Task<int> RunAsync()
{
var shutdownCancellationSourceDisposed = false;
var shutdownCancellationSource = new CancellationTokenSource();
var shutdownCancellationToken = shutdownCancellationSource.Token;
console.CancelKeyPress += OnCancelKeyPress;

console.KeyPressed += key =>
{
if (!shutdownCancellationSourceDisposed && key.Modifiers.HasFlag(ConsoleModifiers.Control) && key.Key == ConsoleKey.C)
{
// if we already canceled, we force immediate shutdown:
var forceShutdown = shutdownCancellationSource.IsCancellationRequested;

if (!forceShutdown)
{
reporter.Report(MessageDescriptor.ShutdownRequested);
shutdownCancellationSource.Cancel();
}
else
{
Environment.Exit(0);
}
}
};

try
{
Expand Down Expand Up @@ -130,26 +157,9 @@ internal async Task<int> RunAsync()
}
finally
{
console.CancelKeyPress -= OnCancelKeyPress;
shutdownCancellationSourceDisposed = true;
shutdownCancellationSource.Dispose();
}

void OnCancelKeyPress(object? sender, ConsoleCancelEventArgs args)
{
// if we already canceled, we force immediate shutdown:
var forceShutdown = shutdownCancellationSource.IsCancellationRequested;

if (!forceShutdown)
{
reporter.Report(MessageDescriptor.ShutdownRequested);
shutdownCancellationSource.Cancel();
args.Cancel = true;
}
else
{
Environment.Exit(0);
}
}
}

// internal for testing
Expand Down
28 changes: 10 additions & 18 deletions test/dotnet-watch.Tests/ConsoleReporterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,9 @@ private class TestConsole : IConsole
{
private readonly StringBuilder _out;
private readonly StringBuilder _error;

event Action<ConsoleKeyInfo> IConsole.KeyPressed
{
add { }
remove { }
}
public TextWriter Out { get; }
public TextWriter Error { get; }
public ConsoleColor ForegroundColor { get; set; }

public TestConsole()
{
Expand All @@ -82,14 +79,17 @@ public TestConsole()
Error = new StringWriter(_error);
}

event ConsoleCancelEventHandler IConsole.CancelKeyPress
event Action<ConsoleKeyInfo> IConsole.KeyPressed
{
add { }
remove { }
}

public string GetOutput() => _out.ToString();
public string GetError() => _error.ToString();
public string GetOutput()
=> _out.ToString();

public string GetError()
=> _error.ToString();

public void Clear()
{
Expand All @@ -99,16 +99,8 @@ public void Clear()

public void ResetColor()
{
ForegroundColor = default(ConsoleColor);
ForegroundColor = default;
}

public TextWriter Out { get; }
public TextWriter Error { get; }
public TextReader In { get; }
public bool IsInputRedirected { get; }
public bool IsOutputRedirected { get; }
public bool IsErrorRedirected { get; }
public ConsoleColor ForegroundColor { get; set; }
}
}
}
14 changes: 13 additions & 1 deletion test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ public async Task Aspire()
Assert.Equal(0, result.ExitCode);

var serviceSourcePath = Path.Combine(testAsset.Path, "WatchAspire.ApiService", "Program.cs");
App.Start(testAsset, ["-lp", "http"], relativeProjectDirectory: "WatchAspire.AppHost");
App.Start(testAsset, ["-lp", "http"], relativeProjectDirectory: "WatchAspire.AppHost", testFlags: TestFlags.ReadKeyFromStdin);

await App.AssertWaitingForChanges();

Expand All @@ -409,6 +409,18 @@ public async Task Aspire()

// Only one browser should be launched (dashboard). The child process shouldn't launch a browser.
Assert.Equal(1, App.Process.Output.Count(line => line.StartsWith("dotnet watch ⌚ Launching browser: ")));

App.SendControlC();

await App.AssertOutputLineStartsWith("dotnet watch 🛑 Shutdown requested. Press Ctrl+C again to force exit.");

await App.AssertOutputLineStartsWith("dotnet watch ❌ [WatchAspire.ApiService (net9.0)] Exited");
await App.AssertOutputLineStartsWith("dotnet watch ❌ [WatchAspire.AppHost (net9.0)] Exited");

await App.AssertOutputLineStartsWith("dotnet watch ⭐ Waiting for server to shutdown ...");

App.AssertOutputContains("dotnet watch ⭐ Stop session #1");
App.AssertOutputContains("dotnet watch ⭐ [#1] Sending 'sessionTerminated'");
}
}
}
13 changes: 0 additions & 13 deletions test/dotnet-watch.Tests/Utilities/TestConsole.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ namespace Microsoft.Extensions.Tools.Internal
{
internal class TestConsole : IConsole
{
public event ConsoleCancelEventHandler? CancelKeyPress;
public event Action<ConsoleKeyInfo>? KeyPressed;

private readonly TestOutputWriter _testWriter;
Expand Down Expand Up @@ -37,18 +36,6 @@ public void PressKey(ConsoleKeyInfo key)
KeyPressed.Invoke(key);
}

public void PressCancelKey()
{
Assert.NotNull(CancelKeyPress);

var ctor = typeof(ConsoleCancelEventArgs)
.GetTypeInfo()
.DeclaredConstructors
.Single(c => c.GetParameters().First().ParameterType == typeof(ConsoleSpecialKey));

CancelKeyPress.Invoke(this, (ConsoleCancelEventArgs)ctor.Invoke([ConsoleSpecialKey.ControlC]));
}

public void ResetColor()
{
}
Expand Down
Loading

0 comments on commit 76877d6

Please sign in to comment.