From 35837b3fbdfa8694adf3ee9ca24214fb152b1b9a Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 9 Jul 2026 03:14:58 +0900 Subject: [PATCH 1/2] Keep shared Roslyn worker shutdown fallback reachable Separate graceful shutdown from forced termination so broken input channels cannot skip the kill fallback. Log documented process termination failures, always dispose the owned process, and cover each shutdown path with process-free state-transition tests. --- .../SharedRoslynCompilerWorkerHostTests.cs | 115 ++++++++++++++++++ .../SharedRoslynCompilerWorkerSession.cs | 84 +++++++++++-- 2 files changed, 188 insertions(+), 11 deletions(-) diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs index e8355b815e..58fb02d4c6 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs @@ -1,4 +1,5 @@ using System; +using System.ComponentModel; using System.Diagnostics; using System.IO; using System.Text; @@ -79,6 +80,120 @@ public void StartProcessLocked_WhenReplacingCachedProcess_ShouldDisposePreviousH } } + /// + /// Verifies a broken graceful shutdown channel still falls back to forced termination. + /// + [Test] + public void ExecuteProcessShutdown_WhenGracefulRequestThrowsIOException_ShouldStillForceKill() + { + IOException shutdownFailure = new("worker input closed"); + Exception loggedFailure = null; + int forceKillCallCount = 0; + int disposeCallCount = 0; + + SharedRoslynCompilerWorkerSession.ExecuteProcessShutdown( + hasExited: () => false, + requestGracefulShutdown: () => throw shutdownFailure, + forceKill: () => forceKillCallCount++, + dispose: () => disposeCallCount++, + logFailure: ex => loggedFailure = ex); + + Assert.That(loggedFailure, Is.SameAs(shutdownFailure)); + Assert.That(forceKillCallCount, Is.EqualTo(1)); + Assert.That(disposeCallCount, Is.EqualTo(1)); + } + + /// + /// Verifies a disposed graceful shutdown channel still falls back to forced termination. + /// + [Test] + public void ExecuteProcessShutdown_WhenGracefulRequestThrowsObjectDisposedException_ShouldStillForceKill() + { + ObjectDisposedException shutdownFailure = new("worker input"); + Exception loggedFailure = null; + int forceKillCallCount = 0; + int disposeCallCount = 0; + + SharedRoslynCompilerWorkerSession.ExecuteProcessShutdown( + hasExited: () => false, + requestGracefulShutdown: () => throw shutdownFailure, + forceKill: () => forceKillCallCount++, + dispose: () => disposeCallCount++, + logFailure: ex => loggedFailure = ex); + + Assert.That(loggedFailure, Is.SameAs(shutdownFailure)); + Assert.That(forceKillCallCount, Is.EqualTo(1)); + Assert.That(disposeCallCount, Is.EqualTo(1)); + } + + /// + /// Verifies an operating-system kill failure is logged without escaping process disposal. + /// + [Test] + public void ExecuteProcessShutdown_WhenForceKillThrowsWin32Exception_ShouldLogAndDispose() + { + Win32Exception shutdownFailure = new(5, "worker kill denied"); + Exception loggedFailure = null; + int disposeCallCount = 0; + + Assert.DoesNotThrow(() => SharedRoslynCompilerWorkerSession.ExecuteProcessShutdown( + hasExited: () => false, + requestGracefulShutdown: () => { }, + forceKill: () => throw shutdownFailure, + dispose: () => disposeCallCount++, + logFailure: ex => loggedFailure = ex)); + + Assert.That(loggedFailure, Is.SameAs(shutdownFailure)); + Assert.That(disposeCallCount, Is.EqualTo(1)); + } + + /// + /// Verifies successful graceful shutdown skips forced termination and disposes once. + /// + [Test] + public void ExecuteProcessShutdown_WhenGracefulRequestExitsProcess_ShouldSkipForceKill() + { + bool processExited = false; + int forceKillCallCount = 0; + int disposeCallCount = 0; + int failureLogCount = 0; + + SharedRoslynCompilerWorkerSession.ExecuteProcessShutdown( + hasExited: () => processExited, + requestGracefulShutdown: () => processExited = true, + forceKill: () => forceKillCallCount++, + dispose: () => disposeCallCount++, + logFailure: ex => failureLogCount++); + + Assert.That(forceKillCallCount, Is.Zero); + Assert.That(failureLogCount, Is.Zero); + Assert.That(disposeCallCount, Is.EqualTo(1)); + } + + /// + /// Verifies an already exited process skips both termination phases and disposes once. + /// + [Test] + public void ExecuteProcessShutdown_WhenProcessAlreadyExited_ShouldOnlyDispose() + { + int gracefulRequestCallCount = 0; + int forceKillCallCount = 0; + int disposeCallCount = 0; + int failureLogCount = 0; + + SharedRoslynCompilerWorkerSession.ExecuteProcessShutdown( + hasExited: () => true, + requestGracefulShutdown: () => gracefulRequestCallCount++, + forceKill: () => forceKillCallCount++, + dispose: () => disposeCallCount++, + logFailure: ex => failureLogCount++); + + Assert.That(gracefulRequestCallCount, Is.Zero); + Assert.That(forceKillCallCount, Is.Zero); + Assert.That(failureLogCount, Is.Zero); + Assert.That(disposeCallCount, Is.EqualTo(1)); + } + /// /// Verifies a pending compiler stream does not keep the bounded drain waiting. /// diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs index 88a28b942e..489ad671b3 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs @@ -1,4 +1,5 @@ using System; +using System.ComponentModel; using System.Diagnostics; using System.IO; using System.Threading; @@ -104,37 +105,98 @@ internal void ShutdownProcessLocked() return; } - try - { - if (!workerProcess.HasExited) + ExecuteProcessShutdown( + hasExited: () => workerProcess.HasExited, + requestGracefulShutdown: () => { workerProcess.StandardInput.WriteLine( SharedRoslynCompilerWorkerProtocol.SharedCompilerWorkerQuitCommand); workerProcess.StandardInput.Flush(); workerProcess.WaitForExit(500); - } - - if (!workerProcess.HasExited) + }, + forceKill: () => { workerProcess.Kill(); workerProcess.WaitForExit(500); + }, + dispose: workerProcess.Dispose, + logFailure: LogWorkerShutdownFailure); + } + + internal static void ExecuteProcessShutdown( + Func hasExited, + Action requestGracefulShutdown, + Action forceKill, + Action dispose, + Action logFailure) + { + Debug.Assert(hasExited != null, "hasExited must not be null"); + Debug.Assert(requestGracefulShutdown != null, "requestGracefulShutdown must not be null"); + Debug.Assert(forceKill != null, "forceKill must not be null"); + Debug.Assert(dispose != null, "dispose must not be null"); + Debug.Assert(logFailure != null, "logFailure must not be null"); + + try + { + // A failed graceful request must not prevent the forced termination phase. + TryRequestGracefulShutdown(hasExited, requestGracefulShutdown, logFailure); + TryForceKill(hasExited, forceKill, logFailure); + } + finally + { + dispose(); + } + } + + private static void TryRequestGracefulShutdown( + Func hasExited, + Action requestGracefulShutdown, + Action logFailure) + { + try + { + if (!hasExited()) + { + requestGracefulShutdown(); } } catch (IOException ex) { - LogWorkerShutdownFailure(ex); + logFailure(ex); } catch (ObjectDisposedException ex) { - LogWorkerShutdownFailure(ex); + logFailure(ex); } catch (InvalidOperationException ex) { - LogWorkerShutdownFailure(ex); + logFailure(ex); } - finally + catch (Win32Exception ex) + { + logFailure(ex); + } + } + + private static void TryForceKill( + Func hasExited, + Action forceKill, + Action logFailure) + { + try + { + if (!hasExited()) + { + forceKill(); + } + } + catch (Win32Exception ex) + { + logFailure(ex); + } + catch (InvalidOperationException ex) { - workerProcess.Dispose(); + logFailure(ex); } } From 6cbedfc938597c54d74552ec824514c7c54e0a5a Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 9 Jul 2026 03:25:21 +0900 Subject: [PATCH 2/2] Force worker termination when exit state is unknown Treat Win32 exit-code lookup failures as an unknown process state and still attempt forced termination, while preserving the no-process behavior for InvalidOperationException. Cover both query outcomes with process-free tests. --- .../SharedRoslynCompilerWorkerHostTests.cs | 78 +++++++++++++++++++ .../SharedRoslynCompilerWorkerSession.cs | 27 ++++++- 2 files changed, 101 insertions(+), 4 deletions(-) diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs index 58fb02d4c6..7f7e3a3c08 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs @@ -147,6 +147,84 @@ public void ExecuteProcessShutdown_WhenForceKillThrowsWin32Exception_ShouldLogAn Assert.That(disposeCallCount, Is.EqualTo(1)); } + /// + /// Verifies an unknown exit state still attempts forced termination after logging the query failure. + /// + [Test] + public void ExecuteProcessShutdown_WhenForceExitQueryThrowsWin32Exception_ShouldStillForceKill() + { + Win32Exception queryFailure = new(5, "worker exit code unavailable"); + Exception loggedFailure = null; + int hasExitedCallCount = 0; + int forceKillCallCount = 0; + int disposeCallCount = 0; + int failureLogCount = 0; + + SharedRoslynCompilerWorkerSession.ExecuteProcessShutdown( + hasExited: () => + { + hasExitedCallCount++; + if (hasExitedCallCount == 2) + { + throw queryFailure; + } + + return false; + }, + requestGracefulShutdown: () => { }, + forceKill: () => forceKillCallCount++, + dispose: () => disposeCallCount++, + logFailure: ex => + { + failureLogCount++; + loggedFailure = ex; + }); + + Assert.That(loggedFailure, Is.SameAs(queryFailure)); + Assert.That(failureLogCount, Is.EqualTo(1)); + Assert.That(forceKillCallCount, Is.EqualTo(1)); + Assert.That(disposeCallCount, Is.EqualTo(1)); + } + + /// + /// Verifies a missing associated process skips forced termination after logging the query failure. + /// + [Test] + public void ExecuteProcessShutdown_WhenForceExitQueryThrowsInvalidOperationException_ShouldSkipForceKill() + { + InvalidOperationException queryFailure = new("worker process unavailable"); + Exception loggedFailure = null; + int hasExitedCallCount = 0; + int forceKillCallCount = 0; + int disposeCallCount = 0; + int failureLogCount = 0; + + SharedRoslynCompilerWorkerSession.ExecuteProcessShutdown( + hasExited: () => + { + hasExitedCallCount++; + if (hasExitedCallCount == 2) + { + throw queryFailure; + } + + return false; + }, + requestGracefulShutdown: () => { }, + forceKill: () => forceKillCallCount++, + dispose: () => disposeCallCount++, + logFailure: ex => + { + failureLogCount++; + loggedFailure = ex; + }); + + Assert.That(loggedFailure, Is.SameAs(queryFailure)); + Assert.That(failureLogCount, Is.EqualTo(1)); + Assert.That(forceKillCallCount, Is.Zero); + Assert.That(disposeCallCount, Is.EqualTo(1)); + } + /// /// Verifies successful graceful shutdown skips forced termination and disposes once. /// diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs index 489ad671b3..da39886346 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs @@ -183,12 +183,31 @@ private static void TryForceKill( Action forceKill, Action logFailure) { + bool shouldForceKill; try { - if (!hasExited()) - { - forceKill(); - } + shouldForceKill = !hasExited(); + } + catch (Win32Exception ex) + { + logFailure(ex); + // An unavailable exit code leaves process state unknown, so forced termination is safer. + shouldForceKill = true; + } + catch (InvalidOperationException ex) + { + logFailure(ex); + return; + } + + if (!shouldForceKill) + { + return; + } + + try + { + forceKill(); } catch (Win32Exception ex) {