Skip to content

chore: Simplify shared Roslyn worker lifecycle ownership #1629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hatayama merged 1 commit into from
Jul 8, 2026
Merged

chore: Simplify shared Roslyn worker lifecycle ownership #1629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,22 @@ public void ConfigureWorkerDotnetRuntimeEnvironment_WhenCalled_ShouldDisableMult
Is.EqualTo(SharedRoslynCompilerWorkerAssemblyBuilder.DotnetMultilevelLookupDisabledValue));
}

/// <summary>
/// Verifies shutdown remains an idempotent no-op before a worker process or directory exists.
/// </summary>
[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));
}

/// <summary>
/// Verifies a pending compiler stream does not keep the bounded drain waiting.
/// </summary>
Expand Down
4 changes: 3 additions & 1 deletion Assets/Tests/Editor/StaticFacadeStateGuardTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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[]
Expand Down
205 changes: 22 additions & 183 deletions ...r/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Threading;
using UnityEditor;
using UnityEditor.Compilation;
using Debug = UnityEngine.Debug;

using io.github.hatayama.UnityCliLoop.ToolContracts;

Expand All @@ -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<string> s_deleteWorkerDirectory = path => Directory.Delete(path, true);
private static Action<Process, string> s_sendCompileRequest = SendCompileRequestCore;
private static Func<ExternalCompilerPaths, string, string, string, CompilerMessage[]>
s_compileWorkerAssemblyForTests;
private static Process _sharedCompilerWorkerProcess;
private static string _workerDirectoryPath;
private static readonly SharedRoslynCompilerWorkerSession ServiceValue = new();

/// <summary>
/// Carries the result data produced by Worker Attempt behavior.
Expand Down Expand Up @@ -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(
Expand All @@ -173,7 +164,7 @@ private static CompilerMessage[] TryCompileWithRetries(
return attemptResult.Messages;
}

ShutdownWorkerProcessLocked();
ServiceValue.ShutdownProcessLocked();

if (attemptResult.ShouldRetry && attempt < SharedCompilerWorkerMaxAttempts)
{
Expand Down Expand Up @@ -215,7 +206,7 @@ private static WorkerAttemptResult TryCompileOnce(

private static WorkerStartupResult EnsureWorkerReady(ExternalCompilerPaths externalCompilerPaths)
{
if (HasLiveWorkerProcess())
if (ServiceValue.HasLiveProcessLocked())
{
return WorkerStartupResult.Ready();
}
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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",
Expand All @@ -297,7 +288,7 @@ private static WorkerAttemptResult InvokeWorkerOnce(
Action markBuildFinished,
Action incrementBuildCount)
{
if (!HasLiveWorkerProcess())
if (!ServiceValue.HasLiveProcessLocked())
{
return WorkerAttemptResult.RetryableFailure(
"worker_process_missing",
Expand All @@ -323,7 +314,7 @@ private static WorkerAttemptResult InvokeWorkerOnce(
}
catch (OperationCanceledException)
{
ShutdownWorkerProcessLocked();
ServiceValue.ShutdownProcessLocked();
throw;
}
finally
Expand All @@ -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);
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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<CompilerMessage> messages)
{
foreach (CompilerMessage message in messages)
Expand Down Expand Up @@ -513,127 +469,24 @@ internal static void ShutdownForServerReset()

internal static Action<string> SwapWorkerDirectoryDeleterForTests(Action<string> deleter)
{
Debug.Assert(deleter != null, "deleter must not be null");

Action<string> previous = s_deleteWorkerDirectory;
s_deleteWorkerDirectory = deleter;
return previous;
return ServiceValue.SwapWorkerDirectoryDeleterForTests(deleter);
}

internal static Action<Process, string> SwapCompileRequestSenderForTests(Action<Process, string> sender)
{
Debug.Assert(sender != null, "sender must not be null");

Action<Process, string> previous = s_sendCompileRequest;
s_sendCompileRequest = sender;
return previous;
return ServiceValue.SwapCompileRequestSenderForTests(sender);
}

internal static Func<ExternalCompilerPaths, string, string, string, CompilerMessage[]>
SwapWorkerAssemblyCompilerForTests(
Func<ExternalCompilerPaths, string, string, string, CompilerMessage[]> compiler)
{
Func<ExternalCompilerPaths, string, string, string, CompilerMessage[]> 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(
Expand All @@ -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
Expand Down
Loading
Loading