From c1f9c09ba90e7dc3bb971ef6bc29a601773dd31b Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Wed, 11 Jun 2025 17:01:54 -0700 Subject: [PATCH] Fix deadlock if an MSBuild task is writing to stdout When we switched over to communicating over a named pipe rather than stdin/stdout, we were still redirecting stdin and stdout. This had the side effect that if a build task was directly writing to standard out, the build would eventually deadlock since we weren't reading from the other side. I thought about simply not redirecting stdin/stdout, but I could imagine other problems might come up if we were to have multiple build hosts trying to share stdin/stdout. So now we'll log stdout the same way we log stderr, and explicitly close stdin so readers won't deadlock waiting for input. Fixes https://github.com/dotnet/roslyn/issues/78766 --- .../MSBuild/Core/MSBuild/BuildHostProcessManager.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Workspaces/MSBuild/Core/MSBuild/BuildHostProcessManager.cs b/src/Workspaces/MSBuild/Core/MSBuild/BuildHostProcessManager.cs index 2a85ccb6ed9d2..0d58b27a46171 100644 --- a/src/Workspaces/MSBuild/Core/MSBuild/BuildHostProcessManager.cs +++ b/src/Workspaces/MSBuild/Core/MSBuild/BuildHostProcessManager.cs @@ -398,7 +398,8 @@ public BuildHostProcess(Process process, string pipeName, ILoggerFactory? logger _process.EnableRaisingEvents = true; _process.Exited += Process_Exited; - _process.ErrorDataReceived += Process_ErrorDataReceived; + _process.OutputDataReceived += (_, e) => LogProcessOutput(e, "stdout"); + _process.ErrorDataReceived += (_, e) => LogProcessOutput(e, "stderr"); var pipeClient = NamedPipeUtil.CreateClient(".", pipeName, PipeDirection.InOut, PipeOptions.Asynchronous); pipeClient.Connect(TimeOutMsNewProcess); @@ -412,7 +413,11 @@ public BuildHostProcess(Process process, string pipeName, ILoggerFactory? logger _rpcClient.Disconnected += Process_Exited; BuildHost = new RemoteBuildHost(_rpcClient); - // Call this last so our type is fully constructed before we start firing events + // Close the standard input stream so that if any build tasks were to try reading from the console, they won't deadlock waiting for input. + _process.StandardInput.Close(); + + // Call Begin*ReadLine methods last so so our type is fully constructed before we start firing events. + _process.BeginOutputReadLine(); _process.BeginErrorReadLine(); } @@ -421,14 +426,14 @@ private void Process_Exited(object? sender, EventArgs e) Disconnected?.Invoke(this, EventArgs.Empty); } - private void Process_ErrorDataReceived(object sender, DataReceivedEventArgs e) + private void LogProcessOutput(DataReceivedEventArgs e, string outputName) { if (e.Data is not null) { lock (_processLogMessages) _processLogMessages.AppendLine(e.Data); - _logger?.LogTrace($"Message from Process: {e.Data}"); + _logger?.LogTrace($"Message on {outputName}: {e.Data}"); } }