diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs index 51413deadf..0eef762da5 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs @@ -28,6 +28,22 @@ public void ConfigureWorkerDotnetRuntimeEnvironment_WhenCalled_ShouldDisableMult Is.EqualTo(SharedRoslynCompilerWorkerAssemblyBuilder.DotnetMultilevelLookupDisabledValue)); } + /// + /// Verifies shutdown remains an idempotent no-op before a worker process or directory exists. + /// + [Test] + public void Shutdown_WhenWorkerWasNeverStarted_ShouldRemainIdempotent() + { + SharedRoslynCompilerWorkerSession session = new(); + string unusedWorkerDirectoryPath = Path.Combine( + Path.GetTempPath(), + $"SharedRoslynCompilerWorkerSessionTests_{Guid.NewGuid():N}"); + + Assert.That(Directory.Exists(unusedWorkerDirectoryPath), Is.False); + Assert.DoesNotThrow(() => session.Shutdown(unusedWorkerDirectoryPath)); + Assert.DoesNotThrow(() => session.Shutdown(unusedWorkerDirectoryPath)); + } + /// /// Verifies a pending compiler stream does not keep the bounded drain waiting. /// diff --git a/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs b/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs index 99ee87d9fa..433f950558 100644 --- a/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs +++ b/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs @@ -20,6 +20,7 @@ public sealed class StaticFacadeStateGuardTests "Packages/src/Editor/Application/UnityCliLoopToolRegistrar.cs", "Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeServices.cs", "Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeForegroundWarmupState.cs", + "Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerHost.cs", "Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs", "Packages/src/Editor/FirstPartyTools/Common/InputRecording/InputRecorder.cs", "Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs", @@ -52,7 +53,8 @@ public sealed class StaticFacadeStateGuardTests "Packages/src/Editor/Application/SessionRecoveryService.cs", "Packages/src/Editor/Application/UseCases/SkillSetupUseCase.cs", "Packages/src/Editor/Domain/UnityCliLoopCompileSessionLifecycleService.cs", - "Packages/src/Editor/Infrastructure/Server/DomainReloadDetectionFileService.cs" + "Packages/src/Editor/Infrastructure/Server/DomainReloadDetectionFileService.cs", + "Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs" }; private static readonly string[] AsyncCancellationTokenGuardPaths = new string[] diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerHost.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerHost.cs index db6be6a175..be7d466233 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerHost.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerHost.cs @@ -5,7 +5,6 @@ using System.Threading; using UnityEditor; using UnityEditor.Compilation; -using Debug = UnityEngine.Debug; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -21,13 +20,7 @@ internal static class SharedRoslynCompilerWorkerHost private const string RoslynWorkerAssemblyFileName = "RoslynCompilerWorker.dll"; private const string RoslynWorkerCompileResponseFileName = "RoslynCompilerWorker.rsp"; - private static readonly object SharedCompilerWorkerLock = new(); - private static Action s_deleteWorkerDirectory = path => Directory.Delete(path, true); - private static Action s_sendCompileRequest = SendCompileRequestCore; - private static Func - s_compileWorkerAssemblyForTests; - private static Process _sharedCompilerWorkerProcess; - private static string _workerDirectoryPath; + private static readonly SharedRoslynCompilerWorkerSession ServiceValue = new(); /// /// Carries the result data produced by Worker Attempt behavior. @@ -138,16 +131,14 @@ public static CompilerMessage[] TryCompile( Action markBuildFinished, Action incrementBuildCount) { - lock (SharedCompilerWorkerLock) - { - return TryCompileWithRetries( + return ServiceValue.ExecuteLocked( + () => TryCompileWithRetries( requestFilePath, externalCompilerPaths, ct, markBuildStarted, markBuildFinished, - incrementBuildCount); - } + incrementBuildCount)); } private static CompilerMessage[] TryCompileWithRetries( @@ -173,7 +164,7 @@ private static CompilerMessage[] TryCompileWithRetries( return attemptResult.Messages; } - ShutdownWorkerProcessLocked(); + ServiceValue.ShutdownProcessLocked(); if (attemptResult.ShouldRetry && attempt < SharedCompilerWorkerMaxAttempts) { @@ -215,7 +206,7 @@ private static WorkerAttemptResult TryCompileOnce( private static WorkerStartupResult EnsureWorkerReady(ExternalCompilerPaths externalCompilerPaths) { - if (HasLiveWorkerProcess()) + if (ServiceValue.HasLiveProcessLocked()) { return WorkerStartupResult.Ready(); } @@ -243,11 +234,12 @@ private static WorkerStartupResult EnsureWorkerAssemblyBuilt( return WorkerStartupResult.Ready(); } - SharedRoslynCompilerWorkerAssemblyBuilder.WorkerAssemblyBuildResult buildResult = CompileWorkerAssembly( - externalCompilerPaths, - workerPaths.SourcePath, - workerPaths.AssemblyPath, - workerPaths.CompileResponseFilePath); + SharedRoslynCompilerWorkerAssemblyBuilder.WorkerAssemblyBuildResult buildResult = + ServiceValue.CompileWorkerAssemblyLocked( + externalCompilerPaths, + workerPaths.SourcePath, + workerPaths.AssemblyPath, + workerPaths.CompileResponseFilePath); if (!buildResult.StartedSuccessfully) { return WorkerStartupResult.Failure( @@ -275,8 +267,7 @@ private static WorkerStartupResult StartWorkerProcess( WorkerPaths workerPaths) { ProcessStartInfo startInfo = CreateWorkerStartInfo(externalCompilerPaths, workerPaths); - _sharedCompilerWorkerProcess = ProcessStartHelper.TryStart(startInfo); - if (_sharedCompilerWorkerProcess == null) + if (!ServiceValue.StartProcessLocked(startInfo)) { return WorkerStartupResult.Failure( "worker_start_failed", @@ -297,7 +288,7 @@ private static WorkerAttemptResult InvokeWorkerOnce( Action markBuildFinished, Action incrementBuildCount) { - if (!HasLiveWorkerProcess()) + if (!ServiceValue.HasLiveProcessLocked()) { return WorkerAttemptResult.RetryableFailure( "worker_process_missing", @@ -323,7 +314,7 @@ private static WorkerAttemptResult InvokeWorkerOnce( } catch (OperationCanceledException) { - ShutdownWorkerProcessLocked(); + ServiceValue.ShutdownProcessLocked(); throw; } finally @@ -335,21 +326,14 @@ private static WorkerAttemptResult InvokeWorkerOnce( private static void SendCompileRequest(string requestFilePath) { string absoluteRequestFilePath = Path.GetFullPath(requestFilePath); - s_sendCompileRequest(_sharedCompilerWorkerProcess, absoluteRequestFilePath); - } - - private static void SendCompileRequestCore(Process workerProcess, string requestFilePath) - { - string requestCommand = SharedRoslynCompilerWorkerProtocol.CreateCompileRequestCommand(requestFilePath); - workerProcess.StandardInput.WriteLine(requestCommand); - workerProcess.StandardInput.Flush(); + ServiceValue.SendCompileRequestLocked(absoluteRequestFilePath); } private static WorkerAttemptResult ReadWorkerResponse( string requestFilePath, CancellationToken ct) { - StreamReader reader = _sharedCompilerWorkerProcess.StandardOutput; + StreamReader reader = ServiceValue.GetOutputReaderLocked(); string responseHeader = SharedRoslynCompilerWorkerProtocol.ReadProtocolLine( reader, ct); @@ -384,7 +368,7 @@ private static WorkerPaths CreateWorkerPaths() { string workerDirectoryPath = GetWorkerDirectoryPath(); Directory.CreateDirectory(workerDirectoryPath); - _workerDirectoryPath = workerDirectoryPath; + ServiceValue.RecordWorkerDirectoryLocked(workerDirectoryPath); return new WorkerPaths( workerDirectoryPath, Path.Combine(workerDirectoryPath, RoslynWorkerSourceFileName), @@ -437,34 +421,6 @@ private static ProcessStartInfo CreateWorkerStartInfo( return startInfo; } - private static SharedRoslynCompilerWorkerAssemblyBuilder.WorkerAssemblyBuildResult CompileWorkerAssembly( - ExternalCompilerPaths externalCompilerPaths, - string workerSourcePath, - string workerAssemblyPath, - string workerCompileResponseFilePath) - { - if (s_compileWorkerAssemblyForTests != null) - { - return SharedRoslynCompilerWorkerAssemblyBuilder.WorkerAssemblyBuildResult.Started( - s_compileWorkerAssemblyForTests( - externalCompilerPaths, - workerSourcePath, - workerAssemblyPath, - workerCompileResponseFilePath)); - } - - return SharedRoslynCompilerWorkerAssemblyBuilder.CompileWorkerAssembly( - externalCompilerPaths, - workerSourcePath, - workerAssemblyPath, - workerCompileResponseFilePath); - } - - private static bool HasLiveWorkerProcess() - { - return _sharedCompilerWorkerProcess != null && !_sharedCompilerWorkerProcess.HasExited; - } - private static bool HasErrors(IReadOnlyCollection messages) { foreach (CompilerMessage message in messages) @@ -513,127 +469,24 @@ internal static void ShutdownForServerReset() internal static Action SwapWorkerDirectoryDeleterForTests(Action deleter) { - Debug.Assert(deleter != null, "deleter must not be null"); - - Action previous = s_deleteWorkerDirectory; - s_deleteWorkerDirectory = deleter; - return previous; + return ServiceValue.SwapWorkerDirectoryDeleterForTests(deleter); } internal static Action SwapCompileRequestSenderForTests(Action sender) { - Debug.Assert(sender != null, "sender must not be null"); - - Action previous = s_sendCompileRequest; - s_sendCompileRequest = sender; - return previous; + return ServiceValue.SwapCompileRequestSenderForTests(sender); } internal static Func SwapWorkerAssemblyCompilerForTests( Func compiler) { - Func previous = - s_compileWorkerAssemblyForTests; - s_compileWorkerAssemblyForTests = compiler; - return previous; + return ServiceValue.SwapWorkerAssemblyCompilerForTests(compiler); } private static void Shutdown() { - lock (SharedCompilerWorkerLock) - { - ShutdownWorkerProcessLocked(); - CleanupWorkerDirectoryLocked(); - } - } - - private static void ShutdownWorkerProcessLocked() - { - Process workerProcess = _sharedCompilerWorkerProcess; - _sharedCompilerWorkerProcess = null; - if (workerProcess == null) - { - return; - } - - try - { - if (!workerProcess.HasExited) - { - workerProcess.StandardInput.WriteLine( - SharedRoslynCompilerWorkerProtocol.SharedCompilerWorkerQuitCommand); - workerProcess.StandardInput.Flush(); - workerProcess.WaitForExit(500); - } - - if (!workerProcess.HasExited) - { - workerProcess.Kill(); - workerProcess.WaitForExit(500); - } - } - catch (IOException ex) - { - LogWorkerShutdownFailure(ex); - } - catch (ObjectDisposedException ex) - { - LogWorkerShutdownFailure(ex); - } - catch (InvalidOperationException ex) - { - LogWorkerShutdownFailure(ex); - } - finally - { - workerProcess.Dispose(); - } - } - - private static void CleanupWorkerDirectoryLocked() - { - string workerDirectoryPath = _workerDirectoryPath ?? GetWorkerDirectoryPath(); - if (!Directory.Exists(workerDirectoryPath)) - { - _workerDirectoryPath = null; - return; - } - - TryDeleteWorkerDirectory(workerDirectoryPath); - _workerDirectoryPath = null; - } - - private static void TryDeleteWorkerDirectory(string workerDirectoryPath) - { - try - { - s_deleteWorkerDirectory(workerDirectoryPath); - } - catch (IOException ex) - { - LogWorkerDirectoryCleanupFailure(workerDirectoryPath, ex); - } - catch (UnauthorizedAccessException ex) - { - LogWorkerDirectoryCleanupFailure(workerDirectoryPath, ex); - } - } - - private static void LogWorkerDirectoryCleanupFailure(string workerDirectoryPath, Exception ex) - { - VibeLogger.LogWarning( - "dynamic_code_shared_worker_cleanup_failed", - "execute-dynamic-code shared Roslyn worker directory cleanup failed during shutdown", - new - { - worker_directory_path = workerDirectoryPath, - exception_type = ex.GetType().FullName, - exception_message = ex.Message - }, - humanNote: "Shared Roslyn worker cleanup could not remove its temporary directory during shutdown.", - aiTodo: "Investigate file locks or permission issues if temporary worker directories continue to accumulate."); - Debug.LogWarning($"[{UnityCliLoopConstants.PROJECT_NAME}] Failed to delete shared Roslyn worker directory '{workerDirectoryPath}': {ex.Message}"); + ServiceValue.Shutdown(GetWorkerDirectoryPath()); } private static WorkerAttemptResult CreateRetryableWorkerCommunicationFailure( @@ -650,20 +503,6 @@ private static WorkerAttemptResult CreateRetryableWorkerCommunicationFailure( }); } - private static void LogWorkerShutdownFailure(Exception ex) - { - VibeLogger.LogWarning( - "dynamic_code_shared_worker_shutdown_failed", - "execute-dynamic-code shared Roslyn worker shutdown observed a communication failure", - new - { - exception_type = ex.GetType().FullName, - exception_message = ex.Message - }, - humanNote: "Shared Roslyn worker shutdown saw a broken communication channel while cleaning up a crashed worker.", - aiTodo: "Investigate repeated worker shutdown communication failures if shared compilation stops recovering cleanly."); - } - private static object AppendAttempt(object failureContext, int attempt) { return new diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs new file mode 100644 index 0000000000..0d9e4e6966 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs @@ -0,0 +1,243 @@ +using System; +using System.Diagnostics; +using System.IO; +using System.Threading; +using UnityEditor.Compilation; +using Debug = UnityEngine.Debug; + +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Owns the shared Roslyn compiler worker process, temporary directory, and synchronized lifecycle. + /// + internal sealed class SharedRoslynCompilerWorkerSession + { + private readonly object _syncRoot = new(); + private Action _deleteWorkerDirectory = path => Directory.Delete(path, true); + private Action _sendCompileRequest = SendCompileRequestCore; + private Func + _compileWorkerAssemblyForTests; + private Process _workerProcess; + private string _workerDirectoryPath; + + internal T ExecuteLocked(Func operation) + { + Debug.Assert(operation != null, "operation must not be null"); + + lock (_syncRoot) + { + return operation(); + } + } + + internal bool HasLiveProcessLocked() + { + AssertLockHeld(); + return _workerProcess != null && !_workerProcess.HasExited; + } + + internal bool StartProcessLocked(ProcessStartInfo startInfo) + { + AssertLockHeld(); + _workerProcess = ProcessStartHelper.TryStart(startInfo); + return _workerProcess != null; + } + + internal void SendCompileRequestLocked(string requestFilePath) + { + AssertLockHeld(); + _sendCompileRequest(_workerProcess, requestFilePath); + } + + internal StreamReader GetOutputReaderLocked() + { + AssertLockHeld(); + return _workerProcess.StandardOutput; + } + + internal void RecordWorkerDirectoryLocked(string workerDirectoryPath) + { + AssertLockHeld(); + _workerDirectoryPath = workerDirectoryPath; + } + + internal SharedRoslynCompilerWorkerAssemblyBuilder.WorkerAssemblyBuildResult CompileWorkerAssemblyLocked( + ExternalCompilerPaths externalCompilerPaths, + string workerSourcePath, + string workerAssemblyPath, + string workerCompileResponseFilePath) + { + AssertLockHeld(); + if (_compileWorkerAssemblyForTests != null) + { + return SharedRoslynCompilerWorkerAssemblyBuilder.WorkerAssemblyBuildResult.Started( + _compileWorkerAssemblyForTests( + externalCompilerPaths, + workerSourcePath, + workerAssemblyPath, + workerCompileResponseFilePath)); + } + + return SharedRoslynCompilerWorkerAssemblyBuilder.CompileWorkerAssembly( + externalCompilerPaths, + workerSourcePath, + workerAssemblyPath, + workerCompileResponseFilePath); + } + + internal void ShutdownProcessLocked() + { + AssertLockHeld(); + Process workerProcess = _workerProcess; + _workerProcess = null; + if (workerProcess == null) + { + return; + } + + try + { + if (!workerProcess.HasExited) + { + workerProcess.StandardInput.WriteLine( + SharedRoslynCompilerWorkerProtocol.SharedCompilerWorkerQuitCommand); + workerProcess.StandardInput.Flush(); + workerProcess.WaitForExit(500); + } + + if (!workerProcess.HasExited) + { + workerProcess.Kill(); + workerProcess.WaitForExit(500); + } + } + catch (IOException ex) + { + LogWorkerShutdownFailure(ex); + } + catch (ObjectDisposedException ex) + { + LogWorkerShutdownFailure(ex); + } + catch (InvalidOperationException ex) + { + LogWorkerShutdownFailure(ex); + } + finally + { + workerProcess.Dispose(); + } + } + + internal void Shutdown(string fallbackWorkerDirectoryPath) + { + lock (_syncRoot) + { + ShutdownProcessLocked(); + CleanupWorkerDirectoryLocked(fallbackWorkerDirectoryPath); + } + } + + internal Action SwapWorkerDirectoryDeleterForTests(Action deleter) + { + Debug.Assert(deleter != null, "deleter must not be null"); + + Action previous = _deleteWorkerDirectory; + _deleteWorkerDirectory = deleter; + return previous; + } + + internal Action SwapCompileRequestSenderForTests(Action sender) + { + Debug.Assert(sender != null, "sender must not be null"); + + Action previous = _sendCompileRequest; + _sendCompileRequest = sender; + return previous; + } + + internal Func + SwapWorkerAssemblyCompilerForTests( + Func compiler) + { + Func previous = + _compileWorkerAssemblyForTests; + _compileWorkerAssemblyForTests = compiler; + return previous; + } + + private static void SendCompileRequestCore(Process workerProcess, string requestFilePath) + { + string requestCommand = SharedRoslynCompilerWorkerProtocol.CreateCompileRequestCommand(requestFilePath); + workerProcess.StandardInput.WriteLine(requestCommand); + workerProcess.StandardInput.Flush(); + } + + private void CleanupWorkerDirectoryLocked(string fallbackWorkerDirectoryPath) + { + AssertLockHeld(); + string workerDirectoryPath = _workerDirectoryPath ?? fallbackWorkerDirectoryPath; + if (!Directory.Exists(workerDirectoryPath)) + { + _workerDirectoryPath = null; + return; + } + + TryDeleteWorkerDirectory(workerDirectoryPath); + _workerDirectoryPath = null; + } + + private void TryDeleteWorkerDirectory(string workerDirectoryPath) + { + try + { + _deleteWorkerDirectory(workerDirectoryPath); + } + catch (IOException ex) + { + LogWorkerDirectoryCleanupFailure(workerDirectoryPath, ex); + } + catch (UnauthorizedAccessException ex) + { + LogWorkerDirectoryCleanupFailure(workerDirectoryPath, ex); + } + } + + private static void LogWorkerDirectoryCleanupFailure(string workerDirectoryPath, Exception ex) + { + VibeLogger.LogWarning( + "dynamic_code_shared_worker_cleanup_failed", + "execute-dynamic-code shared Roslyn worker directory cleanup failed during shutdown", + new + { + worker_directory_path = workerDirectoryPath, + exception_type = ex.GetType().FullName, + exception_message = ex.Message + }, + humanNote: "Shared Roslyn worker cleanup could not remove its temporary directory during shutdown.", + aiTodo: "Investigate file locks or permission issues if temporary worker directories continue to accumulate."); + Debug.LogWarning($"[{UnityCliLoopConstants.PROJECT_NAME}] Failed to delete shared Roslyn worker directory '{workerDirectoryPath}': {ex.Message}"); + } + + private static void LogWorkerShutdownFailure(Exception ex) + { + VibeLogger.LogWarning( + "dynamic_code_shared_worker_shutdown_failed", + "execute-dynamic-code shared Roslyn worker shutdown observed a communication failure", + new + { + exception_type = ex.GetType().FullName, + exception_message = ex.Message + }, + humanNote: "Shared Roslyn worker shutdown saw a broken communication channel while cleaning up a crashed worker.", + aiTodo: "Investigate repeated worker shutdown communication failures if shared compilation stops recovering cleanly."); + } + + private void AssertLockHeld() + { + Debug.Assert(Monitor.IsEntered(_syncRoot), "Shared worker session lock must be held"); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs.meta b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs.meta new file mode 100644 index 0000000000..710133d89d --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 35eec184e19040a3a2dad9a75933e215 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: