Skip to content

Commit 56f5c8f

Browse files
lateralusXlateralusX
authored
Improve Ctrl+C support in dotnet-trace tool (#5519)
Changing the cmd line tooling implementation in the following commit: 6837b8f#diff-d33d6d23aba37ebcfc05462b2fa260136a11df1b03f129f1912a856c36a8d7f3 broke Ctrl+C handling in dotnet-trace, since it disabled the process termination handler leading to process termination without being able to close the session, creating a corrupt file. dotnet-trace and dotnet-dsrouter tooling needs better control of its Ctrl+C shutdown, so this commit implements a custom process termination handler and disable the default handler implemented by the cmd line tools. This handler makes sure to signal a cancelation token if one of the configured signals hits the process. This commit also fix an issue with the integrated execution of dsrouter from other tools. Since Ctrl+C gets passed to child processes, integrated dsrouter could stop before dotnet-trace was able to close the session causing corrupt trace file and exception being logged in dotnet-trace. To mitigate this, dsrouter implements a way to block signals only accepting shutdown sequence on redirected stdin and dotnet-trace will launch dsrouter in this mode. This makes the shutdown sequence deterministic making sure dsrouter won't stop before dotnet-trace completed any outstanding trace sessions.
1 parent 47b73d5 commit 56f5c8f

File tree

10 files changed

+242
-55
lines changed

10 files changed

+242
-55
lines changed

‎src/Tools/Common/Commands/Utils.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,17 @@ public static int FindProcessIdWithName(string name)
4242
// <summary>
4343
// Returns processId that matches the given dsrouter.
4444
// </summary>
45-
// <param name="dsrouter">dsroutercommand</param>
45+
// <param name="dsrouter">dsrouterCommand</param>
4646
// <returns>processId</returns>
47-
public static int LaunchDSRouterProcess(string dsroutercommand)
47+
public static int LaunchDSRouterProcess(string dsrouterCommand)
4848
{
4949
ConsoleColor currentColor = Console.ForegroundColor;
5050
Console.ForegroundColor = ConsoleColor.Yellow;
5151
Console.WriteLine("WARNING: dotnet-dsrouter is a development tool not intended for production environments.");
5252
Console.ForegroundColor = currentColor;
5353
Console.WriteLine("For finer control over the dotnet-dsrouter options, run it separately and connect to it using -p" + Environment.NewLine);
5454

55-
return DsRouterProcessLauncher.Launcher.Start(dsroutercommand, default);
55+
return DsRouterProcessLauncher.Launcher.Start(dsrouterCommand, default);
5656
}
5757

5858

‎src/Tools/Common/DsRouterProcessLauncher.cs

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ private static async Task ReadAndIgnoreAllStreamAsync(StreamReader streamToIgnor
3636

3737
private Process ChildProc => _childProc;
3838

39-
public int Start(string dsroutercommand, CancellationToken ct)
39+
public int Start(string dsrouterCommand, CancellationToken ct)
4040
{
4141
string toolsRoot = System.IO.Path.GetDirectoryName(System.Environment.ProcessPath);
4242
string dotnetDsrouterTool = "dotnet-dsrouter";
@@ -46,10 +46,13 @@ public int Start(string dsroutercommand, CancellationToken ct)
4646
dotnetDsrouterTool = Path.Combine(toolsRoot, dotnetDsrouterTool);
4747
}
4848

49+
// Block SIGINT and SIGQUIT in child process.
50+
dsrouterCommand += " --block-signals SIGINT;SIGQUIT";
51+
4952
_childProc = new Process();
5053

5154
_childProc.StartInfo.FileName = dotnetDsrouterTool;
52-
_childProc.StartInfo.Arguments = dsroutercommand;
55+
_childProc.StartInfo.Arguments = dsrouterCommand;
5356
_childProc.StartInfo.UseShellExecute = false;
5457
_childProc.StartInfo.RedirectStandardOutput = true;
5558
_childProc.StartInfo.RedirectStandardError = true;
@@ -78,12 +81,37 @@ public void Cleanup()
7881
{
7982
try
8083
{
81-
_childProc.Kill();
84+
_childProc.StandardInput.WriteLine("Q");
85+
_childProc.StandardInput.Flush();
86+
87+
_childProc.WaitForExit(1000);
88+
}
89+
// if process exited while we were trying to write to stdin, it can throw IOException
90+
catch (IOException) { }
91+
92+
try
93+
{
94+
if (!_childProc.HasExited)
95+
{
96+
_childProc.Kill();
97+
}
8298
}
8399
// if process exited while we were trying to kill it, it can throw IOE
84100
catch (InvalidOperationException) { }
85-
_stdOutTask.Wait();
86-
_stdErrTask.Wait();
101+
102+
try
103+
{
104+
_stdOutTask.Wait();
105+
}
106+
// Ignore any fault/cancel state of task.
107+
catch (AggregateException) { }
108+
109+
try
110+
{
111+
_stdErrTask.Wait();
112+
}
113+
// Ignore any fault/cancel state of task.
114+
catch (AggregateException) { }
87115
}
88116
}
89117
}

