From 37c3e5cb3f75fc4cf3de2fc56b184b028673e678 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Wed, 13 May 2026 19:22:06 +0200 Subject: [PATCH] Server: Restore Console color/encoding at end of each request (LOG-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Snapshot Console.ForegroundColor, Console.BackgroundColor, and Console.OutputEncoding once at server startup (in OutOfProcServerNode.Run, before the listener loop), then restore them in a try/finally around each HandleServerNodeBuildCommand invocation. Why --- Without this, console state can leak across requests in a reused server: - BaseConsoleLogger.SetColor() mutates Console.ForegroundColor and only sometimes restores via ResetColor() (e.g. an errored build leaves the console red). - The per-request application of ConsoleConfiguration.BackgroundColor at line 414-417 is asymmetric: set per request, never restored. - OutputEncoding can be mutated by tasks/loggers (e.g. for non-ASCII output), and the next request inherits whatever the prior build left. Symmetric snapshot/restore mirrors the existing working-directory restore pattern in HandleShutdown. Implementation -------------- - Snapshot is taken once in Run() in try/catch since Console.ForegroundColor and Console.BackgroundColor throw on redirected/headless stdout (CI without a TTY). - _originalConsoleColorsCaptured / _originalOutputEncodingCaptured flags track whether the snapshot succeeded so restore is skipped if the platform doesn't support the property. - Restore is wrapped in try/catch for the same headless-CI scenario. Investigation context -------------------- Investigation #9379 LOG-2 / W1c logger lifecycle audit. One of three small server state-hygiene PRs: - #13756 (companion) — CurrentCulture/CurrentUICulture restore - (this) — Console color/encoding restore - (next) — -mt implies server with explicit MSBUILDUSESERVER=0 opt-out All 9 MSBuildServer_Tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Build/BackEnd/Node/OutOfProcServerNode.cs | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/Build/BackEnd/Node/OutOfProcServerNode.cs b/src/Build/BackEnd/Node/OutOfProcServerNode.cs index 3abc9faaaab..50b967db660 100644 --- a/src/Build/BackEnd/Node/OutOfProcServerNode.cs +++ b/src/Build/BackEnd/Node/OutOfProcServerNode.cs @@ -70,6 +70,17 @@ public sealed class OutOfProcServerNode : INode, INodePacketFactory, INodePacket private bool _cancelRequested = false; private string _serverBusyMutexName = default!; + // Snapshot of process-global Console state captured once at server startup. + // Restored at the end of each request to prevent loggers (e.g. BaseConsoleLogger + // which sets ForegroundColor in SetColor() but only sometimes restores via ResetColor) + // or per-request ConsoleConfiguration application from leaking visible state into the + // next request or into the idle server's state. + private ConsoleColor _originalForegroundColor; + private ConsoleColor _originalBackgroundColor; + private bool _originalConsoleColorsCaptured; + private System.Text.Encoding _originalOutputEncoding = default!; + private bool _originalOutputEncodingCaptured; + public OutOfProcServerNode(BuildCallback buildFunction) { _buildFunction = buildFunction; @@ -93,6 +104,31 @@ public OutOfProcServerNode(BuildCallback buildFunction) /// The reason for shutting down. public NodeEngineShutdownReason Run(out Exception? shutdownException) { + // Capture process-global Console state once. Restoring per-request keeps reused + // server requests from observing each other's color/encoding mutations. + // Wrapped in try/catch because Console.ForegroundColor/BackgroundColor throw on + // some redirected/headless setups (e.g. CI without a TTY). + try + { + _originalForegroundColor = Console.ForegroundColor; + _originalBackgroundColor = Console.BackgroundColor; + _originalConsoleColorsCaptured = true; + } + catch + { + _originalConsoleColorsCaptured = false; + } + + try + { + _originalOutputEncoding = Console.OutputEncoding; + _originalOutputEncodingCaptured = true; + } + catch + { + _originalOutputEncodingCaptured = false; + } + ServerNodeHandshake handshake = new( CommunicationsUtilities.GetHandshakeOptions(taskHost: false, taskHostParameters: TaskHostParameters.Empty, architectureFlagToSet: XMakeAttributes.GetCurrentMSBuildArchitecture())); @@ -363,6 +399,53 @@ private void HandleServerNodeBuildCommandAsync(ServerNodeBuildCommand command) } private void HandleServerNodeBuildCommand(ServerNodeBuildCommand command) + { + try + { + HandleServerNodeBuildCommandCore(command); + } + finally + { + RestoreConsoleStateAfterRequest(); + } + } + + /// + /// Restore process-global Console state to the snapshot captured at server startup. + /// Called from a finally block in so that + /// per-request mutations (loggers writing to Console.ForegroundColor, the per-request + /// application of ConsoleConfiguration.BackgroundColor, etc.) do not leak across + /// requests in a reused server. See investigation #9379 (LOG-2 / W1c). + /// + private void RestoreConsoleStateAfterRequest() + { + if (_originalConsoleColorsCaptured) + { + try + { + Console.ForegroundColor = _originalForegroundColor; + Console.BackgroundColor = _originalBackgroundColor; + } + catch + { + // Console color mutation can throw on redirected stdout in CI; safe to ignore. + } + } + + if (_originalOutputEncodingCaptured) + { + try + { + Console.OutputEncoding = _originalOutputEncoding; + } + catch + { + // Some hosts disallow OutputEncoding mutation after Console initialization. + } + } + } + + private void HandleServerNodeBuildCommandCore(ServerNodeBuildCommand command) { CommunicationsUtilities.Trace($"Building with MSBuild server with command line {command.CommandLine}"); using var serverBusyMutex = ServerNamedMutex.OpenOrCreateMutex(name: _serverBusyMutexName, createdNew: out var holdsMutex);