diff --git a/.github/workflows/unity-compile-check-and-test-runner.yml b/.github/workflows/unity-compile-check-and-test-runner.yml index ab654f8b3..4b00b478e 100644 --- a/.github/workflows/unity-compile-check-and-test-runner.yml +++ b/.github/workflows/unity-compile-check-and-test-runner.yml @@ -65,7 +65,9 @@ jobs: - name: Run concurrency unit tests if: steps.changes.outputs.csharp == 'true' - run: dotnet test tests/DynamicCodeExecutionScheduler.UnitTests/DynamicCodeExecutionScheduler.UnitTests.csproj --configuration Release + run: | + dotnet test tests/DynamicCodeExecutionScheduler.UnitTests/DynamicCodeExecutionScheduler.UnitTests.csproj --configuration Release + dotnet test tests/SharedRoslynCompilerWorker.UnitTests/SharedRoslynCompilerWorker.UnitTests.csproj --configuration Release - name: Cache Library folder if: env.HAS_UNITY_LICENSE == 'true' diff --git a/.github/workflows/unity-editmode-tests.yml b/.github/workflows/unity-editmode-tests.yml index b0e62e0c4..826b6fc97 100644 --- a/.github/workflows/unity-editmode-tests.yml +++ b/.github/workflows/unity-editmode-tests.yml @@ -27,7 +27,9 @@ jobs: dotnet-version: 10.0.x - name: Run concurrency unit tests - run: dotnet test tests/DynamicCodeExecutionScheduler.UnitTests/DynamicCodeExecutionScheduler.UnitTests.csproj --configuration Release + run: | + dotnet test tests/DynamicCodeExecutionScheduler.UnitTests/DynamicCodeExecutionScheduler.UnitTests.csproj --configuration Release + dotnet test tests/SharedRoslynCompilerWorker.UnitTests/SharedRoslynCompilerWorker.UnitTests.csproj --configuration Release - name: Cache Library folder if: env.HAS_UNITY_LICENSE == 'true' diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/WrapperTemplateTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/WrapperTemplateTests.cs new file mode 100644 index 000000000..05bb62999 --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/WrapperTemplateTests.cs @@ -0,0 +1,34 @@ +using System.Collections.Generic; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.DynamicCodeToolTests +{ + /// + /// String-level coverage for WrapperTemplate entry-point generation (no compile-and-run). + /// + [TestFixture] + public class WrapperTemplateTests + { + [Test] + public void Build_ShouldGenerateExecuteAsyncWithCancellationTokenAndOmitSyncExecute() + { + // Verifies wrapped snippets expose only ExecuteAsync with ct, not a sync Execute that + // would discard the runtime token via ExecuteAsync(parameters, default). + string wrappedSource = WrapperTemplate.Build( + new List(), + System.Array.Empty(), + "TestNs", + "TestClass", + "return 1;"); + + Assert.That(wrappedSource, Does.Contain("public async System.Threading.Tasks.Task ExecuteAsync(")); + Assert.That(wrappedSource, Does.Contain("System.Threading.CancellationToken ct = default)")); + Assert.That(wrappedSource, Does.Not.Contain("public object Execute(")); + Assert.That(wrappedSource, Does.Not.Contain("ExecuteAsync(parameters, default)")); + Assert.That(wrappedSource, Does.Not.Contain("GetAwaiter().GetResult()")); + } + } +} diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/WrapperTemplateTests.cs.meta b/Assets/Tests/Editor/DynamicCodeToolTests/WrapperTemplateTests.cs.meta new file mode 100644 index 000000000..f22e098d9 --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/WrapperTemplateTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: d1f6b4a37a0e4c5b9e8d2c1a0f3b5e7a +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerBackend.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerBackend.cs index 930b76996..edebe7b8d 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerBackend.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerBackend.cs @@ -71,13 +71,13 @@ public static async Task CompileAsync( defineSymbols, allowUnsafeCode); - CompilerMessage[] workerMessages = SharedRoslynCompilerWorkerHost.TryCompile( + CompilerMessage[] workerMessages = await SharedRoslynCompilerWorkerHost.TryCompileAsync( workerRequestFilePath, externalCompilerPaths, ct, markBuildStarted, markBuildFinished, - incrementBuildCount); + incrementBuildCount).ConfigureAwait(false); if (workerMessages != null) { return new DynamicCompilationBackendResult( diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerHost.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerHost.cs index 586de3c94..355cefd48 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerHost.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerHost.cs @@ -3,6 +3,7 @@ using System.Diagnostics; using System.IO; using System.Threading; +using System.Threading.Tasks; using UnityEditor; using UnityEditor.Compilation; @@ -123,7 +124,7 @@ internal static void RegisterLifecycleForEditorStartup() EditorApplication.quitting += ShutdownForQuit; } - public static CompilerMessage[] TryCompile( + public static Task TryCompileAsync( string requestFilePath, ExternalCompilerPaths externalCompilerPaths, CancellationToken ct, @@ -131,17 +132,18 @@ public static CompilerMessage[] TryCompile( Action markBuildFinished, Action incrementBuildCount) { - return ServiceValue.ExecuteLocked( - () => TryCompileWithRetries( + return ServiceValue.RunSerializedCompileAsync( + operationCt => TryCompileWithRetriesAsync( requestFilePath, externalCompilerPaths, - ct, + operationCt, markBuildStarted, markBuildFinished, - incrementBuildCount)); + incrementBuildCount), + ct); } - private static CompilerMessage[] TryCompileWithRetries( + private static async Task TryCompileWithRetriesAsync( string requestFilePath, ExternalCompilerPaths externalCompilerPaths, CancellationToken ct, @@ -151,20 +153,20 @@ private static CompilerMessage[] TryCompileWithRetries( { for (int attempt = 1; attempt <= SharedCompilerWorkerMaxAttempts; attempt++) { - WorkerAttemptResult attemptResult = TryCompileOnce( + WorkerAttemptResult attemptResult = await TryCompileOnceAsync( requestFilePath, externalCompilerPaths, ct, markBuildStarted, markBuildFinished, - incrementBuildCount); + incrementBuildCount).ConfigureAwait(false); if (attemptResult.Succeeded) { return attemptResult.Messages; } - ServiceValue.ShutdownProcessLocked(); + ServiceValue.ExecuteWithStateLock(ServiceValue.ShutdownProcessLocked); if (attemptResult.ShouldRetry && attempt < SharedCompilerWorkerMaxAttempts) { @@ -180,7 +182,7 @@ private static CompilerMessage[] TryCompileWithRetries( return null; } - private static WorkerAttemptResult TryCompileOnce( + private static async Task TryCompileOnceAsync( string requestFilePath, ExternalCompilerPaths externalCompilerPaths, CancellationToken ct, @@ -196,17 +198,17 @@ private static WorkerAttemptResult TryCompileOnce( startupResult.FailureContext); } - return InvokeWorkerOnce( + return await InvokeWorkerOnceAsync( requestFilePath, ct, markBuildStarted, markBuildFinished, - incrementBuildCount); + incrementBuildCount).ConfigureAwait(false); } private static WorkerStartupResult EnsureWorkerReady(ExternalCompilerPaths externalCompilerPaths) { - if (ServiceValue.HasLiveProcessLocked()) + if (ServiceValue.ExecuteWithStateLock(ServiceValue.HasLiveProcessLocked)) { return WorkerStartupResult.Ready(); } @@ -234,8 +236,10 @@ private static WorkerStartupResult EnsureWorkerAssemblyBuilt( return WorkerStartupResult.Ready(); } + // Why outside state lock: worker DLL compile can take seconds; shutdown must still kill + // an already-running shared worker without waiting on this build. SharedRoslynCompilerWorkerAssemblyBuilder.WorkerAssemblyBuildResult buildResult = - ServiceValue.CompileWorkerAssemblyLocked( + ServiceValue.CompileWorkerAssembly( externalCompilerPaths, workerPaths.SourcePath, workerPaths.AssemblyPath, @@ -267,7 +271,9 @@ private static WorkerStartupResult StartWorkerProcess( WorkerPaths workerPaths) { ProcessStartInfo startInfo = CreateWorkerStartInfo(externalCompilerPaths, workerPaths); - if (!ServiceValue.StartProcessLocked(startInfo)) + bool started = ServiceValue.ExecuteWithStateLock( + () => ServiceValue.StartProcessLocked(startInfo)); + if (!started) { return WorkerStartupResult.Failure( "worker_start_failed", @@ -281,14 +287,28 @@ private static WorkerStartupResult StartWorkerProcess( return WorkerStartupResult.Ready(); } - private static WorkerAttemptResult InvokeWorkerOnce( + private static async Task InvokeWorkerOnceAsync( string requestFilePath, CancellationToken ct, Action markBuildStarted, Action markBuildFinished, Action incrementBuildCount) { - if (!ServiceValue.HasLiveProcessLocked()) + StreamReader reader = null; + // Why not send here: stdin WriteLine/Flush can throw IOException if the worker dies + // after HasLiveProcessLocked; keep send inside the retryable try below. + bool prepared = ServiceValue.ExecuteWithStateLock(() => + { + if (!ServiceValue.HasLiveProcessLocked()) + { + return false; + } + + reader = ServiceValue.GetOutputReaderLocked(); + return true; + }); + + if (!prepared) { return WorkerAttemptResult.RetryableFailure( "worker_process_missing", @@ -301,8 +321,8 @@ private static WorkerAttemptResult InvokeWorkerOnce( try { - SendCompileRequest(requestFilePath); - return ReadWorkerResponse(requestFilePath, ct); + ServiceValue.ExecuteWithStateLock(() => SendCompileRequestLocked(requestFilePath)); + return await ReadWorkerResponseAsync(requestFilePath, reader, ct).ConfigureAwait(false); } catch (IOException ex) { @@ -314,7 +334,7 @@ private static WorkerAttemptResult InvokeWorkerOnce( } catch (OperationCanceledException) { - ServiceValue.ShutdownProcessLocked(); + ServiceValue.ExecuteWithStateLock(ServiceValue.ShutdownProcessLocked); throw; } finally @@ -323,22 +343,26 @@ private static WorkerAttemptResult InvokeWorkerOnce( } } - private static void SendCompileRequest(string requestFilePath) + private static void SendCompileRequestLocked(string requestFilePath) { string absoluteRequestFilePath = Path.GetFullPath(requestFilePath); ServiceValue.SendCompileRequestLocked(absoluteRequestFilePath); } - private static WorkerAttemptResult ReadWorkerResponse( + private static async Task ReadWorkerResponseAsync( string requestFilePath, + StreamReader reader, CancellationToken ct) { - StreamReader reader = ServiceValue.GetOutputReaderLocked(); - string responseHeader = SharedRoslynCompilerWorkerProtocol.ReadProtocolLine( + int timeoutMilliseconds = ServiceValue.ResponseTimeoutMilliseconds; + string responseHeader = await SharedRoslynCompilerWorkerProtocol.ReadProtocolLineAsync( reader, - ct); + ct, + timeoutMilliseconds).ConfigureAwait(false); if (string.IsNullOrEmpty(responseHeader)) { + // Why kill immediately: abandoned ReadLine tasks unblock only after the pipe closes. + ServiceValue.ExecuteWithStateLock(ServiceValue.ShutdownProcessLocked); return WorkerAttemptResult.RetryableFailure( "worker_empty_header", new { request_file_path = requestFilePath }); @@ -346,21 +370,29 @@ private static WorkerAttemptResult ReadWorkerResponse( if (!SharedRoslynCompilerWorkerProtocol.TryParseResponseHeader(responseHeader, out int exitCode)) { + ServiceValue.ExecuteWithStateLock(ServiceValue.ShutdownProcessLocked); return WorkerAttemptResult.RetryableFailure( SharedRoslynCompilerWorkerProtocol.GetResponseHeaderFailureReason(responseHeader), new { header = responseHeader }); } - List outputLines = SharedRoslynCompilerWorkerProtocol.ReadDiagnosticLines(reader, ct); + List outputLines = await SharedRoslynCompilerWorkerProtocol.ReadDiagnosticLinesAsync( + reader, + ct, + timeoutMilliseconds).ConfigureAwait(false); if (outputLines == null) { + ServiceValue.ExecuteWithStateLock(ServiceValue.ShutdownProcessLocked); return WorkerAttemptResult.RetryableFailure( "worker_missing_end_marker", new { request_file_path = requestFilePath }); } string combinedOutput = string.Join("\n", outputLines); - CompilerMessage[] compilerMessages = ExternalCompilerMessageParser.Parse(combinedOutput, string.Empty, exitCode); + CompilerMessage[] compilerMessages = ExternalCompilerMessageParser.Parse( + combinedOutput, + string.Empty, + exitCode); return WorkerAttemptResult.Successful(compilerMessages); } @@ -368,7 +400,8 @@ private static WorkerPaths CreateWorkerPaths() { string workerDirectoryPath = GetWorkerDirectoryPath(); Directory.CreateDirectory(workerDirectoryPath); - ServiceValue.RecordWorkerDirectoryLocked(workerDirectoryPath); + ServiceValue.ExecuteWithStateLock( + () => ServiceValue.RecordWorkerDirectoryLocked(workerDirectoryPath)); return new WorkerPaths( workerDirectoryPath, Path.Combine(workerDirectoryPath, RoslynWorkerSourceFileName), @@ -403,7 +436,8 @@ private static ProcessStartInfo CreateWorkerStartInfo( ExternalCompilerPaths externalCompilerPaths, WorkerPaths workerPaths) { - ProcessStartInfo startInfo = new() { + ProcessStartInfo startInfo = new() + { FileName = externalCompilerPaths.DotnetHostPath, Arguments = "exec" + " --runtimeconfig " + SharedRoslynCompilerWorkerAssemblyBuilder.QuoteCommandLineArgument(externalCompilerPaths.CompilerRuntimeConfigPath) @@ -479,6 +513,11 @@ internal static Func + /// Async line reader for the shared Roslyn worker stdout protocol. + /// Kept free of UnityEngine so pure unit tests can cover timeout and abandonment. + /// + internal static class SharedRoslynCompilerWorkerLineReader + { + public const int DefaultResponseTimeoutMilliseconds = 30000; + + /// + /// Reads one line with an upper bound. On timeout or cancel, the in-flight ReadLine task is + /// observed but not awaited; callers must close the stream/process so ReadLine can finish. + /// + public static async Task ReadLineAsync( + TextReader reader, + CancellationToken ct, + int timeoutMilliseconds = DefaultResponseTimeoutMilliseconds) + { + Debug.Assert(reader != null, "reader must not be null"); + Debug.Assert(timeoutMilliseconds > 0, "timeoutMilliseconds must be positive"); + ct.ThrowIfCancellationRequested(); + + // Why Task.Run: StreamReader.ReadLine has no cancel token; isolation lets timeout return + // while the blocking read continues until the caller closes the pipe/process. + Task readTask = Task.Run(() => reader.ReadLine(), CancellationToken.None); + using CancellationTokenSource timeoutCancellationTokenSource = + CancellationTokenSource.CreateLinkedTokenSource(ct); + Task delayTask = Task.Delay(timeoutMilliseconds, timeoutCancellationTokenSource.Token); + // Why observe delay faults: WhenAny leaves a canceled/faulted delay unobserved otherwise. + _ = delayTask.ContinueWith( + static observedTask => _ = observedTask.Exception, + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + + Task completedTask = await Task.WhenAny(readTask, delayTask).ConfigureAwait(false); + if (ReferenceEquals(completedTask, readTask)) + { + timeoutCancellationTokenSource.Cancel(); + return await readTask.ConfigureAwait(false); + } + + ObserveAbandonedRead(readTask); + ct.ThrowIfCancellationRequested(); + return null; + } + + public static async Task> ReadDiagnosticLinesAsync( + TextReader reader, + string endMarker, + CancellationToken ct, + int timeoutMilliseconds = DefaultResponseTimeoutMilliseconds) + { + Debug.Assert(reader != null, "reader must not be null"); + Debug.Assert(!string.IsNullOrEmpty(endMarker), "endMarker must not be empty"); + + List outputLines = new(); + while (true) + { + string outputLine = await ReadLineAsync(reader, ct, timeoutMilliseconds) + .ConfigureAwait(false); + if (outputLine == null) + { + return null; + } + + if (outputLine == endMarker) + { + return outputLines; + } + + outputLines.Add(outputLine); + } + } + + private static void ObserveAbandonedRead(Task readTask) + { + _ = readTask.ContinueWith( + static observedTask => _ = observedTask.Exception, + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerLineReader.cs.meta b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerLineReader.cs.meta new file mode 100644 index 000000000..d99ea4b7d --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerLineReader.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: e2a7c4b18d9f4e6a0b3c5d7e9f1a2b4c +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerProtocol.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerProtocol.cs index 8379a631a..0c6f94f7a 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerProtocol.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerProtocol.cs @@ -25,7 +25,6 @@ internal static class SharedRoslynCompilerWorkerProtocol private const string SharedCompilerWorkerEndMarkerToken = "{{SHARED_COMPILER_WORKER_END_MARKER}}"; private const string SharedCompilerWorkerQuitCommandToken = "{{SHARED_COMPILER_WORKER_QUIT_COMMAND}}"; private const string CompileRequestPathPrefixToken = "{{COMPILE_REQUEST_PATH_PREFIX}}"; - private const int SharedCompilerWorkerResponseTimeoutMilliseconds = 30000; internal static string CreateCompileRequestCommand(string requestFilePath) { @@ -59,40 +58,24 @@ internal static string GetResponseHeaderFailureReason(string responseHeader) return "worker_invalid_exit_code"; } - internal static List ReadDiagnosticLines(StreamReader reader, CancellationToken ct) + internal static Task> ReadDiagnosticLinesAsync( + TextReader reader, + CancellationToken ct, + int timeoutMilliseconds = SharedRoslynCompilerWorkerLineReader.DefaultResponseTimeoutMilliseconds) { - List outputLines = new(); - while (true) - { - string outputLine = ReadProtocolLine(reader, ct); - if (outputLine == null) - { - return null; - } - - if (outputLine == SharedCompilerWorkerEndMarker) - { - return outputLines; - } - - outputLines.Add(outputLine); - } + return SharedRoslynCompilerWorkerLineReader.ReadDiagnosticLinesAsync( + reader, + SharedCompilerWorkerEndMarker, + ct, + timeoutMilliseconds); } - internal static string ReadProtocolLine(StreamReader reader, CancellationToken ct) + internal static Task ReadProtocolLineAsync( + TextReader reader, + CancellationToken ct, + int timeoutMilliseconds = SharedRoslynCompilerWorkerLineReader.DefaultResponseTimeoutMilliseconds) { - Debug.Assert(reader != null, "reader must not be null"); - - Task readTask = Task.Run(() => reader.ReadLine()); - Task timeoutTask = Task.Delay(SharedCompilerWorkerResponseTimeoutMilliseconds, ct); - Task completedTask = Task.WhenAny(readTask, timeoutTask).GetAwaiter().GetResult(); - if (!ReferenceEquals(completedTask, readTask)) - { - ct.ThrowIfCancellationRequested(); - return null; - } - - return readTask.GetAwaiter().GetResult(); + return SharedRoslynCompilerWorkerLineReader.ReadLineAsync(reader, ct, timeoutMilliseconds); } internal static string CreateProgramSource() diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs index 8b4b71a4f..a2f6e1bc6 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs @@ -3,6 +3,7 @@ using System.Diagnostics; using System.IO; using System.Threading; +using System.Threading.Tasks; using UnityEditor.Compilation; using Debug = UnityEngine.Debug; @@ -12,36 +13,79 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { /// /// Owns the shared Roslyn compiler worker process, temporary directory, and synchronized lifecycle. + /// Compile conversations are serialized with an async gate; process state uses a short sync lock + /// so shutdown can kill the worker without waiting for an in-flight read. /// internal sealed class SharedRoslynCompilerWorkerSession { - private readonly object _syncRoot = new(); + private readonly SharedRoslynCompilerWorkerSessionCoordination _coordination = new(); private Func _startProcess = ProcessStartHelper.TryStart; private Action _sendCompileRequest = SendCompileRequestCore; private Func _compileWorkerAssemblyForTests; private Process _workerProcess; private string _workerDirectoryPath; + private int _responseTimeoutMilliseconds = + SharedRoslynCompilerWorkerLineReader.DefaultResponseTimeoutMilliseconds; + + /// + /// Serializes worker request/response conversations without holding the state lock across awaits. + /// + internal Task RunSerializedCompileAsync( + Func> operation, + CancellationToken ct) + { + return _coordination.RunSerializedCompileAsync(operation, ct); + } + + /// + /// Runs a short critical section over process/directory state. + /// + internal T ExecuteWithStateLock(Func operation) + { + return _coordination.ExecuteWithStateLock(operation); + } + + internal void ExecuteWithStateLock(Action operation) + { + _coordination.ExecuteWithStateLock(operation); + } + /// + /// Legacy sync entry used by EditMode tests that only touch process start/dispose. + /// internal T ExecuteLocked(Func operation) { - Debug.Assert(operation != null, "operation must not be null"); + return ExecuteWithStateLock(operation); + } - lock (_syncRoot) + internal int ResponseTimeoutMilliseconds + { + get { - return operation(); + return _coordination.ExecuteWithStateLock(() => _responseTimeoutMilliseconds); } } + internal void SetResponseTimeoutMillisecondsForTests(int timeoutMilliseconds) + { + Debug.Assert(timeoutMilliseconds > 0, "timeoutMilliseconds must be positive"); + + _coordination.ExecuteWithStateLock(() => + { + _responseTimeoutMilliseconds = timeoutMilliseconds; + }); + } + internal bool HasLiveProcessLocked() { - AssertLockHeld(); + AssertStateLockHeld(); return _workerProcess != null && !_workerProcess.HasExited; } internal bool StartProcessLocked(ProcessStartInfo startInfo) { - AssertLockHeld(); + AssertStateLockHeld(); // EnsureWorkerReady reaches this method only after the same lock observed no live process, // so replacement releases the stale handle without retrying graceful shutdown. Process previousProcess = _workerProcess; @@ -54,29 +98,28 @@ internal bool StartProcessLocked(ProcessStartInfo startInfo) internal void SendCompileRequestLocked(string requestFilePath) { - AssertLockHeld(); + AssertStateLockHeld(); _sendCompileRequest(_workerProcess, requestFilePath); } internal StreamReader GetOutputReaderLocked() { - AssertLockHeld(); + AssertStateLockHeld(); return _workerProcess.StandardOutput; } internal void RecordWorkerDirectoryLocked(string workerDirectoryPath) { - AssertLockHeld(); + AssertStateLockHeld(); _workerDirectoryPath = workerDirectoryPath; } - internal SharedRoslynCompilerWorkerAssemblyBuilder.WorkerAssemblyBuildResult CompileWorkerAssemblyLocked( + internal SharedRoslynCompilerWorkerAssemblyBuilder.WorkerAssemblyBuildResult CompileWorkerAssembly( ExternalCompilerPaths externalCompilerPaths, string workerSourcePath, string workerAssemblyPath, string workerCompileResponseFilePath) { - AssertLockHeld(); if (_compileWorkerAssemblyForTests != null) { return SharedRoslynCompilerWorkerAssemblyBuilder.WorkerAssemblyBuildResult.Started( @@ -96,7 +139,7 @@ internal SharedRoslynCompilerWorkerAssemblyBuilder.WorkerAssemblyBuildResult Com internal void ShutdownProcessLocked() { - AssertLockHeld(); + AssertStateLockHeld(); Process workerProcess = _workerProcess; _workerProcess = null; if (workerProcess == null) @@ -218,13 +261,17 @@ private static void TryForceKill( } } + /// + /// Shuts down the worker without waiting for the compile gate. + /// In-flight readers fail fast when the process pipes close. + /// internal void Shutdown(string fallbackWorkerDirectoryPath) { - lock (_syncRoot) + _coordination.RunShutdownWithoutCompileGate(() => { ShutdownProcessLocked(); CleanupWorkerDirectoryLocked(fallbackWorkerDirectoryPath); - } + }); } internal Func SwapProcessStarterForTests( @@ -265,7 +312,7 @@ private static void SendCompileRequestCore(Process workerProcess, string request private void CleanupWorkerDirectoryLocked(string fallbackWorkerDirectoryPath) { - AssertLockHeld(); + AssertStateLockHeld(); string workerDirectoryPath = _workerDirectoryPath ?? fallbackWorkerDirectoryPath; if (!Directory.Exists(workerDirectoryPath)) { @@ -323,9 +370,9 @@ private static void LogWorkerShutdownFailure(Exception ex) aiTodo: "Investigate repeated worker shutdown communication failures if shared compilation stops recovering cleanly."); } - private void AssertLockHeld() + private void AssertStateLockHeld() { - Debug.Assert(Monitor.IsEntered(_syncRoot), "Shared worker session lock must be held"); + _coordination.AssertStateLockHeld(); } } } diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSessionCoordination.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSessionCoordination.cs new file mode 100644 index 000000000..9a070baf2 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSessionCoordination.cs @@ -0,0 +1,80 @@ +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Separates async compile conversation serialization from short process-state locking + /// so shutdown can kill the worker without waiting on an in-flight ReadLine. + /// Kept free of UnityEngine so pure unit tests can cover the shutdown interrupt path. + /// + internal sealed class SharedRoslynCompilerWorkerSessionCoordination + { + private readonly object _syncRoot = new(); + private readonly SemaphoreSlim _compileGate = new(1, 1); + + /// + /// Serializes worker request/response conversations without holding the state lock across awaits. + /// + public async Task RunSerializedCompileAsync( + Func> operation, + CancellationToken ct) + { + Debug.Assert(operation != null, "operation must not be null"); + + await _compileGate.WaitAsync(ct).ConfigureAwait(false); + try + { + return await operation(ct).ConfigureAwait(false); + } + finally + { + _compileGate.Release(); + } + } + + /// + /// Runs a short critical section over process/directory state. + /// Why not hold this across ReadLine: shutdown must be able to kill the worker while a read waits. + /// + public T ExecuteWithStateLock(Func operation) + { + Debug.Assert(operation != null, "operation must not be null"); + + lock (_syncRoot) + { + return operation(); + } + } + + public void ExecuteWithStateLock(Action operation) + { + Debug.Assert(operation != null, "operation must not be null"); + + lock (_syncRoot) + { + operation(); + } + } + + /// + /// Runs shutdown under the state lock without acquiring the compile gate. + /// + public void RunShutdownWithoutCompileGate(Action shutdownUnderStateLock) + { + Debug.Assert(shutdownUnderStateLock != null, "shutdownUnderStateLock must not be null"); + + lock (_syncRoot) + { + shutdownUnderStateLock(); + } + } + + public void AssertStateLockHeld() + { + Debug.Assert(Monitor.IsEntered(_syncRoot), "Shared worker session state lock must be held"); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSessionCoordination.cs.meta b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSessionCoordination.cs.meta new file mode 100644 index 000000000..b432dbef1 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSessionCoordination.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a8f3c1d0e4b64f2a9c7e5d1b0a6f8e2c +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrapperTemplate.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrapperTemplate.cs index 92d2e6c3f..fb642d58c 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrapperTemplate.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrapperTemplate.cs @@ -78,12 +78,9 @@ public static string Build( sb.AppendLine(UserCodeEndMarker); sb.AppendLine("#line hidden"); sb.AppendLine(" }"); - sb.AppendLine(); - sb.AppendLine(" public object Execute("); - sb.AppendLine(" System.Collections.Generic.Dictionary parameters = null)"); - sb.AppendLine(" {"); - sb.AppendLine(" return ExecuteAsync(parameters, default).GetAwaiter().GetResult();"); - sb.AppendLine(" }"); + // Why no sync Execute(): the runtime always prefers ExecuteAsync and passes the real + // combined CancellationToken. A sync wrapper that called ExecuteAsync(..., default) + // discarded that token and added an unused sync-over-async landmine. sb.AppendLine(" }"); sb.AppendLine("}"); diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs index 282557b00..1ee8da37e 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs @@ -53,37 +53,6 @@ private CancellationTokenSource CreateCombinedCancellationTokenSource(ExecutionC ); } - public ExecutionResult Execute(ExecutionContext context) - { - string correlationId = UnityCliLoopConstants.GenerateCorrelationId(); - if (!TryBeginExecution(out int undoGroup)) - { - return CreateErrorResult(UnityCliLoopConstants.ERROR_MESSAGE_EXECUTION_IN_PROGRESS); - } - - try - { - using CancellationTokenSource combinedCts = CreateCombinedCancellationTokenSource(context); - return ExecuteInternal(context, combinedCts.Token); - } - catch (OperationCanceledException) - { - return CreateCancelledResult(); - } - catch (Exception ex) - { - LogExecutionError(ex, correlationId); - - return CreateErrorResult( - ex.Message, - new List { $"Exception: {ex.Message}" }); - } - finally - { - EndExecution(undoGroup); - } - } - public void Cancel() { _cancellationTokenSource?.Cancel(); diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionFacade.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionFacade.cs index b8b983c73..bfe9f1d38 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionFacade.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionFacade.cs @@ -20,7 +20,12 @@ internal sealed class DynamicCodeExecutionFacade : IShutdownAwareDynamicCodeExec public DynamicCodeExecutionFacade(IDynamicCodeExecutorPool executorPool) { _executorPool = executorPool ?? throw new ArgumentNullException(nameof(executorPool)); - _executionScheduler = new DynamicCodeExecutionScheduler(_executorPool.Dispose); + DynamicCodeExecutionSchedulerHooks hooks = new() + { + // Why inject: scheduler stays pure C# for unit tests; Unity logging belongs at the facade edge. + LogWarning = message => Debug.LogWarning(message) + }; + _executionScheduler = new DynamicCodeExecutionScheduler(_executorPool.Dispose, hooks); } public async Task ExecuteAsync( diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionScheduler.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionScheduler.cs index e0c33fb96..74e1738de 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionScheduler.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionScheduler.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.Threading; using System.Threading.Tasks; @@ -11,11 +12,13 @@ internal sealed class DynamicCodeExecutionScheduler : IDisposable { private const int BusyHandoffWindowMilliseconds = 50; private const int CancelledPrewarmHandoffWindowMilliseconds = 500; + private const int DefaultShutdownTimeoutMilliseconds = 5000; private readonly Action _disposeResources; private readonly DynamicCodeExecutionSchedulerHooks _hooks; private readonly int _busyHandoffWindowMilliseconds; private readonly int _cancelledPrewarmHandoffWindowMilliseconds; + private readonly int _shutdownTimeoutMilliseconds; private readonly SemaphoreSlim _executionSemaphore = new(1, 1); private readonly CancellationTokenSource _lifetimeCancellationTokenSource = new(); private readonly TaskCompletionSource _shutdownCompletionSource = @@ -32,12 +35,20 @@ public DynamicCodeExecutionScheduler( Action disposeResources, DynamicCodeExecutionSchedulerHooks hooks = null, int busyHandoffWindowMilliseconds = BusyHandoffWindowMilliseconds, - int cancelledPrewarmHandoffWindowMilliseconds = CancelledPrewarmHandoffWindowMilliseconds) + int cancelledPrewarmHandoffWindowMilliseconds = CancelledPrewarmHandoffWindowMilliseconds, + int shutdownTimeoutMilliseconds = DefaultShutdownTimeoutMilliseconds) { + Debug.Assert(busyHandoffWindowMilliseconds > 0, "busyHandoffWindowMilliseconds must be positive"); + Debug.Assert( + cancelledPrewarmHandoffWindowMilliseconds > 0, + "cancelledPrewarmHandoffWindowMilliseconds must be positive"); + Debug.Assert(shutdownTimeoutMilliseconds > 0, "shutdownTimeoutMilliseconds must be positive"); + _disposeResources = disposeResources ?? throw new ArgumentNullException(nameof(disposeResources)); _hooks = hooks ?? new DynamicCodeExecutionSchedulerHooks(); _busyHandoffWindowMilliseconds = busyHandoffWindowMilliseconds; _cancelledPrewarmHandoffWindowMilliseconds = cancelledPrewarmHandoffWindowMilliseconds; + _shutdownTimeoutMilliseconds = shutdownTimeoutMilliseconds; } public void ThrowIfDisposed() @@ -168,10 +179,43 @@ public void Dispose() } } - public Task ShutdownAsync() + /// + /// Cancels in-flight work and waits for resource dispose up to the shutdown timeout. + /// Why timeout then TrySetResult: waiters must unblock even when user code ignores + /// cancellation; pool dispose stays deferred to the running action's finally. + /// + public async Task ShutdownAsync() { Dispose(); - return _shutdownCompletionSource.Task; + if (_shutdownCompletionSource.Task.IsCompleted) + { + return; + } + + using CancellationTokenSource timeoutCancellationTokenSource = new(); + Task delayTask = Task.Delay( + _shutdownTimeoutMilliseconds, + timeoutCancellationTokenSource.Token); + // Why observe via ContinueWith: canceling Delay leaves a canceled task; observing it + // avoids unobserved-task noise without a try/catch around await. + _ = delayTask.ContinueWith( + _ => { }, + CancellationToken.None, + TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + + Task completedTask = await Task.WhenAny(_shutdownCompletionSource.Task, delayTask); + if (completedTask == _shutdownCompletionSource.Task) + { + timeoutCancellationTokenSource.Cancel(); + return; + } + + _hooks.InvokeLogWarning( + "Dynamic code scheduler shutdown drain timed out after " + + _shutdownTimeoutMilliseconds + + "ms; executor pool dispose is deferred until the running action reaches its finally."); + _shutdownCompletionSource.TrySetResult(true); } private async Task<(bool Entered, T Result)> TryRunWithoutForegroundYieldAsync( diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionSchedulerHooks.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionSchedulerHooks.cs index e9551cf60..e4ab17150 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionSchedulerHooks.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionSchedulerHooks.cs @@ -14,6 +14,12 @@ internal sealed class DynamicCodeExecutionSchedulerHooks public Func AfterBusySemaphoreProbeFailedAsync { get; set; } + /// + /// Optional warning sink. Why not Debug.LogWarning here: the scheduler is pure C# and + /// must stay Unity-free for the dedicated unit-test project. + /// + public Action LogWarning { get; set; } + public async Task InvokeAfterBackgroundExecutionStatePublishedAsync() { if (AfterBackgroundExecutionStatePublishedAsync == null) @@ -33,5 +39,10 @@ public async Task InvokeAfterBusySemaphoreProbeFailedAsync() await AfterBusySemaphoreProbeFailedAsync(); } + + public void InvokeLogWarning(string message) + { + LogWarning?.Invoke(message); + } } } diff --git a/tests/DynamicCodeExecutionScheduler.UnitTests/DynamicCodeExecutionSchedulerTests.cs b/tests/DynamicCodeExecutionScheduler.UnitTests/DynamicCodeExecutionSchedulerTests.cs index a42ea5847..6d796d716 100644 --- a/tests/DynamicCodeExecutionScheduler.UnitTests/DynamicCodeExecutionSchedulerTests.cs +++ b/tests/DynamicCodeExecutionScheduler.UnitTests/DynamicCodeExecutionSchedulerTests.cs @@ -214,15 +214,110 @@ public void RunForegroundAsync_WhenDisposedAfterSemaphoreAcquire_ShouldThrowObje } } + [Test] + public async Task ShutdownAsync_WhenIdle_ShouldDisposeResourcesAndComplete() + { + // Verifies idle shutdown completes immediately and disposes resources once. + int disposeCalls = 0; + DynamicCodeExecutionScheduler scheduler = CreateScheduler( + disposeResources: () => disposeCalls++); + + await scheduler.ShutdownAsync(); + + Assert.That(disposeCalls, Is.EqualTo(1)); + Assert.That(scheduler.ShutdownAsync().IsCompletedSuccessfully, Is.True); + } + + [Test] + public async Task ShutdownAsync_WhenRunningActionObservesCancellation_ShouldDisposeBeforeTimeout() + { + // Verifies cooperative cancellation lets shutdown finish without hitting the timeout path. + int disposeCalls = 0; + System.Collections.Generic.List warnings = new(); + DynamicCodeExecutionSchedulerHooks hooks = new() + { + LogWarning = message => warnings.Add(message) + }; + DynamicCodeExecutionScheduler scheduler = CreateScheduler( + hooks, + () => disposeCalls++, + shutdownTimeoutMilliseconds: 1000); + + TaskCompletionSource executionStarted = + new(TaskCreationOptions.RunContinuationsAsynchronously); + Task executionTask = scheduler.RunForegroundAsync( + async cancellationToken => + { + executionStarted.TrySetResult(true); + await WaitForCancellationAsync(cancellationToken); + return "canceled"; + }, + () => "busy", + CancellationToken.None); + + await executionStarted.Task; + await scheduler.ShutdownAsync(); + + Assert.That(async () => await executionTask, Throws.InstanceOf()); + Assert.That(disposeCalls, Is.EqualTo(1)); + Assert.That(warnings, Is.Empty); + } + + [Test] + public async Task ShutdownAsync_WhenRunningActionIgnoresCancellation_ShouldCompleteAfterTimeoutAndDeferDispose() + { + // Verifies timeout unblocks shutdown while pool dispose waits for the late finally. + int disposeCalls = 0; + System.Collections.Generic.List warnings = new(); + DynamicCodeExecutionSchedulerHooks hooks = new() + { + LogWarning = message => warnings.Add(message) + }; + DynamicCodeExecutionScheduler scheduler = CreateScheduler( + hooks, + () => disposeCalls++, + shutdownTimeoutMilliseconds: 40); + + TaskCompletionSource executionStarted = + new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource allowExecutionToComplete = + new(TaskCreationOptions.RunContinuationsAsynchronously); + Task executionTask = scheduler.RunForegroundAsync( + async _ => + { + executionStarted.TrySetResult(true); + await allowExecutionToComplete.Task; + return "late"; + }, + () => "busy", + CancellationToken.None); + + await executionStarted.Task; + await scheduler.ShutdownAsync(); + + Assert.That(disposeCalls, Is.EqualTo(0)); + Assert.That(warnings, Has.Count.EqualTo(1)); + Assert.That(warnings[0], Does.Contain("timed out after 40ms")); + Assert.That(warnings[0], Does.Contain("deferred until the running action reaches its finally")); + + allowExecutionToComplete.TrySetResult(true); + string result = await executionTask; + + Assert.That(result, Is.EqualTo("late")); + Assert.That(disposeCalls, Is.EqualTo(1)); + } + private static DynamicCodeExecutionScheduler CreateScheduler( DynamicCodeExecutionSchedulerHooks hooks = null, - Action disposeResources = null) + Action disposeResources = null, + int shutdownTimeoutMilliseconds = 1000) { return new DynamicCodeExecutionScheduler( disposeResources ?? (() => { }), hooks, busyHandoffWindowMilliseconds: 20, - cancelledPrewarmHandoffWindowMilliseconds: 40); + cancelledPrewarmHandoffWindowMilliseconds: 40, + shutdownTimeoutMilliseconds: shutdownTimeoutMilliseconds); } private static async Task WaitForCancellationAsync(CancellationToken cancellationToken) diff --git a/tests/SharedRoslynCompilerWorker.UnitTests/SharedRoslynCompilerWorker.UnitTests.csproj b/tests/SharedRoslynCompilerWorker.UnitTests/SharedRoslynCompilerWorker.UnitTests.csproj new file mode 100644 index 000000000..4de203e0c --- /dev/null +++ b/tests/SharedRoslynCompilerWorker.UnitTests/SharedRoslynCompilerWorker.UnitTests.csproj @@ -0,0 +1,20 @@ + + + net10.0 + disable + disable + false + latest + + + + + + + + + + + + + diff --git a/tests/SharedRoslynCompilerWorker.UnitTests/SharedRoslynCompilerWorkerLineReaderTests.cs b/tests/SharedRoslynCompilerWorker.UnitTests/SharedRoslynCompilerWorkerLineReaderTests.cs new file mode 100644 index 000000000..80f3eb60c --- /dev/null +++ b/tests/SharedRoslynCompilerWorker.UnitTests/SharedRoslynCompilerWorkerLineReaderTests.cs @@ -0,0 +1,220 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.IO.Pipes; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.UnitTests +{ + /// + /// Pure unit coverage for async Roslyn worker line reading and compile-gate shutdown. + /// + [TestFixture] + public class SharedRoslynCompilerWorkerLineReaderTests + { + [Test] + public async Task ReadLineAsync_WhenLineAvailable_ShouldReturnLine() + { + // Verifies a normal protocol line is returned without hitting the timeout path. + using StringReader reader = new("hello\n"); + + string line = await SharedRoslynCompilerWorkerLineReader.ReadLineAsync( + reader, + CancellationToken.None, + timeoutMilliseconds: 1000); + + Assert.That(line, Is.EqualTo("hello")); + } + + [Test] + public async Task ReadLineAsync_WhenNoLineWithinTimeout_ShouldReturnNull() + { + // Verifies timeout returns null while the abandoned ReadLine is only observed. + using AnonymousPipeServerStream server = new(PipeDirection.Out); + using AnonymousPipeClientStream client = new( + PipeDirection.In, + server.ClientSafePipeHandle); + using StreamReader reader = new(client); + + string line = await SharedRoslynCompilerWorkerLineReader.ReadLineAsync( + reader, + CancellationToken.None, + timeoutMilliseconds: 40); + + Assert.That(line, Is.Null); + server.Dispose(); + } + + [Test] + public void ReadLineAsync_WhenCanceled_ShouldThrowOperationCanceledException() + { + // Verifies cooperative cancel surfaces as OperationCanceledException before ReadLine starts. + using CancellationTokenSource cancellationTokenSource = new(); + cancellationTokenSource.Cancel(); + using StringReader reader = new(string.Empty); + + Assert.ThrowsAsync( + async () => await SharedRoslynCompilerWorkerLineReader.ReadLineAsync( + reader, + cancellationTokenSource.Token, + timeoutMilliseconds: 1000)); + } + + [Test] + public async Task ReadLineAsync_WhenCanceledDuringWait_ShouldThrowAndAllowPipeClose() + { + // Verifies mid-wait cancel returns without holding the caller on the abandoned ReadLine. + using AnonymousPipeServerStream server = new(PipeDirection.Out); + using AnonymousPipeClientStream client = new( + PipeDirection.In, + server.ClientSafePipeHandle); + using StreamReader reader = new(client); + using CancellationTokenSource cancellationTokenSource = new(); + Task readTask = SharedRoslynCompilerWorkerLineReader.ReadLineAsync( + reader, + cancellationTokenSource.Token, + timeoutMilliseconds: 5000); + + await Task.Delay(20); + cancellationTokenSource.Cancel(); + + Assert.ThrowsAsync(async () => await readTask); + server.Dispose(); + } + + [Test] + public async Task ReadDiagnosticLinesAsync_WhenEndMarkerArrives_ShouldReturnCollectedLines() + { + // Verifies diagnostic aggregation stops at the protocol end marker. + using StringReader reader = new("diag-a\ndiag-b\n__ULOOP_END__\n"); + + List lines = await SharedRoslynCompilerWorkerLineReader.ReadDiagnosticLinesAsync( + reader, + "__ULOOP_END__", + CancellationToken.None, + timeoutMilliseconds: 1000); + + Assert.That(lines, Is.EqualTo(new[] { "diag-a", "diag-b" })); + } + } + + [TestFixture] + public class SharedRoslynCompilerWorkerSessionCoordinationTests + { + [Test] + public async Task RunShutdownWithoutCompileGate_WhileCompileGateHeld_ShouldCompleteWithoutWaitingForGate() + { + // Verifies shutdown bypasses the async compile gate so a stuck read cannot block it. + SharedRoslynCompilerWorkerSessionCoordination coordination = new(); + TaskCompletionSource gateEntered = + new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource allowCompileToFinish = + new(TaskCreationOptions.RunContinuationsAsynchronously); + int shutdownStateLockEntries = 0; + + Task compileTask = coordination.RunSerializedCompileAsync( + async _ => + { + gateEntered.TrySetResult(true); + await allowCompileToFinish.Task; + return true; + }, + CancellationToken.None); + + await gateEntered.Task; + Stopwatch stopwatch = Stopwatch.StartNew(); + coordination.RunShutdownWithoutCompileGate(() => + { + coordination.AssertStateLockHeld(); + shutdownStateLockEntries++; + }); + stopwatch.Stop(); + + Assert.That(stopwatch.ElapsedMilliseconds, Is.LessThan(500)); + Assert.That(shutdownStateLockEntries, Is.EqualTo(1)); + + allowCompileToFinish.TrySetResult(true); + bool compileResult = await compileTask; + Assert.That(compileResult, Is.True); + } + + [Test] + public async Task RunSerializedCompileAsync_AfterShutdown_ShouldStillRunSerializedBody() + { + // Verifies a later compile conversation is not stuck after shutdown cleared process state. + SharedRoslynCompilerWorkerSessionCoordination coordination = new(); + bool processCleared = false; + + coordination.RunShutdownWithoutCompileGate(() => + { + processCleared = true; + }); + + bool ran = await coordination.RunSerializedCompileAsync( + _ => Task.FromResult(true), + CancellationToken.None); + + Assert.That(processCleared, Is.True); + Assert.That(ran, Is.True); + Assert.That( + coordination.ExecuteWithStateLock(() => processCleared), + Is.True); + } + + [Test] + public async Task RunSerializedCompileAsync_ShouldSerializeConversations() + { + // Verifies write→read conversations stay single-flight under the async gate. + SharedRoslynCompilerWorkerSessionCoordination coordination = new(); + int concurrent = 0; + int maxConcurrent = 0; + object concurrentLock = new(); + + Task first = coordination.RunSerializedCompileAsync( + async _ => + { + lock (concurrentLock) + { + concurrent++; + maxConcurrent = Math.Max(maxConcurrent, concurrent); + } + + await Task.Delay(40); + lock (concurrentLock) + { + concurrent--; + } + + return 1; + }, + CancellationToken.None); + + Task second = coordination.RunSerializedCompileAsync( + async _ => + { + lock (concurrentLock) + { + concurrent++; + maxConcurrent = Math.Max(maxConcurrent, concurrent); + } + + await Task.Delay(10); + lock (concurrentLock) + { + concurrent--; + } + + return 2; + }, + CancellationToken.None); + + int[] results = await Task.WhenAll(first, second); + Assert.That(results, Is.EquivalentTo(new[] { 1, 2 })); + Assert.That(maxConcurrent, Is.EqualTo(1)); + } + } +}