‎src/Tools/Common/ProcessTerminationHandler.cs

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.CommandLine;
6+
using System.Runtime.InteropServices;
7+
using System.Threading;
8+
using System.Threading.Tasks;
9+
10+
namespace Microsoft.Internal.Common.Utils
11+
{
12+
internal sealed class ProcessTerminationHandler : IDisposable
13+
{
14+
private bool _isDisposed;
15+
private CancellationTokenSource _cancellationTokenSource;
16+
private readonly PosixSignalRegistration _sigIntRegistration;
17+
private readonly PosixSignalRegistration _sigQuitRegistration;
18+
private readonly PosixSignalRegistration _sigTermRegistration;
19+
private bool _blockSIGINT;
20+
private bool _blockSIGTERM;
21+
private bool _blockSIGQUIT;
22+
23+
internal CancellationToken GetToken => _cancellationTokenSource.Token;
24+
25+
internal static async Task<int> InvokeAsync(ParseResult parseResult, string blockedSignals = "")
26+
{
27+
using ProcessTerminationHandler terminationHandler = ConfigureTerminationHandler(parseResult, blockedSignals);
28+
return await parseResult.InvokeAsync(terminationHandler.GetToken).ConfigureAwait(false);
29+
}
30+
31+
private static ProcessTerminationHandler ConfigureTerminationHandler(ParseResult parseResult, string blockedSignals)
32+
{
33+
// Use custom process terminate handler for the command line tool parse result.
34+
parseResult.Configuration.ProcessTerminationTimeout = null;
35+
return new ProcessTerminationHandler(blockedSignals);
36+
}
37+
38+
private ProcessTerminationHandler(string blockedSignals)
39+
{
40+
_cancellationTokenSource = new();
41+
42+
if (!string.IsNullOrEmpty(blockedSignals))
43+
{
44+
foreach (string signal in blockedSignals.Split(';'))
45+
{
46+
if (signal.Equals("SIGINT", StringComparison.InvariantCultureIgnoreCase))
47+
{
48+
_blockSIGINT = true;
49+
}
50+
else if (signal.Equals("SIGQUIT", StringComparison.InvariantCultureIgnoreCase))
51+
{
52+
_blockSIGQUIT = true;
53+
}
54+
else if (signal.Equals("SIGTERM", StringComparison.InvariantCultureIgnoreCase))
55+
{
56+
_blockSIGTERM = true;
57+
}
58+
}
59+
}
60+
61+
_sigIntRegistration = PosixSignalRegistration.Create(PosixSignal.SIGINT, OnPosixSignal);
62+
_sigQuitRegistration = PosixSignalRegistration.Create(PosixSignal.SIGQUIT, OnPosixSignal);
63+
_sigTermRegistration = PosixSignalRegistration.Create(PosixSignal.SIGTERM, OnPosixSignal);
64+
}
65+
66+
public void Dispose()
67+
{
68+
if (!_isDisposed)
69+
{
70+
_sigIntRegistration?.Dispose();
71+
_sigQuitRegistration?.Dispose();
72+
_sigTermRegistration?.Dispose();
73+
74+
GC.SuppressFinalize(this);
75+
}
76+
77+
_isDisposed = true;
78+
}
79+
80+
private void OnPosixSignal(PosixSignalContext context)
81+
{
82+
context.Cancel = true;
83+
84+
if (_blockSIGINT && context.Signal == PosixSignal.SIGINT)
85+
{
86+
return;
87+
}
88+
else if (_blockSIGQUIT && context.Signal == PosixSignal.SIGQUIT)
89+
{
90+
return;
91+
}
92+
else if (_blockSIGTERM && context.Signal == PosixSignal.SIGTERM)
93+
{
94+
return;
95+
}
96+
97+
Cancel();
98+
}
99+
100+
private void Cancel()
101+
{
102+
if (_cancellationTokenSource.IsCancellationRequested)
103+
{
104+
return;
105+
}
106+
107+
_cancellationTokenSource.Cancel();
108+
}
109+
}
110+
}

