From 1d0b2be78e0bca39f139af8596abaebef5b312fe Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 10 Feb 2021 11:55:29 -0800 Subject: [PATCH] Add --show-child-io feature (#1968) * Fix deadlock when redirecting large amounts of child process output, and add ability to not redirect child process behavior - Use a small set of tasks to consume and ignore arbitrary data passed through standard output/error - Add a new switch to dotnet-trace called --redirect-child-output which can be set to false to allow viewing data. (By default the option defaults to true, which may be confusing. I'd like some feedback on that. * Adjust dotnet-trace to add --show-child-output and improve exit code handling * Add testing * Update doc in repo * Fix issues identified in testing * Move logic which waited for process to exit to avoid premature Dispose operations * Code review feedback --- diagnostics.sln | 43 +++++++ documentation/design-docs/dotnet-tools.md | 4 +- documentation/dotnet-trace-instructions.md | 2 +- src/Tools/Common/Commands/Utils.cs | 29 ++++- .../ReversedServerHelpers.cs | 25 ++-- src/Tools/dotnet-counters/CounterMonitor.cs | 8 +- .../CommandLine/Commands/CollectCommand.cs | 109 +++++++++++++++--- .../ExitCodeTracee/ExitCodeTracee.csproj | 7 ++ src/tests/ExitCodeTracee/Program.cs | 19 +++ src/tests/dotnet-trace/ChildProcessTests.cs | 99 ++++++++++++++++ .../dotnet-trace/DotnetTrace.UnitTests.csproj | 2 + 11 files changed, 312 insertions(+), 35 deletions(-) create mode 100644 src/tests/ExitCodeTracee/ExitCodeTracee.csproj create mode 100644 src/tests/ExitCodeTracee/Program.cs create mode 100644 src/tests/dotnet-trace/ChildProcessTests.cs diff --git a/diagnostics.sln b/diagnostics.sln index 48f3535b4..72b615ddb 100644 --- a/diagnostics.sln +++ b/diagnostics.sln @@ -202,6 +202,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Diagnostics.Exten EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Diagnostics.DebugServices.UnitTests", "src\tests\Microsoft.Diagnostics.DebugServices.UnitTests\Microsoft.Diagnostics.DebugServices.UnitTests.csproj", "{064BC7DD-D44C-400E-9215-7546E092AB98}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ExitCodeTracee", "src\tests\ExitCodeTracee\ExitCodeTracee.csproj", "{61F73DD0-F346-4D7A-AB12-63415B4EEEE1}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Checked|Any CPU = Checked|Any CPU @@ -1659,6 +1661,46 @@ Global {064BC7DD-D44C-400E-9215-7546E092AB98}.RelWithDebInfo|x64.Build.0 = Release|Any CPU {064BC7DD-D44C-400E-9215-7546E092AB98}.RelWithDebInfo|x86.ActiveCfg = Release|Any CPU {064BC7DD-D44C-400E-9215-7546E092AB98}.RelWithDebInfo|x86.Build.0 = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Checked|Any CPU.ActiveCfg = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Checked|Any CPU.Build.0 = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Checked|ARM.ActiveCfg = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Checked|ARM.Build.0 = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Checked|ARM64.ActiveCfg = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Checked|ARM64.Build.0 = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Checked|x64.ActiveCfg = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Checked|x64.Build.0 = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Checked|x86.ActiveCfg = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Checked|x86.Build.0 = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Debug|Any CPU.Build.0 = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Debug|ARM.ActiveCfg = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Debug|ARM.Build.0 = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Debug|ARM64.ActiveCfg = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Debug|ARM64.Build.0 = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Debug|x64.ActiveCfg = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Debug|x64.Build.0 = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Debug|x86.ActiveCfg = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Debug|x86.Build.0 = Debug|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Release|Any CPU.ActiveCfg = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Release|Any CPU.Build.0 = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Release|ARM.ActiveCfg = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Release|ARM.Build.0 = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Release|ARM64.ActiveCfg = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Release|ARM64.Build.0 = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Release|x64.ActiveCfg = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Release|x64.Build.0 = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Release|x86.ActiveCfg = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.Release|x86.Build.0 = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.RelWithDebInfo|Any CPU.ActiveCfg = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.RelWithDebInfo|Any CPU.Build.0 = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.RelWithDebInfo|ARM.ActiveCfg = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.RelWithDebInfo|ARM.Build.0 = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.RelWithDebInfo|ARM64.ActiveCfg = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.RelWithDebInfo|ARM64.Build.0 = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.RelWithDebInfo|x64.ActiveCfg = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.RelWithDebInfo|x64.Build.0 = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.RelWithDebInfo|x86.ActiveCfg = Release|Any CPU + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1}.RelWithDebInfo|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -1711,6 +1753,7 @@ Global {EEC90A42-CDCD-4EE3-B47D-C109D604E7E2} = {41638A4C-0DAF-47ED-A774-ECBBAC0315D7} {5FC66A16-41E9-4D22-A44C-FEBB7DCCAAF8} = {19FAB78C-3351-4911-8F0C-8C6056401740} {064BC7DD-D44C-400E-9215-7546E092AB98} = {03479E19-3F18-49A6-910A-F5041E27E7C0} + {61F73DD0-F346-4D7A-AB12-63415B4EEEE1} = {03479E19-3F18-49A6-910A-F5041E27E7C0} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {46465737-C938-44FC-BE1A-4CE139EBB5E0} diff --git a/documentation/design-docs/dotnet-tools.md b/documentation/design-docs/dotnet-tools.md index 60de1e1f5..b66b5879d 100644 --- a/documentation/design-docs/dotnet-tools.md +++ b/documentation/design-docs/dotnet-tools.md @@ -311,7 +311,7 @@ COLLECT [--providers ] [--format ] - Collects a diagnostic trace from a currently running process + Collects a diagnostic trace from a currently running process or launch a child process and trace it. Append -- to the collect command to instruct the tool to run a command and trace it immediately. -p, --process-id The process to collect the trace from @@ -351,6 +351,8 @@ COLLECT --format The format of the output trace file. The default value is nettrace. + --show-child-io + Shows the input and output streams of a launched child process in the current console. Examples: diff --git a/documentation/dotnet-trace-instructions.md b/documentation/dotnet-trace-instructions.md index d3544f1e4..924b844b0 100644 --- a/documentation/dotnet-trace-instructions.md +++ b/documentation/dotnet-trace-instructions.md @@ -87,7 +87,7 @@ Press or to exit... You can stop collecting the trace by pressing `` or `` key. Doing this will also exit `hello.exe`. ### NOTE -* Launching `hello.exe` via dotnet-trace will make its input/output to be redirected and you won't be able to interact with its stdin/stdout. +* Launching `hello.exe` via dotnet-trace will redirect its input/output and you will not be able to interact with it on the console by default. Use the --show-child-io switch to interact with its stdin/stdout. * Exiting the tool via CTRL+C or SIGTERM will safely end both the tool and the child process. diff --git a/src/Tools/Common/Commands/Utils.cs b/src/Tools/Common/Commands/Utils.cs index 0d1ee6bb5..2205e9875 100644 --- a/src/Tools/Common/Commands/Utils.cs +++ b/src/Tools/Common/Commands/Utils.cs @@ -42,15 +42,35 @@ namespace Microsoft.Internal.Common.Utils } /// - /// A helper method for validating --process-id, --name, --diagnostic-port options for collect and monitor commands. + /// A helper method for validating --process-id, --name, --diagnostic-port options for collect with child process commands. + /// None of these options can be specified, so it checks for them and prints the appropriate error message. + /// + /// process ID + /// name + /// port + /// + public static bool ValidateArgumentsForChildProcess(int processId, string name, string port) + { + if (processId != 0 && name != null && !string.IsNullOrEmpty(port)) + { + Console.WriteLine("None of the --name, --process-id, or --diagnostic-port options may be specified when launching a child process."); + return false; + } + + return true; + } + + /// + /// A helper method for validating --process-id, --name, --diagnostic-port options for collect commands. /// Only one of these options can be specified, so it checks for duplicate options specified and if there is /// such duplication, it prints the appropriate error message. /// /// process ID /// name /// port + /// resolvedProcessId /// - public static bool ValidateArguments(int processId, string name, string port, out int resolvedProcessId) + public static bool ValidateArgumentsForAttach(int processId, string name, string port, out int resolvedProcessId) { resolvedProcessId = -1; if (processId == 0 && name == null && string.IsNullOrEmpty(port)) @@ -97,6 +117,11 @@ namespace Microsoft.Internal.Common.Utils return false; } } + else if (processId == 0) + { + Console.WriteLine("One of the --name, --process-id, or --diagnostic-port options must be specified when attaching to a process."); + return false; + } resolvedProcessId = processId; return true; } diff --git a/src/Tools/Common/ReversedServerHelpers/ReversedServerHelpers.cs b/src/Tools/Common/ReversedServerHelpers/ReversedServerHelpers.cs index 7cfba7b1a..f1a83e375 100644 --- a/src/Tools/Common/ReversedServerHelpers/ReversedServerHelpers.cs +++ b/src/Tools/Common/ReversedServerHelpers/ReversedServerHelpers.cs @@ -83,15 +83,19 @@ namespace Microsoft.Internal.Common.Utils return _childProc; } } - public bool Start(string diagnosticTransportName, CancellationToken ct) + public bool Start( string diagnosticTransportName, CancellationToken ct, bool showChildIO, bool printLaunchCommand) { _childProc.StartInfo.UseShellExecute = false; - _childProc.StartInfo.RedirectStandardOutput = true; - _childProc.StartInfo.RedirectStandardError = true; - _childProc.StartInfo.RedirectStandardInput = true; + _childProc.StartInfo.RedirectStandardOutput = !showChildIO; + _childProc.StartInfo.RedirectStandardError = !showChildIO; + _childProc.StartInfo.RedirectStandardInput = !showChildIO; _childProc.StartInfo.Environment.Add("DOTNET_DiagnosticPorts", $"{diagnosticTransportName}"); try { + if (printLaunchCommand) + { + Console.WriteLine($"Launching: {_childProc.StartInfo.FileName} {_childProc.StartInfo.Arguments}"); + } _childProc.Start(); } catch (Exception e) @@ -100,8 +104,11 @@ namespace Microsoft.Internal.Common.Utils Console.WriteLine(e.ToString()); return false; } - _stdOutTask = ReadAndIgnoreAllStreamAsync(_childProc.StandardOutput, ct); - _stdErrTask = ReadAndIgnoreAllStreamAsync(_childProc.StandardError, ct); + if (!showChildIO) + { + _stdOutTask = ReadAndIgnoreAllStreamAsync(_childProc.StandardOutput, ct); + _stdErrTask = ReadAndIgnoreAllStreamAsync(_childProc.StandardError, ct); + } return true; } @@ -183,7 +190,7 @@ namespace Microsoft.Internal.Common.Utils _timeoutInSec = timeoutInSec; } - public async Task Build(CancellationToken ct, int processId, string portName) + public async Task Build(CancellationToken ct, int processId, string portName, bool showChildIO, bool printLaunchCommand) { if (ProcessLauncher.Launcher.HasChildProc) { @@ -193,9 +200,9 @@ namespace Microsoft.Internal.Common.Utils server.Start(); // Start the child proc - if (!ProcessLauncher.Launcher.Start(diagnosticTransportName, ct)) + if (!ProcessLauncher.Launcher.Start(diagnosticTransportName, ct, showChildIO, printLaunchCommand)) { - throw new InvalidOperationException($"Failed to start {ProcessLauncher.Launcher.ChildProc.ProcessName}."); + throw new InvalidOperationException($"Failed to start '{ProcessLauncher.Launcher.ChildProc.StartInfo.FileName} {ProcessLauncher.Launcher.ChildProc.StartInfo.Arguments}'."); } IpcEndpointInfo endpointInfo; try diff --git a/src/Tools/dotnet-counters/CounterMonitor.cs b/src/Tools/dotnet-counters/CounterMonitor.cs index 04232cad9..5bedd0a84 100644 --- a/src/Tools/dotnet-counters/CounterMonitor.cs +++ b/src/Tools/dotnet-counters/CounterMonitor.cs @@ -92,13 +92,13 @@ namespace Microsoft.Diagnostics.Tools.Counters public async Task Monitor(CancellationToken ct, List counter_list, string counters, IConsole console, int processId, int refreshInterval, string name, string diagnosticPort) { - if (!ProcessLauncher.Launcher.HasChildProc && !CommandUtils.ValidateArguments(processId, name, diagnosticPort, out _processId)) + if (!ProcessLauncher.Launcher.HasChildProc && !CommandUtils.ValidateArgumentsForAttach(processId, name, diagnosticPort, out _processId)) { return 0; } DiagnosticsClientBuilder builder = new DiagnosticsClientBuilder("dotnet-counters", 10); - using (DiagnosticsClientHolder holder = await builder.Build(ct, _processId, diagnosticPort)) + using (DiagnosticsClientHolder holder = await builder.Build(ct, _processId, diagnosticPort, showChildIO: false, printLaunchCommand: false)) { try { @@ -130,12 +130,12 @@ namespace Microsoft.Diagnostics.Tools.Counters public async Task Collect(CancellationToken ct, List counter_list, string counters, IConsole console, int processId, int refreshInterval, CountersExportFormat format, string output, string name, string diagnosticPort) { - if (!ProcessLauncher.Launcher.HasChildProc && !CommandUtils.ValidateArguments(processId, name, diagnosticPort, out _processId)) + if (!ProcessLauncher.Launcher.HasChildProc && !CommandUtils.ValidateArgumentsForAttach(processId, name, diagnosticPort, out _processId)) { return 0; } DiagnosticsClientBuilder builder = new DiagnosticsClientBuilder("dotnet-counters", 10); - using (DiagnosticsClientHolder holder = await builder.Build(ct, _processId, diagnosticPort)) + using (DiagnosticsClientHolder holder = await builder.Build(ct, _processId, diagnosticPort, showChildIO: false, printLaunchCommand: false)) { try { diff --git a/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs b/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs index 841a7515d..0400bec2b 100644 --- a/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs +++ b/src/Tools/dotnet-trace/CommandLine/Commands/CollectCommand.cs @@ -21,10 +21,11 @@ namespace Microsoft.Diagnostics.Tools.Trace { internal static class CollectCommandHandler { - delegate Task CollectDelegate(CancellationToken ct, IConsole console, int processId, FileInfo output, uint buffersize, string providers, string profile, TraceFileFormat format, TimeSpan duration, string clrevents, string clreventlevel, string name, string port); + delegate Task CollectDelegate(CancellationToken ct, IConsole console, int processId, FileInfo output, uint buffersize, string providers, string profile, TraceFileFormat format, TimeSpan duration, string clrevents, string clreventlevel, string name, string port, bool showchildio); /// - /// Collects a diagnostic trace from a currently running process. + /// Collects a diagnostic trace from a currently running process or launch a child process and trace it. + /// Append -- to the collect command to instruct the tool to run a command and trace it immediately. By default the IO from this process is hidden, but the --show-child-io option may be used to show the child process IO. /// /// The cancellation token /// @@ -39,18 +40,48 @@ namespace Microsoft.Diagnostics.Tools.Trace /// A list of CLR events to be emitted. /// The verbosity level of CLR events /// Path to the diagnostic port to be created. + /// Should IO from a child process be hidden. /// - private static async Task Collect(CancellationToken ct, IConsole console, int processId, FileInfo output, uint buffersize, string providers, string profile, TraceFileFormat format, TimeSpan duration, string clrevents, string clreventlevel, string name, string diagnosticPort) + private static async Task Collect(CancellationToken ct, IConsole console, int processId, FileInfo output, uint buffersize, string providers, string profile, TraceFileFormat format, TimeSpan duration, string clrevents, string clreventlevel, string name, string diagnosticPort, bool showchildio) { int ret = 0; + bool collectionStopped = false; + bool cancelOnEnter = true; + bool cancelOnCtrlC = true; + bool printStatusOverTime = true; + try { Debug.Assert(output != null); Debug.Assert(profile != null); + if (ProcessLauncher.Launcher.HasChildProc && showchildio) + { + // If showing IO, then all IO (including CtrlC) behavior is delegated to the child process + cancelOnCtrlC = false; + cancelOnEnter = false; + printStatusOverTime = false; + } + else + { + cancelOnCtrlC = true; + cancelOnEnter = !Console.IsInputRedirected; + printStatusOverTime = !Console.IsInputRedirected; + } + + if (!cancelOnCtrlC) + { + ct = CancellationToken.None; + } + if (!ProcessLauncher.Launcher.HasChildProc) { - if (CommandUtils.ValidateArguments(processId, name, diagnosticPort, out int resolvedProcessId)) + if (showchildio) + { + Console.WriteLine("--show-child-io must not be specified when attaching to a process"); + return ErrorCodes.ArgumentError; + } + if (CommandUtils.ValidateArgumentsForAttach(processId, name, diagnosticPort, out int resolvedProcessId)) { processId = resolvedProcessId; } @@ -59,6 +90,10 @@ namespace Microsoft.Diagnostics.Tools.Trace return ErrorCodes.ArgumentError; } } + else if (!CommandUtils.ValidateArgumentsForChildProcess(processId, name, diagnosticPort)) + { + return ErrorCodes.ArgumentError; + } if (profile.Length == 0 && providers.Length == 0 && clrevents.Length == 0) { @@ -117,7 +152,7 @@ namespace Microsoft.Diagnostics.Tools.Trace DiagnosticsClientBuilder builder = new DiagnosticsClientBuilder("dotnet-trace", 10); bool shouldResumeRuntime = ProcessLauncher.Launcher.HasChildProc || !string.IsNullOrEmpty(diagnosticPort); - using (DiagnosticsClientHolder holder = await builder.Build(ct, processId, diagnosticPort)) + using (DiagnosticsClientHolder holder = await builder.Build(ct, processId, diagnosticPort, showChildIO: showchildio, printLaunchCommand: true)) { diagnosticsClient = holder.Client; if (shouldResumeRuntime) @@ -135,7 +170,7 @@ namespace Microsoft.Diagnostics.Tools.Trace ct.Register(() => shouldExit.Set()); - using (VirtualTerminalMode vTermMode = VirtualTerminalMode.TryEnable()) + using (VirtualTerminalMode vTermMode = printStatusOverTime ? VirtualTerminalMode.TryEnable() : null) { EventPipeSession session = null; try @@ -179,9 +214,10 @@ namespace Microsoft.Diagnostics.Tools.Trace Console.Out.WriteLine("\n\n"); var fileInfo = new FileInfo(output.FullName); - Task copyTask = session.EventStream.CopyToAsync(fs).ContinueWith((task) => shouldExit.Set()); + Task copyTask = session.EventStream.CopyToAsync(fs); + Task shouldExitTask = copyTask.ContinueWith((task) => shouldExit.Set()); - if (!Console.IsOutputRedirected) + if (printStatusOverTime) { rewriter = new LineRewriter { LineToClear = Console.CursorTop - 1 }; Console.CursorVisible = false; @@ -189,7 +225,7 @@ namespace Microsoft.Diagnostics.Tools.Trace Action printStatus = () => { - if (!Console.IsOutputRedirected) + if (printStatusOverTime) { rewriter?.RewriteConsoleLine(); fileInfo.Refresh(); @@ -201,7 +237,7 @@ namespace Microsoft.Diagnostics.Tools.Trace Console.Out.WriteLine("Stopping the trace. This may take up to minutes depending on the application being traced."); }; - while (!shouldExit.WaitOne(100) && !(!Console.IsInputRedirected && Console.KeyAvailable && Console.ReadKey(true).Key == ConsoleKey.Enter)) + while (!shouldExit.WaitOne(100) && !(cancelOnEnter && Console.KeyAvailable && Console.ReadKey(true).Key == ConsoleKey.Enter)) printStatus(); // if the CopyToAsync ended early (target program exited, etc.), the we don't need to stop the session. @@ -210,12 +246,13 @@ namespace Microsoft.Diagnostics.Tools.Trace // Behavior concerning Enter moving text in the terminal buffer when at the bottom of the buffer // is different between Console/Terminals on Windows and Mac/Linux if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && - !Console.IsOutputRedirected && + printStatusOverTime && rewriter != null && Math.Abs(Console.CursorTop - Console.BufferHeight) == 1) { rewriter.LineToClear--; } + collectionStopped = true; durationTimer?.Stop(); rundownRequested = true; session.Stop(); @@ -225,29 +262,57 @@ namespace Microsoft.Diagnostics.Tools.Trace printStatus(); } while (!copyTask.Wait(100)); } + // At this point the copyTask will have finished, so wait on the shouldExitTask in case it threw + // an exception or had some other interesting behavior + shouldExitTask.Wait(); } - Console.Out.WriteLine("\nTrace completed."); + Console.Out.WriteLine($"\nTrace completed."); if (format != TraceFileFormat.NetTrace) TraceFileFormatConverter.ConvertToFormat(format, output.FullName); } + + if (!collectionStopped && !ct.IsCancellationRequested) + { + // If the process is shutting down by itself print the return code from the process. + // Capture this before leaving the using, as the Dispose of the DiagnosticsClientHolder + // may terminate the target process causing it to have the wrong error code + if (ProcessLauncher.Launcher.ChildProc.WaitForExit(5000)) + { + ret = ProcessLauncher.Launcher.ChildProc.ExitCode; + Console.WriteLine($"Process exited with code '{ret}'."); + collectionStopped = true; + } + } } } catch (Exception ex) { Console.Error.WriteLine($"[ERROR] {ex.ToString()}"); ret = ErrorCodes.TracingError; + collectionStopped = true; } finally { - if (console.GetTerminal() != null) - Console.CursorVisible = true; + if (printStatusOverTime) + { + if (console.GetTerminal() != null) + Console.CursorVisible = true; + } - // If we launched a child proc that hasn't exited yet, terminate it before we exit. - if (ProcessLauncher.Launcher.HasChildProc && !ProcessLauncher.Launcher.ChildProc.HasExited) + if (ProcessLauncher.Launcher.HasChildProc) { - ProcessLauncher.Launcher.ChildProc.Kill(); + if (!collectionStopped || ct.IsCancellationRequested) + { + ret = ErrorCodes.TracingError; + } + + // If we launched a child proc that hasn't exited yet, terminate it before we exit. + if (!ProcessLauncher.Launcher.ChildProc.HasExited) + { + ProcessLauncher.Launcher.ChildProc.Kill(); + } } } return await Task.FromResult(ret); @@ -284,7 +349,7 @@ namespace Microsoft.Diagnostics.Tools.Trace public static Command CollectCommand() => new Command( name: "collect", - description: "Collects a diagnostic trace from a currently running process") + description: "Collects a diagnostic trace from a currently running process or launch a child process and trace it. Append -- to the collect command to instruct the tool to run a command and trace it immediately. When tracing a child process, the exit code of dotnet-trace shall be that of the traced process unless the trace process encounters an error.") { // Handler HandlerDescriptor.FromDelegate((CollectDelegate)Collect).GetCommandHandler(), @@ -300,6 +365,7 @@ namespace Microsoft.Diagnostics.Tools.Trace CLREventLevelOption(), CommonOptions.NameOption(), DiagnosticPortOption(), + ShowChildIOOption() }; private static uint DefaultCircularBufferSizeInMB() => 256; @@ -375,5 +441,12 @@ namespace Microsoft.Diagnostics.Tools.Trace { Argument = new Argument(name: "diagnosticPort", getDefaultValue: () => string.Empty) }; + private static Option ShowChildIOOption() => + new Option( + alias: "--show-child-io", + description: @"Shows the input and output streams of a launched child process in the current console.") + { + Argument = new Argument(name: "show-child-io", getDefaultValue: () => false) + }; } } diff --git a/src/tests/ExitCodeTracee/ExitCodeTracee.csproj b/src/tests/ExitCodeTracee/ExitCodeTracee.csproj new file mode 100644 index 000000000..cd0510ad2 --- /dev/null +++ b/src/tests/ExitCodeTracee/ExitCodeTracee.csproj @@ -0,0 +1,7 @@ + + + Exe + $(BuildProjectFramework) + netcoreapp3.1;net5.0 + + diff --git a/src/tests/ExitCodeTracee/Program.cs b/src/tests/ExitCodeTracee/Program.cs new file mode 100644 index 000000000..b58526ba0 --- /dev/null +++ b/src/tests/ExitCodeTracee/Program.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; + +namespace Tracee +{ + class Program + { + static int Main(string[] args) + { + foreach (string s in args) + Console.WriteLine(s); + + return Int32.Parse(args[0]); + } + } +} diff --git a/src/tests/dotnet-trace/ChildProcessTests.cs b/src/tests/dotnet-trace/ChildProcessTests.cs new file mode 100644 index 000000000..0d48d8802 --- /dev/null +++ b/src/tests/dotnet-trace/ChildProcessTests.cs @@ -0,0 +1,99 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.Diagnostics.NETCore.Client; +using System; +using Xunit; +using Xunit.Abstractions; +using System.Collections.Generic; +using System.Linq; +using System.Diagnostics; + +namespace Microsoft.Diagnostics.Tools.Trace +{ + + public class ChildProcessTests + { + // Pass ITestOutputHelper into the test class, which xunit provides per-test + public ChildProcessTests(ITestOutputHelper outputHelper) + { + OutputHelper = outputHelper; + } + + private ITestOutputHelper OutputHelper { get; } + + private void LaunchDotNetTrace(string command, out int exitCode, out string stdOut, out string stdErr) + { + string dotnetTracePathWithArgs = CommonHelper.GetTraceePathWithArgs(traceeName: "dotnet-trace").Replace("net5.0", "netcoreapp2.1"); + ProcessStartInfo startInfo = new ProcessStartInfo(CommonHelper.HostExe, $"{dotnetTracePathWithArgs} {command}"); + + OutputHelper.WriteLine($"Launching: {startInfo.FileName} {startInfo.Arguments}"); + startInfo.RedirectStandardInput = true; + startInfo.RedirectStandardError = true; + startInfo.RedirectStandardOutput = true; + + using (Process process = Process.Start(startInfo)) + { + const int processTimeout = 15000; + bool processExitedCleanly = process.WaitForExit(processTimeout); + if (!processExitedCleanly) + { + OutputHelper.WriteLine($"Forced kill of process after {processTimeout}ms"); + process.Kill(); + } + + OutputHelper.WriteLine("StdErr"); + stdErr = process.StandardError.ReadToEnd(); + OutputHelper.WriteLine(stdErr); + OutputHelper.WriteLine("StdOut"); + stdOut = process.StandardOutput.ReadToEnd(); + OutputHelper.WriteLine(stdOut); + + Assert.True(processExitedCleanly, "Launched process failed to exit"); + exitCode = process.ExitCode; + } + } + + [Theory] + [InlineData("232", 232)] + [InlineData("0", 0)] + public void VerifyExitCode(string commandLineArg, int exitCode) + { + string exitCodeTraceePath = CommonHelper.GetTraceePathWithArgs(traceeName: "ExitCodeTracee", targetFramework: "net5.0"); + + LaunchDotNetTrace($"collect -o verifyexitcode.nettrace -- {CommonHelper.HostExe} {exitCodeTraceePath} {commandLineArg}", out int dotnetTraceExitCode, out string stdOut, out string stdErr); + Assert.Equal(exitCode, dotnetTraceExitCode); + + Assert.Contains($"Process exited with code '{exitCode}'.", stdOut); + } + + [Theory] + [InlineData("0 this is a message", new string[] { "\nthis\n", "\nis\n", "\na\n" })] + public void VerifyHideIO(string commandLineArg, string[] stringsInOutput) + { + string exitCodeTraceePath = CommonHelper.GetTraceePathWithArgs(traceeName: "ExitCodeTracee", targetFramework: "net5.0"); + + LaunchDotNetTrace($"collect -o VerifyHideIO.nettrace -- {CommonHelper.HostExe} {exitCodeTraceePath} {commandLineArg}", out int dotnetTraceExitCode, out string stdOut, out string stdErr); + Assert.Equal(0, dotnetTraceExitCode); + stdOut = stdOut.Replace("\r", ""); + + foreach (string s in stringsInOutput) + Assert.DoesNotContain(s, stdOut); + } + + [Theory] + [InlineData("0 this is a message", new string[] { "\nthis\n", "\nis\n", "\na\n" })] + public void VerifyShowIO(string commandLineArg, string[] stringsInOutput) + { + string exitCodeTraceePath = CommonHelper.GetTraceePathWithArgs(traceeName: "ExitCodeTracee", targetFramework: "net5.0"); + + LaunchDotNetTrace($"collect -o VerifyShowIO.nettrace --show-child-io -- {CommonHelper.HostExe} {exitCodeTraceePath} {commandLineArg}", out int dotnetTraceExitCode, out string stdOut, out string stdErr); + Assert.Equal(0, dotnetTraceExitCode); + stdOut = stdOut.Replace("\r", ""); + + foreach (string s in stringsInOutput) + Assert.Contains(s, stdOut); + } + } +} diff --git a/src/tests/dotnet-trace/DotnetTrace.UnitTests.csproj b/src/tests/dotnet-trace/DotnetTrace.UnitTests.csproj index 2cc2d8618..5c99730da 100644 --- a/src/tests/dotnet-trace/DotnetTrace.UnitTests.csproj +++ b/src/tests/dotnet-trace/DotnetTrace.UnitTests.csproj @@ -6,6 +6,8 @@ + + -- 2.34.1