‎src/Tools/Common/ReversedServerHelpers/ReversedServerHelpers.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,20 @@ public void Cleanup()
128128
}
129129
// if process exited while we were trying to kill it, it can throw IOE
130130
catch (InvalidOperationException) { }
131-
_stdOutTask.Wait();
132-
_stdErrTask.Wait();
131+
132+
try
133+
{
134+
_stdOutTask.Wait();
135+
}
136+
// Ignore any fault/cancel state of task.
137+
catch (AggregateException) { }
138+
139+
try
140+
{
141+
_stdErrTask.Wait();
142+
}
143+
// Ignore any fault/cancel state of task.
144+
catch (AggregateException) { }
133145
}
134146
}
135147
}

‎src/Tools/dotnet-dsrouter/ADBTcpRouterFactory.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,11 @@ public override async Task Stop()
194194

195195
try
196196
{
197-
_portReverseTaskCancelToken.Cancel();
198-
await _portReverseTask.ConfigureAwait(false);
197+
if (_portReverseTaskCancelToken is not null && _portReverseTask is not null)
198+
{
199+
_portReverseTaskCancelToken.Cancel();
200+
await _portReverseTask.ConfigureAwait(false);
201+
}
199202
}
200203
catch { }
201204

@@ -265,8 +268,11 @@ public override void Stop()
265268
{
266269
try
267270
{
268-
_portForwardTaskCancelToken.Cancel();
269-
_portForwardTask.Wait();
271+
if (_portForwardTaskCancelToken is not null && _portForwardTask is not null)
272+
{
273+
_portForwardTaskCancelToken.Cancel();
274+
_portForwardTask.Wait();
275+
}
270276
}
271277
catch { }
272278

‎src/Tools/dotnet-dsrouter/DiagnosticsServerRouterCommands.cs

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,44 @@ protected SpecificRunnerBase(LogLevel logLevel)
7272
LogLevel = logLevel;
7373
}
7474

75+
protected static async Task WaitForQuitAsync(CancellationToken token)
76+
{
77+
if (!Console.IsInputRedirected)
78+
{
79+
while (!token.IsCancellationRequested)
80+
{
81+
if (Console.KeyAvailable)
82+
{
83+
ConsoleKey cmd = Console.ReadKey(true).Key;
84+
if (cmd == ConsoleKey.Q)
85+
{
86+
break;
87+
}
88+
}
89+
await Task.Delay(250, token).ConfigureAwait(false);
90+
}
91+
}
92+
else
93+
{
94+
await Task.Run(async() => {
95+
Memory<char> buffer = new char[1];
96+
while (!token.IsCancellationRequested)
97+
{
98+
int result = await Console.In.ReadAsync(buffer, token).ConfigureAwait(false);
99+
if (result != 0)
100+
{
101+
char key = buffer.Span[0];
102+
if (key == 'Q' || key == 'q')
103+
{
104+
break;
105+
}
106+
}
107+
await Task.Delay(250, token).ConfigureAwait(false);
108+
}
109+
}, token).ConfigureAwait(false);
110+
}
111+
}
112+
75113
public abstract void ConfigureLauncher(CancellationToken cancellationToken);
76114

77115
// The basic run loop: configure logging and the launcher, then create the router and run it until it exits or the user interrupts
@@ -91,26 +129,11 @@ public async Task<int> CommonRunLoop(Func<ILogger, DiagnosticsServerRouterRunner
91129
logger.LogInformation($"Starting dotnet-dsrouter using pid={pid}");
92130

93131
Task<int> routerTask = createRouterTask(logger, Launcher, linkedCancelToken);
132+
Task waitForQuitTask = WaitForQuitAsync(linkedCancelToken.Token);
94133

95-
while (!linkedCancelToken.IsCancellationRequested)
134+
if (!linkedCancelToken.IsCancellationRequested)
96135
{
97-
await Task.WhenAny(routerTask, Task.Delay(
98-
250,
99-
linkedCancelToken.Token)).ConfigureAwait(false);
100-
if (routerTask.IsCompleted)
101-
{
102-
break;
103-
}
104-
105-
if (!Console.IsInputRedirected && Console.KeyAvailable)
106-
{
107-
ConsoleKey cmd = Console.ReadKey(true).Key;
108-
if (cmd == ConsoleKey.Q)
109-
{
110-
cancelRouterTask.Cancel();
111-
break;
112-
}
113-
}
136+
await Task.WhenAny(routerTask, waitForQuitTask).ConfigureAwait(false);
114137
}
115138

116139
if (!routerTask.IsCompleted)

0 commit comments

Comments
 (0)