From 577a0a155d728a9284beceb302041a57ad84cfd5 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 14 Jun 2026 06:36:55 +0900 Subject: [PATCH 1/5] fix: keep screenshot frame waits from hanging Race screenshot frame waits against a timer-based timeout so stalled editor frames return a clear timeout response instead of leaving the CLI call busy. Keep typed tool continuations and execution-slot cleanup off captured Unity synchronization contexts, then switch screenshot continuations back through the main-thread dispatcher before touching Unity APIs. --- .../CaptureTest/EditorWindowCaptureTest.cs | 14 +- .../UnityCliLoopToolExecutionServiceTests.cs | 140 ++++++++++++++++++ .../UnityCliLoopToolExecutionService.cs | 2 +- .../Screenshot/EditorWindowCaptureUtility.cs | 81 +++++++++- .../Screenshot/ScreenshotResponse.cs | 2 + .../Screenshot/ScreenshotTool.cs | 7 +- .../Screenshot/ScreenshotUseCase.cs | 88 ++++++++++- ...p.FirstPartyTools.Screenshot.Editor.asmdef | 1 + .../Screenshot/UnityCliLoopScreenshotTypes.cs | 2 + .../Editor/ToolContracts/EditorFrameWaiter.cs | 42 ++++++ .../Editor/ToolContracts/UnityCliLoopTool.cs | 2 +- 11 files changed, 359 insertions(+), 22 deletions(-) diff --git a/Assets/Editor/CaptureTest/EditorWindowCaptureTest.cs b/Assets/Editor/CaptureTest/EditorWindowCaptureTest.cs index 0c04e852b9..166a31c49a 100644 --- a/Assets/Editor/CaptureTest/EditorWindowCaptureTest.cs +++ b/Assets/Editor/CaptureTest/EditorWindowCaptureTest.cs @@ -13,6 +13,8 @@ namespace io.github.hatayama.UnityCliLoop.Dev /// public class EditorWindowCaptureTest : EditorWindow { + private const int FRAME_WAIT_TIMEOUT_MILLISECONDS = 5000; + private string _windowName = "Console"; private WindowMatchMode _matchMode = WindowMatchMode.exact; private string _lastResult = ""; @@ -136,7 +138,16 @@ private async void CaptureWindowAsync() for (int i = 0; i < windows.Length; i++) { EditorWindow window = windows[i]; - Texture2D texture = await EditorWindowCaptureUtility.CaptureWindowAsync(window, 1.0f, CancellationToken.None); + (Texture2D texture, bool timedOut) = await EditorWindowCaptureUtility.CaptureWindowAsync( + window, + 1.0f, + FRAME_WAIT_TIMEOUT_MILLISECONDS, + CancellationToken.None); + if (timedOut) + { + sb.AppendLine($" [{i + 1}] Timed out while waiting for Editor frames"); + continue; + } if (texture == null) { @@ -193,4 +204,3 @@ private void OpenOutputFolder() } } } - diff --git a/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs b/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs index dfe04781c3..b92bf80c1e 100644 --- a/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs @@ -1,7 +1,14 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; using NUnit.Framework; using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.FirstPartyTools; using io.github.hatayama.UnityCliLoop.Infrastructure; +using io.github.hatayama.UnityCliLoop.ToolContracts; namespace io.github.hatayama.UnityCliLoop.Tests.Editor { @@ -37,6 +44,73 @@ public void CreateBusyException_WhenDispatcherReportsBackgroundThread_UsesEditor } } + [Test] + public async Task ExecuteToolAsync_WhenTypedToolCompletesUnderBlockedSynchronizationContext_ReleasesExecutionSlot() + { + // Verifies timeout-style tool completions are not trapped behind a captured Editor SynchronizationContext. + UnityCliLoopToolRegistry registry = ToolRegistryTestFactory.Create(); + PendingTypedTool pendingTool = new PendingTypedTool(); + registry.RegisterTool(pendingTool); + UnityCliLoopToolExecutionService executionService = new UnityCliLoopToolExecutionService(); + ImmediateMainThreadDispatcher dispatcher = new ImmediateMainThreadDispatcher(); + MainThreadSwitcher.RegisterService(dispatcher); + BlockingSynchronizationContext blockedContext = new BlockingSynchronizationContext(); + System.Threading.SynchronizationContext previousContext = + System.Threading.SynchronizationContext.Current; + + try + { + System.Threading.SynchronizationContext.SetSynchronizationContext(blockedContext); + Task firstTask = executionService.ExecuteToolAsync( + registry, + PendingTypedTool.Name, + null, + CancellationToken.None); + System.Threading.SynchronizationContext.SetSynchronizationContext(previousContext); + + Assert.That(firstTask.IsCompleted, Is.False); + + pendingTool.Complete(); + UnityCliLoopToolResponse firstResponse = await AwaitResponseWithTimeout( + firstTask, + CancellationToken.None); + + Assert.That(firstResponse, Is.InstanceOf()); + Assert.That(blockedContext.PostCount, Is.EqualTo(0)); + + UnityCliLoopToolResponse secondResponse = await executionService.ExecuteToolAsync( + registry, + PendingTypedTool.Name, + null, + CancellationToken.None); + + Assert.That(secondResponse, Is.InstanceOf()); + } + finally + { + System.Threading.SynchronizationContext.SetSynchronizationContext(previousContext); + RestoreEditorMainThreadDispatcher(); + } + } + + [Test] + public void ScreenshotTool_ToResponse_WhenCaptureTimedOut_PreservesTimeoutDetails() + { + // Verifies screenshot timeout results are visible to CLI callers without pretending an image was captured. + UnityCliLoopScreenshotResult result = new UnityCliLoopScreenshotResult + { + TimedOut = true, + Message = "Timed out while waiting for frames.", + Screenshots = new List(), + }; + + ScreenshotResponse response = ScreenshotTool.ToResponse(result); + + Assert.That(response.TimedOut, Is.True); + Assert.That(response.Message, Is.EqualTo("Timed out while waiting for frames.")); + Assert.That(response.ScreenshotCount, Is.EqualTo(0)); + } + private static void RestoreEditorMainThreadDispatcher() { EditorMainThreadDispatcher dispatcher = new EditorMainThreadDispatcher(); @@ -44,6 +118,18 @@ private static void RestoreEditorMainThreadDispatcher() dispatcher.Initialize(); } + private static async Task AwaitResponseWithTimeout( + Task task, + CancellationToken ct) + { + ct.ThrowIfCancellationRequested(); + + Task timeoutTask = Task.Delay(TimeSpan.FromSeconds(1), ct); + Task completedTask = await Task.WhenAny(task, timeoutTask); + Assert.That(completedTask, Is.SameAs(task), "Tool execution did not complete before the test timeout."); + return await task; + } + private sealed class CapturingMainThreadDispatcher : IMainThreadDispatcher { public bool IsMainThread => false; @@ -57,5 +143,59 @@ public void AddContinuation(System.Action continuation) Assert.That(continuation, Is.Not.Null); } } + + private sealed class ImmediateMainThreadDispatcher : IMainThreadDispatcher + { + public bool IsMainThread => true; + + public void Initialize() + { + } + + public void AddContinuation(Action continuation) + { + Assert.That(continuation, Is.Not.Null); + continuation(); + } + } + + private sealed class BlockingSynchronizationContext : System.Threading.SynchronizationContext + { + public int PostCount { get; private set; } + + public override void Post(SendOrPostCallback callback, object state) + { + Assert.That(callback, Is.Not.Null); + PostCount++; + } + } + + private sealed class PendingTypedTool : UnityCliLoopTool + { + public const string Name = "pending-typed-tool"; + + private readonly TaskCompletionSource _completionSource = + new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + public override string ToolName => Name; + + protected override Task ExecuteAsync(PendingTypedSchema parameters, CancellationToken ct) + { + return _completionSource.Task; + } + + public void Complete() + { + _completionSource.TrySetResult(new PendingTypedResponse()); + } + } + + private sealed class PendingTypedSchema : UnityCliLoopToolSchema + { + } + + private sealed class PendingTypedResponse : UnityCliLoopToolResponse + { + } } } diff --git a/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs b/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs index 5d67695078..681e7833fc 100644 --- a/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs +++ b/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs @@ -59,7 +59,7 @@ internal async Task ExecuteToolAsync( ct.ThrowIfCancellationRequested(); UnityCliLoopEditorStateGuard.Validate(toolName); - UnityCliLoopToolResponse response = await tool.ExecuteAsync(paramsToken, ct); + UnityCliLoopToolResponse response = await tool.ExecuteAsync(paramsToken, ct).ConfigureAwait(false); if (response == null) { throw new InvalidOperationException($"Tool returned null response: {toolName}"); diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/EditorWindowCaptureUtility.cs b/Packages/src/Editor/FirstPartyTools/Screenshot/EditorWindowCaptureUtility.cs index 892e0cfbeb..a27466e7f2 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/EditorWindowCaptureUtility.cs +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/EditorWindowCaptureUtility.cs @@ -6,6 +6,7 @@ using UnityEditor; using UnityEngine; +using io.github.hatayama.UnityCliLoop.Application; using io.github.hatayama.UnityCliLoop.InternalAPIBridge; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -65,16 +66,31 @@ public static EditorWindow[] FindWindowsByName(string windowName, WindowMatchMod /// Resolution scale (0.1 to 1.0) /// Cancellation token /// Captured Texture2D, or null if capture failed - public static async Task CaptureWindowAsync(EditorWindow window, float resolutionScale, CancellationToken ct) + public static async Task<(Texture2D? texture, bool timedOut)> CaptureWindowAsync( + EditorWindow window, + float resolutionScale, + int frameWaitTimeoutMilliseconds, + CancellationToken ct) { if (window == null) { - return null; + return (null, false); } + SynchronizationContext editorContext = + CapturedEditorSynchronizationContext.RequireCurrent("window screenshot capture"); window.ShowTab(); - await EditorFrameWaiter.WaitFramesAsync(2, ct); - return CaptureWindowInternal(window, resolutionScale); + bool framesReady = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( + 2, + frameWaitTimeoutMilliseconds, + ct).ConfigureAwait(false); + if (!framesReady) + { + return (null, true); + } + + await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); + return (CaptureWindowInternal(window, resolutionScale), false); } /// @@ -152,18 +168,32 @@ public static string[] GetOpenWindowNames() // Coordinate mapping to simulate-mouse: // sim_x = image_x / resolutionScale, sim_y = image_y / resolutionScale + YOffset // where YOffset = Screen.height - RenderTexture.height (returned in the tuple). - public static async Task<(Texture2D? texture, int yOffset)> CaptureGameRenderingAsync(float resolutionScale, CancellationToken ct) + public static async Task<(Texture2D? texture, int yOffset, bool timedOut)> CaptureGameRenderingAsync( + float resolutionScale, + int frameWaitTimeoutMilliseconds, + CancellationToken ct) { Debug.Assert(UnityEditor.EditorApplication.isPlaying, "CaptureGameRenderingAsync requires PlayMode"); + SynchronizationContext editorContext = + CapturedEditorSynchronizationContext.RequireCurrent("rendering screenshot capture"); // Wait for the game camera to complete at least one full render cycle after any state change - await EditorFrameWaiter.WaitFramesAsync(2, ct); + bool framesReady = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( + 2, + frameWaitTimeoutMilliseconds, + ct).ConfigureAwait(false); + if (!framesReady) + { + return (null, 0, true); + } + + await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); RenderTexture rt = GameViewBridge.GetRenderTexture(); if (rt == null) { Debug.LogWarning("[EditorWindowCaptureUtility] GameView RenderTexture is not available"); - return (null, 0); + return (null, 0, false); } int yOffset = (int)Handles.GetMainGameViewSize().y - rt.height; @@ -201,7 +231,7 @@ public static string[] GetOpenWindowNames() texture = ApplyResolutionScaling(texture, resolutionScale); } - return (texture, yOffset); + return (texture, yOffset, false); } private static Texture2D ApplyResolutionScaling(Texture2D originalTexture, float scale) @@ -234,4 +264,39 @@ private static Texture2D ApplyResolutionScaling(Texture2D originalTexture, float return scaledTexture; } } + + /// + /// Switches screenshot continuations back to the Editor synchronization context captured before a timeout race. + /// + internal static class CapturedEditorSynchronizationContext + { + public static SynchronizationContext RequireCurrent(string operationName) + { + Debug.Assert(!string.IsNullOrWhiteSpace(operationName), "operationName must not be null or whitespace"); + + SynchronizationContext context = SynchronizationContext.Current; + Debug.Assert(context != null, $"{operationName} must start on Unity's editor synchronization context."); + if (context == null) + { + throw new InvalidOperationException( + $"{operationName} must start on Unity's editor synchronization context."); + } + + return context; + } + + public static SwitchToMainThreadAwaitable SwitchTo( + SynchronizationContext context, + CancellationToken ct) + { + Debug.Assert(context != null, "context must not be null"); + + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + return MainThreadSwitcher.SwitchToMainThread(ct); + } + } } diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotResponse.cs b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotResponse.cs index 4d78095dd2..0b1dc78131 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotResponse.cs @@ -37,6 +37,8 @@ public class ScreenshotInfo public class ScreenshotResponse : UnityCliLoopToolResponse { public List Screenshots { get; set; } = new List(); + public bool TimedOut { get; set; } + public string Message { get; set; } = ""; public int ScreenshotCount => Screenshots.Count; } diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotTool.cs b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotTool.cs index ecc369d848..eaec6371db 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotTool.cs +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotTool.cs @@ -17,7 +17,8 @@ public class ScreenshotTool : UnityCliLoopTool ExecuteAsync(ScreenshotSchema parameters, CancellationToken ct) { ScreenshotUseCase useCase = new(); - UnityCliLoopScreenshotResult result = await useCase.CaptureAsync(ToRequest(parameters), ct); + UnityCliLoopScreenshotResult result = await useCase.CaptureAsync(ToRequest(parameters), ct) + .ConfigureAwait(false); return ToResponse(result); } @@ -40,7 +41,7 @@ private static UnityCliLoopScreenshotRequest ToRequest(ScreenshotSchema paramete }; } - private static ScreenshotResponse ToResponse(UnityCliLoopScreenshotResult result) + internal static ScreenshotResponse ToResponse(UnityCliLoopScreenshotResult result) { if (result == null) { @@ -50,6 +51,8 @@ private static ScreenshotResponse ToResponse(UnityCliLoopScreenshotResult result return new ScreenshotResponse { Screenshots = ToScreenshotInfos(result.Screenshots), + TimedOut = result.TimedOut, + Message = result.Message, }; } diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs index aa393a0e4a..648c6b3272 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs @@ -16,6 +16,7 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools public class ScreenshotUseCase : IUnityCliLoopScreenshotService { private const int ANNOTATION_OVERLAY_RENDER_WAIT_FRAMES = 2; + private const int FRAME_WAIT_TIMEOUT_MILLISECONDS = 5000; public async Task CaptureAsync( UnityCliLoopScreenshotRequest request, @@ -38,15 +39,18 @@ public async Task CaptureAsync( if (request.CaptureMode == CaptureMode.rendering) { - return await CaptureRenderingAsync(request, correlationId, ct); + return await CaptureRenderingAsync(request, correlationId, ct).ConfigureAwait(false); } - return await CaptureWindowsAsync(request, correlationId, ct); + return await CaptureWindowsAsync(request, correlationId, ct).ConfigureAwait(false); } private async Task CaptureRenderingAsync( UnityCliLoopScreenshotRequest request, string correlationId, CancellationToken ct) { + SynchronizationContext editorContext = + CapturedEditorSynchronizationContext.RequireCurrent("rendering screenshot use case"); + if (!EditorApplication.isPlaying) { VibeLogger.LogError( @@ -80,6 +84,7 @@ private async Task CaptureRenderingAsync( GameObject annotationOverlay = null; Texture2D texture; int yOffset; + bool timedOut; try { if (request.AnnotateElements) @@ -89,15 +94,32 @@ private async Task CaptureRenderingAsync( request.ResolutionScale); Canvas.ForceUpdateCanvases(); // Chained CLI calls can read the previous GameView RT before overlay rendering catches up. - await EditorFrameWaiter.WaitFramesAsync(ANNOTATION_OVERLAY_RENDER_WAIT_FRAMES, ct); + bool overlayFramesReady = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( + ANNOTATION_OVERLAY_RENDER_WAIT_FRAMES, + FRAME_WAIT_TIMEOUT_MILLISECONDS, + ct).ConfigureAwait(false); + if (!overlayFramesReady) + { + return CreateTimedOutResult("annotation overlay render", correlationId); + } + + await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); } - (texture, yOffset) = await EditorWindowCaptureUtility.CaptureGameRenderingAsync( - request.ResolutionScale, ct); + (texture, yOffset, timedOut) = await EditorWindowCaptureUtility.CaptureGameRenderingAsync( + request.ResolutionScale, + FRAME_WAIT_TIMEOUT_MILLISECONDS, + ct).ConfigureAwait(false); + if (timedOut) + { + return CreateTimedOutResult("GameView rendering capture", correlationId); + } + + await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); } finally { - UIElementAnnotator.DestroyAnnotationOverlay(annotationOverlay); + DestroyAnnotationOverlay(annotationOverlay, editorContext); } UIElementAnnotator.ConvertToSimCoordinates(annotatedElements, (int)Handles.GetMainGameViewSize().y); @@ -125,7 +147,8 @@ private async Task CaptureRenderingAsync( SaveTextureAsPng(texture, savedPath); FileInfo savedFileInfo = new(savedPath); - UnityCliLoopScreenshotInfo info = new() { + UnityCliLoopScreenshotInfo info = new() + { ImagePath = savedPath, FileSizeBytes = savedFileInfo.Length, Width = width, @@ -167,6 +190,8 @@ private async Task CaptureRenderingAsync( private async Task CaptureWindowsAsync( UnityCliLoopScreenshotRequest request, string correlationId, CancellationToken ct) { + SynchronizationContext editorContext = + CapturedEditorSynchronizationContext.RequireCurrent("window screenshot use case"); EditorWindow[] windows = EditorWindowCaptureUtility.FindWindowsByName(request.WindowName, request.MatchMode); if (windows.Length == 0) { @@ -186,7 +211,17 @@ private async Task CaptureWindowsAsync( for (int i = 0; i < windows.Length; i++) { EditorWindow window = windows[i]; - Texture2D texture = await EditorWindowCaptureUtility.CaptureWindowAsync(window, request.ResolutionScale, ct); + (Texture2D texture, bool timedOut) = await EditorWindowCaptureUtility.CaptureWindowAsync( + window, + request.ResolutionScale, + FRAME_WAIT_TIMEOUT_MILLISECONDS, + ct).ConfigureAwait(false); + if (timedOut) + { + return CreateTimedOutResult("EditorWindow capture", correlationId); + } + + await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); if (texture == null) { VibeLogger.LogWarning( @@ -243,6 +278,43 @@ private async Task CaptureWindowsAsync( return new UnityCliLoopScreenshotResult { Screenshots = screenshots }; } + private static UnityCliLoopScreenshotResult CreateTimedOutResult(string waitName, string correlationId) + { + string message = + $"Timed out after {FRAME_WAIT_TIMEOUT_MILLISECONDS}ms while waiting for {waitName} frames."; + VibeLogger.LogWarning( + "screenshot_timeout", + message, + new { WaitName = waitName, TimeoutMilliseconds = FRAME_WAIT_TIMEOUT_MILLISECONDS }, + correlationId: correlationId + ); + + return new UnityCliLoopScreenshotResult + { + TimedOut = true, + Message = message, + }; + } + + private static void DestroyAnnotationOverlay( + GameObject annotationOverlay, + SynchronizationContext editorContext) + { + if (annotationOverlay == null) + { + return; + } + + if (SynchronizationContext.Current == editorContext) + { + UIElementAnnotator.DestroyAnnotationOverlay(annotationOverlay); + return; + } + + // Why: timeout results may complete from the timer thread while Unity's main thread is stalled. + editorContext.Post(_ => UIElementAnnotator.DestroyAnnotationOverlay(annotationOverlay), null); + } + private void ValidateParameters(UnityCliLoopScreenshotRequest request) { if (request.CaptureMode != CaptureMode.rendering && diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCLILoop.FirstPartyTools.Screenshot.Editor.asmdef b/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCLILoop.FirstPartyTools.Screenshot.Editor.asmdef index 701dcadbec..55c4576d25 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCLILoop.FirstPartyTools.Screenshot.Editor.asmdef +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCLILoop.FirstPartyTools.Screenshot.Editor.asmdef @@ -2,6 +2,7 @@ "name": "UnityCLILoop.FirstPartyTools.Screenshot.Editor", "rootNamespace": "io.github.hatayama.UnityCliLoop.FirstPartyTools", "references": [ + "GUID:214998e563c124e8a88199b2dd1f522d", "GUID:5079a8d3a72924a81aa1cbc25f65ed1b", "GUID:a2d87883023de4cf59b7d1962d5dd8aa", "GUID:fc3fd32eddbee40e39c2d76dc184957b" diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCliLoopScreenshotTypes.cs b/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCliLoopScreenshotTypes.cs index ab2d503612..d8562024b1 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCliLoopScreenshotTypes.cs +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCliLoopScreenshotTypes.cs @@ -69,5 +69,7 @@ public sealed class UnityCliLoopScreenshotInfo public sealed class UnityCliLoopScreenshotResult { public List Screenshots { get; set; } = new List(); + public bool TimedOut { get; set; } + public string Message { get; set; } = ""; } } diff --git a/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs b/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs index deab286801..b89895c7c8 100644 --- a/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs +++ b/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs @@ -71,6 +71,40 @@ public Task WaitFramesAsync(int frameCount, CancellationToken ct) return completionSource.Task; } + public async Task WaitFramesOrTimeoutAsync( + int frameCount, + int timeoutMilliseconds, + CancellationToken ct) + { + Debug.Assert(frameCount >= 0, "frameCount must be non-negative"); + Debug.Assert(timeoutMilliseconds > 0, "timeoutMilliseconds must be positive"); + ct.ThrowIfCancellationRequested(); + + if (frameCount <= 0) + { + return true; + } + + using CancellationTokenSource frameCancellationSource = + CancellationTokenSource.CreateLinkedTokenSource(ct); + Task frameTask = WaitFramesAsync(frameCount, frameCancellationSource.Token); + using CancellationTokenSource timeoutCancellationSource = + CancellationTokenSource.CreateLinkedTokenSource(ct); + Task timeoutTask = TimerDelay.Wait(timeoutMilliseconds, timeoutCancellationSource.Token); + + Task completedTask = await Task.WhenAny(frameTask, timeoutTask).ConfigureAwait(false); + if (completedTask == timeoutTask) + { + await timeoutTask.ConfigureAwait(false); + frameCancellationSource.Cancel(); + return false; + } + + timeoutCancellationSource.Cancel(); + await frameTask.ConfigureAwait(false); + return true; + } + public void ClearAllForTests() { List requestsToCancel; @@ -239,6 +273,14 @@ public static Task WaitFramesAsync(int frameCount, CancellationToken ct = defaul return ServiceValue.WaitFramesAsync(frameCount, ct); } + public static Task WaitFramesOrTimeoutAsync( + int frameCount, + int timeoutMilliseconds, + CancellationToken ct = default) + { + return ServiceValue.WaitFramesOrTimeoutAsync(frameCount, timeoutMilliseconds, ct); + } + public static void ClearAllForTests() { ServiceValue.ClearAllForTests(); diff --git a/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs b/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs index 612753b9fd..c3f9e0627b 100644 --- a/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs +++ b/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs @@ -61,7 +61,7 @@ public async Task ExecuteAsync(JToken paramsToken, Can TSchema parameters = ConvertToSchema(paramsToken); // Execute with type-safe parameters - TResponse response = await ExecuteAsync(parameters, ct); + TResponse response = await ExecuteAsync(parameters, ct).ConfigureAwait(false); // Return as UnityCliLoopToolResponse for IUnityCliLoopTool interface compatibility return response; From bbc65eb9667df91b289a33d8a35859f3efe50ed8 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 14 Jun 2026 07:04:58 +0900 Subject: [PATCH 2/5] Remove raw editor frame waits from public use Expose only timeout-guarded editor frame waits so CLI tools cannot wait forever when Unity frames stop advancing. Move remaining production callers to WaitFramesOrTimeoutAsync and switch back to the Unity main thread before touching Unity APIs where needed. --- Assets/Editor/EditorDelayManualTests.cs | 26 +++++--- .../InputSystem/InputSystemUpdateHelper.cs | 26 +++----- .../DynamicCodeDomainReloadWaitSignal.cs | 12 +++- ...artyTools.ExecuteDynamicCode.Editor.asmdef | 1 + .../Screenshot/ScreenshotUseCase.cs | 11 ++-- .../SimulateKeyboardUseCase.cs | 5 +- .../SimulateMouseUi/SimulateMouseUiUseCase.cs | 30 ++++++--- ...stPartyTools.SimulateMouseUi.Editor.asmdef | 1 + .../Editor/ToolContracts/EditorFrameWaiter.cs | 13 ++-- .../src/Editor/ToolContracts/TimerDelay.cs | 63 +++++++++++++++++-- .../ToolContracts/UnityCliLoopConstants.cs | 1 + 11 files changed, 135 insertions(+), 54 deletions(-) diff --git a/Assets/Editor/EditorDelayManualTests.cs b/Assets/Editor/EditorDelayManualTests.cs index cb296e3aa1..a60d52aa43 100644 --- a/Assets/Editor/EditorDelayManualTests.cs +++ b/Assets/Editor/EditorDelayManualTests.cs @@ -16,6 +16,14 @@ public static class EditorFrameWaiterManualTests { private static int testFrameStart; private static int testCounter = 0; + + private static Task WaitFramesForManualTestAsync(int frameCount, CancellationToken ct = default) + { + return EditorFrameWaiter.WaitFramesOrTimeoutAsync( + frameCount, + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, + ct); + } [MenuItem("UnityCliLoop/Debug/EditorFrameWaiter Tests/Basic Frame Wait Tests")] public static void TestBasicDelays() @@ -37,7 +45,7 @@ private static async Task TestZeroFrameDelayAsync() int currentFrame = Time.frameCount; Debug.Log($"[Test {++testCounter}] Zero Frame Delay - Start (Frame: {currentFrame})"); - await EditorFrameWaiter.WaitFramesAsync(0); + await WaitFramesForManualTestAsync(0); int completionFrame = Time.frameCount; Debug.Log($"[Test {testCounter}] Zero Frame Delay - Complete (Frame: {completionFrame}) - Immediate: {currentFrame == completionFrame}"); @@ -48,7 +56,7 @@ private static async Task TestSingleFrameDelayAsync() int currentFrame = Time.frameCount; Debug.Log($"[Test {++testCounter}] Single Frame Delay - Start (Frame: {currentFrame})"); - await EditorFrameWaiter.WaitFramesAsync(1); + await WaitFramesForManualTestAsync(1); int completionFrame = Time.frameCount; int framesDiff = completionFrame - currentFrame; @@ -61,7 +69,7 @@ private static async Task TestMultipleFrameDelayAsync() int currentFrame = Time.frameCount; Debug.Log($"[Test {++testCounter}] Multiple Frame Delay ({delayFrames}) - Start (Frame: {currentFrame})"); - await EditorFrameWaiter.WaitFramesAsync(delayFrames); + await WaitFramesForManualTestAsync(delayFrames); int completionFrame = Time.frameCount; int framesDiff = completionFrame - currentFrame; @@ -92,7 +100,7 @@ private static async Task ConcurrentTaskAsync(string taskName, int frames) int startFrame = Time.frameCount; Debug.Log($"Task {taskName}: Start (Frame: {startFrame}, Delay: {frames} frames)"); - await EditorFrameWaiter.WaitFramesAsync(frames); + await WaitFramesForManualTestAsync(frames); int endFrame = Time.frameCount; int elapsed = endFrame - testFrameStart; @@ -132,7 +140,7 @@ public static void TestStressLoad() private static async Task StressTaskAsync(int taskId, Action onComplete) { - await EditorFrameWaiter.WaitFramesAsync(2); // All tasks execute after 2 frames + await WaitFramesForManualTestAsync(2); // All tasks execute after 2 frames onComplete?.Invoke(); } @@ -160,7 +168,7 @@ private static async Task TestCancellableTaskAsync(CancellationToken cancellatio try { Debug.Log("Cancellable Task: Start (will be cancelled)"); - await EditorFrameWaiter.WaitFramesAsync(10, cancellationToken); + await WaitFramesForManualTestAsync(10, cancellationToken); Debug.Log("Cancellable Task: Complete (should not reach here)"); } catch (OperationCanceledException) @@ -191,13 +199,13 @@ private static async Task SimulateServerRestorationAsync() Debug.Log("Simulation: Starting server restoration sequence..."); // Test with the same pattern as UnityCliLoopServerController - await EditorFrameWaiter.WaitFramesAsync(1); + await WaitFramesForManualTestAsync(1); Debug.Log("Simulation: Phase 1 - Port release wait completed"); - await EditorFrameWaiter.WaitFramesAsync(1); + await WaitFramesForManualTestAsync(1); Debug.Log("Simulation: Phase 2 - Server startup completed"); - await EditorFrameWaiter.WaitFramesAsync(1); + await WaitFramesForManualTestAsync(1); Debug.Log("Simulation: Phase 3 - Notification sent"); Debug.Log("Simulation: Server restoration sequence completed!"); diff --git a/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemUpdateHelper.cs b/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemUpdateHelper.cs index 3d41933306..9a7db23b90 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemUpdateHelper.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemUpdateHelper.cs @@ -34,8 +34,8 @@ internal static class InputSystemUpdateHelper { private const int StandardPressObservationFrames = 2; private const int ManualPressObservationFrames = 3; - private const int DefaultApplyTimeoutMilliseconds = 5000; - private const int DefaultFrameObservationTimeoutMilliseconds = 5000; + private const int DefaultApplyTimeoutMilliseconds = UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS; + private const int DefaultFrameObservationTimeoutMilliseconds = UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS; private const int PressDurationTimeoutGraceMilliseconds = 5000; private const int ApplyWaitStateWaiting = 0; private const int ApplyWaitStateApplying = 1; @@ -412,21 +412,13 @@ private static async Task WaitOneRuntimeFrameOrTimeo return InputSimulationWaitOutcome.TimedOut; } - using CancellationTokenSource frameCts = CancellationTokenSource.CreateLinkedTokenSource(ct); - Task frameTask = EditorFrameWaiter.WaitFramesAsync(1, frameCts.Token); - using CancellationTokenSource timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(ct); - Task timeoutTask = TimerDelay.Wait(remainingMilliseconds, timeoutCts.Token); - Task completedTask = await Task.WhenAny(frameTask, timeoutTask).ConfigureAwait(false); - if (completedTask == timeoutTask) - { - await timeoutTask.ConfigureAwait(false); - frameCts.Cancel(); - return InputSimulationWaitOutcome.TimedOut; - } - - timeoutCts.Cancel(); - await frameTask.ConfigureAwait(false); - return InputSimulationWaitOutcome.Completed; + bool frameObserved = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( + 1, + remainingMilliseconds, + ct).ConfigureAwait(false); + return frameObserved + ? InputSimulationWaitOutcome.Completed + : InputSimulationWaitOutcome.TimedOut; } private static int GetPressLifetimeTimeoutMilliseconds(float duration) diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs index f331d62664..212de9e56b 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs @@ -5,6 +5,7 @@ using UnityEditor; using UnityEditor.Compilation; +using io.github.hatayama.UnityCliLoop.Application; using io.github.hatayama.UnityCliLoop.ToolContracts; namespace io.github.hatayama.UnityCliLoop.FirstPartyTools @@ -53,7 +54,16 @@ public async Task ShouldWaitAsync(CancellationToken ct) for (int frame = 0; frame < SIGNAL_SETTLE_FRAMES; frame++) { - await EditorFrameWaiter.WaitFramesAsync(1, ct); + bool frameReady = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( + 1, + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, + ct).ConfigureAwait(false); + if (!frameReady) + { + return false; + } + + await MainThreadSwitcher.SwitchToMainThread(ct); if (ShouldRequestWait(_parameters, EditorApplication.isCompiling, _reloadSignalObserved)) { return true; diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/UnityCLILoop.FirstPartyTools.ExecuteDynamicCode.Editor.asmdef b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/UnityCLILoop.FirstPartyTools.ExecuteDynamicCode.Editor.asmdef index b54a177112..8005d027de 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/UnityCLILoop.FirstPartyTools.ExecuteDynamicCode.Editor.asmdef +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/UnityCLILoop.FirstPartyTools.ExecuteDynamicCode.Editor.asmdef @@ -2,6 +2,7 @@ "name": "UnityCLILoop.FirstPartyTools.ExecuteDynamicCode.Editor", "rootNamespace": "io.github.hatayama.UnityCliLoop.FirstPartyTools", "references": [ + "GUID:214998e563c124e8a88199b2dd1f522d", "GUID:5c4588558a3624eacbce0f50007cf1eb", "GUID:fc3fd32eddbee40e39c2d76dc184957b", "GUID:616b0fafa8621452d8d1ae18a41bab94" diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs index 648c6b3272..4568653b92 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs @@ -16,7 +16,6 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools public class ScreenshotUseCase : IUnityCliLoopScreenshotService { private const int ANNOTATION_OVERLAY_RENDER_WAIT_FRAMES = 2; - private const int FRAME_WAIT_TIMEOUT_MILLISECONDS = 5000; public async Task CaptureAsync( UnityCliLoopScreenshotRequest request, @@ -96,7 +95,7 @@ private async Task CaptureRenderingAsync( // Chained CLI calls can read the previous GameView RT before overlay rendering catches up. bool overlayFramesReady = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( ANNOTATION_OVERLAY_RENDER_WAIT_FRAMES, - FRAME_WAIT_TIMEOUT_MILLISECONDS, + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, ct).ConfigureAwait(false); if (!overlayFramesReady) { @@ -108,7 +107,7 @@ private async Task CaptureRenderingAsync( (texture, yOffset, timedOut) = await EditorWindowCaptureUtility.CaptureGameRenderingAsync( request.ResolutionScale, - FRAME_WAIT_TIMEOUT_MILLISECONDS, + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, ct).ConfigureAwait(false); if (timedOut) { @@ -214,7 +213,7 @@ private async Task CaptureWindowsAsync( (Texture2D texture, bool timedOut) = await EditorWindowCaptureUtility.CaptureWindowAsync( window, request.ResolutionScale, - FRAME_WAIT_TIMEOUT_MILLISECONDS, + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, ct).ConfigureAwait(false); if (timedOut) { @@ -281,11 +280,11 @@ private async Task CaptureWindowsAsync( private static UnityCliLoopScreenshotResult CreateTimedOutResult(string waitName, string correlationId) { string message = - $"Timed out after {FRAME_WAIT_TIMEOUT_MILLISECONDS}ms while waiting for {waitName} frames."; + $"Timed out after {UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS}ms while waiting for {waitName} frames."; VibeLogger.LogWarning( "screenshot_timeout", message, - new { WaitName = waitName, TimeoutMilliseconds = FRAME_WAIT_TIMEOUT_MILLISECONDS }, + new { WaitName = waitName, TimeoutMilliseconds = UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS }, correlationId: correlationId ); diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs index 8bdb86bdb7..078c6107d3 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs @@ -487,7 +487,10 @@ private static async Task FinalizePressOverlay(CancellationToken ct) } SimulateKeyboardOverlayState.ReleasePress(); - await EditorFrameWaiter.WaitFramesAsync(1, CancellationToken.None).ConfigureAwait(false); + await EditorFrameWaiter.WaitFramesOrTimeoutAsync( + 1, + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, + CancellationToken.None).ConfigureAwait(false); } private static async Task RollbackHeldKey(Keyboard keyboard, Key key, string keyName) diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.cs index bbd5e4a159..7454ad5e25 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.cs @@ -8,6 +8,7 @@ using UnityEngine.EventSystems; using UnityEngine.UI; +using io.github.hatayama.UnityCliLoop.Application; using io.github.hatayama.UnityCliLoop.Runtime; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -429,7 +430,7 @@ private async Task ExecuteLongPress( while (elapsed < parameters.Duration) { SimulateMouseUiOverlayState.UpdateLongPressElapsed(elapsed); - await EditorFrameWaiter.WaitFramesAsync(1, ct); + await WaitForEditorFrameOrThrow(ct); elapsed = Time.realtimeSinceStartup - startTime; } SimulateMouseUiOverlayState.UpdateLongPressElapsed(parameters.Duration); @@ -570,7 +571,7 @@ private async Task ExecuteDragOneShot( { await PlayExpandAnimation(ct); await InterpolateDragPosition(pointerData, target, screenEnd, parameters.DragSpeed, ct); - await EditorFrameWaiter.WaitFramesAsync(1, ct); + await WaitForEditorFrameOrThrow(ct); } finally { @@ -656,7 +657,7 @@ private async Task InterpolateDragPosition( if (duration <= 0f) { - await EditorFrameWaiter.WaitFramesAsync(1, ct); + await WaitForEditorFrameOrThrow(ct); Vector2 previousPosition = pointerData.position; pointerData.position = endPos; @@ -672,7 +673,7 @@ private async Task InterpolateDragPosition( do { - await EditorFrameWaiter.WaitFramesAsync(1, ct); + await WaitForEditorFrameOrThrow(ct); float elapsed = Time.realtimeSinceStartup - startTime; t = Mathf.Clamp01(elapsed / duration); @@ -893,7 +894,7 @@ private async Task ExecuteDragEnd( await InterpolateDragPosition( MouseDragState.PointerData!, MouseDragState.Target!, screenEnd, parameters.DragSpeed, ct); - await EditorFrameWaiter.WaitFramesAsync(1, ct); + await WaitForEditorFrameOrThrow(ct); } finally { @@ -962,7 +963,7 @@ private static async Task PlayExpandAnimation(CancellationToken ct) { float t = elapsed / EXPAND_DURATION; overlay.SetCursorScale(Mathf.Lerp(EXPAND_START_SCALE, 1f, t)); - await EditorFrameWaiter.WaitFramesAsync(1, ct); + await WaitForEditorFrameOrThrow(ct); elapsed = Time.realtimeSinceStartup - startTime; } overlay.SetCursorScale(1f); @@ -979,7 +980,7 @@ private static async Task PlayDissipateAnimation(CancellationToken ct) float t = elapsed / DISSIPATE_DURATION; overlay.SetCursorScale(Mathf.Lerp(1f, 0f, t)); overlay.SetAlpha(Mathf.Lerp(1f, 0f, t)); - await EditorFrameWaiter.WaitFramesAsync(1, ct); + await WaitForEditorFrameOrThrow(ct); elapsed = Time.realtimeSinceStartup - startTime; } overlay!.SetCursorScale(0f); @@ -987,6 +988,21 @@ private static async Task PlayDissipateAnimation(CancellationToken ct) SimulateMouseUiOverlayState.Clear(); } + private static async Task WaitForEditorFrameOrThrow(CancellationToken ct) + { + bool frameReady = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( + 1, + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, + ct).ConfigureAwait(false); + if (!frameReady) + { + throw new TimeoutException( + $"Timed out after {UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS}ms while waiting for an editor frame."); + } + + await MainThreadSwitcher.SwitchToMainThread(ct); + } + private static RaycastResult? RaycastUI(Vector2 screenPosition, EventSystem eventSystem) { return UiRaycastHelper.RaycastUI(screenPosition, eventSystem); diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/UnityCLILoop.FirstPartyTools.SimulateMouseUi.Editor.asmdef b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/UnityCLILoop.FirstPartyTools.SimulateMouseUi.Editor.asmdef index 6885487dff..2b2bcd76bf 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/UnityCLILoop.FirstPartyTools.SimulateMouseUi.Editor.asmdef +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/UnityCLILoop.FirstPartyTools.SimulateMouseUi.Editor.asmdef @@ -2,6 +2,7 @@ "name": "UnityCLILoop.FirstPartyTools.SimulateMouseUi.Editor", "rootNamespace": "io.github.hatayama.UnityCliLoop.FirstPartyTools", "references": [ + "GUID:214998e563c124e8a88199b2dd1f522d", "GUID:fc3fd32eddbee40e39c2d76dc184957b", "GUID:f1a38fcd7e85047c5a86331a3e37ddfc", "GUID:a2d87883023de4cf59b7d1962d5dd8aa", diff --git a/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs b/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs index b89895c7c8..119ba157e1 100644 --- a/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs +++ b/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs @@ -10,7 +10,7 @@ namespace io.github.hatayama.UnityCliLoop.ToolContracts /// /// Waits for Unity Editor update frames when code must observe rendered or runtime frame progress. /// - public sealed class EditorFrameWaiterService + internal sealed class EditorFrameWaiterService { private readonly object _lockObject = new object(); private readonly List _requests = new List(); @@ -44,7 +44,7 @@ public void InitializeForEditorStartup() EditorApplication.update += UpdateRequests; } - public Task WaitFramesAsync(int frameCount, CancellationToken ct) + private Task WaitFramesCoreAsync(int frameCount, CancellationToken ct) { Debug.Assert(frameCount >= 0, "frameCount must be non-negative"); ct.ThrowIfCancellationRequested(); @@ -87,7 +87,7 @@ public async Task WaitFramesOrTimeoutAsync( using CancellationTokenSource frameCancellationSource = CancellationTokenSource.CreateLinkedTokenSource(ct); - Task frameTask = WaitFramesAsync(frameCount, frameCancellationSource.Token); + Task frameTask = WaitFramesCoreAsync(frameCount, frameCancellationSource.Token); using CancellationTokenSource timeoutCancellationSource = CancellationTokenSource.CreateLinkedTokenSource(ct); Task timeoutTask = TimerDelay.Wait(timeoutMilliseconds, timeoutCancellationSource.Token); @@ -254,7 +254,7 @@ private void RemoveFromCancellation() } /// - /// Provides Editor update frame waits without representing them as wall-clock timeouts. + /// Provides Editor update frame waits guarded by wall-clock timeouts. /// public static class EditorFrameWaiter { @@ -268,11 +268,6 @@ public static void InitializeForEditorStartup() ServiceValue.InitializeForEditorStartup(); } - public static Task WaitFramesAsync(int frameCount, CancellationToken ct = default) - { - return ServiceValue.WaitFramesAsync(frameCount, ct); - } - public static Task WaitFramesOrTimeoutAsync( int frameCount, int timeoutMilliseconds, diff --git a/Packages/src/Editor/ToolContracts/TimerDelay.cs b/Packages/src/Editor/ToolContracts/TimerDelay.cs index bb16b1d6ab..fdaf9eafd8 100644 --- a/Packages/src/Editor/ToolContracts/TimerDelay.cs +++ b/Packages/src/Editor/ToolContracts/TimerDelay.cs @@ -38,7 +38,7 @@ public static Task Wait(int milliseconds, CancellationToken ct = default) } /// - /// Waits for wall-clock time, then executes an action after the next Editor update. + /// Waits for wall-clock time, then executes an action on Unity's main thread. /// /// Milliseconds to wait /// Action to execute on main thread @@ -61,9 +61,64 @@ public static async Task WaitThenExecuteOnMainThread(int milliseconds, Action ac "WaitThenExecuteOnMainThread must start from Unity's main-thread synchronization context."); } - await Wait(milliseconds, ct); - await EditorFrameWaiter.WaitFramesAsync(1, ct); - action(); + await Wait(milliseconds, ct).ConfigureAwait(false); + await ExecuteOnSynchronizationContextOrTimeoutAsync( + synchronizationContext, + action, + ct).ConfigureAwait(false); + } + + private static async Task ExecuteOnSynchronizationContextOrTimeoutAsync( + SynchronizationContext synchronizationContext, + Action action, + CancellationToken ct) + { + TaskCompletionSource completionSource = + new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + int completionState = 0; + + using CancellationTokenRegistration registration = ct.CanBeCanceled + ? ct.Register(() => + { + if (Interlocked.Exchange(ref completionState, 1) == 0) + { + completionSource.TrySetCanceled(ct); + } + }) + : default; + + synchronizationContext.Post(_ => + { + if (Interlocked.Exchange(ref completionState, 1) != 0) + { + return; + } + + try + { + action(); + completionSource.TrySetResult(true); + } + catch (Exception ex) + { + completionSource.TrySetException(ex); + } + }, null); + + // Why: Unity may be paused or backgrounded after the wall-clock delay, so the posted action must not hang callers forever. + Task timeoutTask = Wait(UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, ct); + Task completedTask = await Task.WhenAny(completionSource.Task, timeoutTask).ConfigureAwait(false); + if (completedTask == timeoutTask) + { + await timeoutTask.ConfigureAwait(false); + if (Interlocked.Exchange(ref completionState, 1) == 0) + { + completionSource.TrySetException(new TimeoutException( + $"Timed out after {UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS}ms while waiting for Unity main-thread action execution.")); + } + } + + await completionSource.Task.ConfigureAwait(false); } } diff --git a/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs b/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs index 13fd12f8c7..d9c6b19623 100644 --- a/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs +++ b/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs @@ -102,6 +102,7 @@ public static UnityEditor.PackageManager.PackageInfo PackageInfo public const int COMPILE_START_TIMEOUT_MS = 5000; public const int COMPILE_START_POLL_INTERVAL_MS = 100; public const int COMPILE_FINISH_MISSED_CALLBACK_GRACE_MS = 2000; + public const int EDITOR_FRAME_WAIT_TIMEOUT_MS = 5000; public const int MAX_SETTINGS_SIZE_BYTES = 1024 * 16; public const string SECURITY_LOG_PREFIX = "[UnityCliLoop Security]"; From 64b9da331652f31b2196c4994341a50c7333a463 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 14 Jun 2026 14:06:25 +0900 Subject: [PATCH 3/5] Harden editor timeout continuations Keep timeout and cleanup paths from recapturing stalled editor continuations or touching Unity APIs off the main thread. - marshal dynamic-code reload signal cleanup, delayed recording cleanup, and mouse UI cleanup back to the editor thread - make debug logging safe when timeout continuations resume off-thread - add focused guards for delayed recording, screenshot cleanup, mouse UI timeout state, and logger environment collection --- .../ExecuteDynamicCodeUseCaseTests.cs | 79 +++ .../Editor/StaticFacadeStateGuardTests.cs | 63 +++ .../DynamicCodeDomainReloadWaitSignal.cs | 44 +- .../ExecuteDynamicCodeTool.cs | 2 +- .../ExecuteDynamicCodeUseCase.cs | 5 +- .../RecordingsApplicationFacade.cs | 44 +- .../RecordInput/RecordInputTool.cs | 3 +- .../RecordInput/RecordInputUseCase.cs | 100 ++-- ....FirstPartyTools.RecordInput.Editor.asmdef | 1 + .../Screenshot/ScreenshotUseCase.cs | 2 +- .../SimulateMouseUi/SimulateMouseUiTool.cs | 2 +- .../SimulateMouseUi/SimulateMouseUiUseCase.cs | 490 +++++++++++++----- .../src/Editor/ToolContracts/VibeLogger.cs | 95 +++- 13 files changed, 760 insertions(+), 170 deletions(-) diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeUseCaseTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeUseCaseTests.cs index d7aaff5be3..7a0c22ef97 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeUseCaseTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeUseCaseTests.cs @@ -59,8 +59,13 @@ public void DynamicCodeDomainReloadWaitSignal_WhenEditorReportsReloadWork_Should schema, editorIsCompiling: true, reloadSignalObserved: false); + bool reloadSignalShouldWait = DynamicCodeDomainReloadWaitSignal.ShouldRequestWait( + schema, + editorIsCompiling: false, + reloadSignalObserved: true); Assert.That(shouldWait, Is.True); + Assert.That(reloadSignalShouldWait, Is.True); } [Test] @@ -91,6 +96,36 @@ public void DynamicCodeDomainReloadWaitSignal_WhenRequestCannotWait_ShouldNotReq Assert.That(noWaitShouldWait, Is.False); } + [Test] + public void DynamicCodeDomainReloadWaitSignal_WhenDisposedOffMainThread_QueuesEditorEventCleanup() + { + // Tests that timeout continuations do not remove UnityEditor event handlers off the editor thread. + QueuedMainThreadDispatcher dispatcher = new(); + MainThreadSwitcher.RegisterService(dispatcher); + DynamicCodeDomainReloadWaitSignal signal = DynamicCodeDomainReloadWaitSignal.Start( + new ExecuteDynamicCodeSchema + { + WaitForDomainReload = true, + CompileOnly = false + }); + + try + { + signal.Dispose(); + + Assert.That(dispatcher.PendingContinuationCount, Is.EqualTo(1)); + + dispatcher.RunQueuedContinuationsAsMainThread(); + + Assert.That(dispatcher.PendingContinuationCount, Is.EqualTo(0)); + } + finally + { + signal.Dispose(); + RestoreEditorMainThreadDispatcher(); + } + } + [Test] public async Task ExecuteAsync_WhenInitialCompilationLooksLikeMissingReturn_ShouldRetryOnce() { @@ -926,6 +961,50 @@ private static void AssertPrewarmCodeMatchesLiteralReturnShape( Assert.That(prewarm.PreparedSource, Is.EqualTo(userReturn.PreparedSource)); } + private static void RestoreEditorMainThreadDispatcher() + { + io.github.hatayama.UnityCliLoop.Infrastructure.EditorMainThreadDispatcher dispatcher = new(); + MainThreadSwitcher.RegisterService(dispatcher); + dispatcher.Initialize(); + } + + private sealed class QueuedMainThreadDispatcher : IMainThreadDispatcher + { + private readonly Queue _continuations = new(); + private bool _isMainThread; + + public bool IsMainThread => _isMainThread; + + public int PendingContinuationCount => _continuations.Count; + + public void Initialize() + { + } + + public void AddContinuation(Action continuation) + { + Assert.That(continuation, Is.Not.Null); + _continuations.Enqueue(continuation); + } + + public void RunQueuedContinuationsAsMainThread() + { + _isMainThread = true; + try + { + while (_continuations.Count > 0) + { + Action continuation = _continuations.Dequeue(); + continuation(); + } + } + finally + { + _isMainThread = false; + } + } + } + /// /// Test support type used by editor and play mode fixtures. /// diff --git a/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs b/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs index a748afae76..e487d9fce9 100644 --- a/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs +++ b/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs @@ -71,6 +71,10 @@ public sealed class StaticFacadeStateGuardTests @"\b(public|internal|private|protected)\s+static\s+class\b", RegexOptions.Compiled); + private static readonly Regex DiscardedMainThreadTimerTaskPattern = new Regex( + @"_\s*=\s*TimerDelay\.WaitThenExecuteOnMainThread", + RegexOptions.Compiled); + private static readonly Regex AllowedStaticIdentifierPattern = new Regex( @"\b(ServiceValue|RepositoryValue|RegistryValue|RegisteredUseCase)\b", RegexOptions.Compiled); @@ -136,11 +140,70 @@ public void MutableStaticFieldLine_WhenIdentifierOnlyContainsAllowedName_IsRepor Assert.That(isViolation, Is.True); } + [Test] + public void RecordingsApplicationFacade_WhenStartingDelayedRecording_DoesNotDiscardTimerTask() + { + // Tests that delayed recording observes timeout faults and keeps its countdown cleanup path reachable. + string source = ReadSourceFile( + "Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs"); + + Assert.That(DiscardedMainThreadTimerTaskPattern.IsMatch(source), Is.False); + Assert.That(source, Does.Contain("QueueCountdownCleanup")); + Assert.That(source, Does.Contain("StartDelayedRecordingAsync(delayMilliseconds, generation, CancellationToken.None)")); + Assert.That(source, Does.Contain("StartDelayedRecordingAsync(int delayMilliseconds, int generation, CancellationToken ct)")); + } + + [Test] + public void ScreenshotUseCase_WhenDestroyingTimedOutAnnotationOverlay_UsesReferenceNullCheck() + { + // Tests that screenshot timeout cleanup does not call UnityEngine.Object null operators off-thread. + string source = ReadSourceFile("Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs"); + + Assert.That(source, Does.Contain("ReferenceEquals(annotationOverlay, null)")); + Assert.That(source, Does.Not.Contain("annotationOverlay == null")); + } + + [Test] + public void SimulateMouseUiUseCase_WhenReturningAfterTimeout_UsesCapturedUnityObjectState() + { + // Tests that timeout result branches use plain captured bools instead of UnityEngine.Object null checks. + string source = ReadSourceFile("Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.cs"); + + Assert.That(source, Does.Contain("bool hitTarget = target != null;")); + Assert.That(source, Does.Contain("bool shouldReleasePointer = rawTarget != null && target != null;")); + Assert.That(source, Does.Not.Contain("CreateClickResult(parameters, inputPos, targetName, target != null)")); + Assert.That(source, Does.Not.Contain("CreateLongPressResult(parameters, inputPos, targetName, target != null)")); + } + + [Test] + public void VibeLogger_WhenCollectingEnvironmentInfo_GatesUnityEditorApiBehindMainThreadCheck() + { + // Tests that debug logging from timeout continuations does not read UnityEditor APIs off-thread. + string source = ReadSourceFile("Packages/src/Editor/ToolContracts/VibeLogger.cs"); + int methodIndex = source.IndexOf("private EnvironmentInfo GetEnvironmentInfo()", System.StringComparison.Ordinal); + int mainThreadCheckIndex = source.IndexOf("if (!IsEditorMainThread)", methodIndex, System.StringComparison.Ordinal); + int editorApplicationIndex = source.IndexOf("EditorApplication.isCompiling", methodIndex, System.StringComparison.Ordinal); + + Assert.That(methodIndex, Is.GreaterThanOrEqualTo(0)); + Assert.That(mainThreadCheckIndex, Is.GreaterThan(methodIndex)); + Assert.That(editorApplicationIndex, Is.GreaterThan(mainThreadCheckIndex)); + Assert.That(source, Does.Contain("DOMAIN_RELOAD_STATE_UNAVAILABLE_OFF_MAIN_THREAD")); + Assert.That(source, Does.Not.Contain("UnityEngine.Application.dataPath")); + Assert.That(source, Does.Not.Contain("Path.Combine(_logDirectory")); + } + private static List FindMutableStaticFieldViolations() { return FindMutableStaticFieldViolations(MigratedFacadePaths); } + private static string ReadSourceFile(string relativePath) + { + string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + string absolutePath = Path.Combine(projectRoot, relativePath); + return File.ReadAllText(absolutePath); + } + private static List FindMutableStaticFieldViolations(string[] relativePaths) { List violations = new(); diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs index 212de9e56b..f193526ff9 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs @@ -21,6 +21,7 @@ internal sealed class DynamicCodeDomainReloadWaitSignal : IDisposable private readonly bool _isObserving; private bool _reloadSignalObserved; + private int _isDisposed; private DynamicCodeDomainReloadWaitSignal(ExecuteDynamicCodeSchema parameters) { @@ -47,7 +48,10 @@ public async Task ShouldWaitAsync(CancellationToken ct) return false; } - if (ShouldRequestWait(_parameters, EditorApplication.isCompiling, _reloadSignalObserved)) + if (ShouldRequestWait( + _parameters, + EditorApplication.isCompiling, + Volatile.Read(ref _reloadSignalObserved))) { return true; } @@ -60,11 +64,18 @@ public async Task ShouldWaitAsync(CancellationToken ct) ct).ConfigureAwait(false); if (!frameReady) { - return false; + // Why: a reload can stop editor frames before the follow-up EditorApplication check can run. + return ShouldRequestWait( + _parameters, + editorIsCompiling: false, + reloadSignalObserved: Volatile.Read(ref _reloadSignalObserved)); } await MainThreadSwitcher.SwitchToMainThread(ct); - if (ShouldRequestWait(_parameters, EditorApplication.isCompiling, _reloadSignalObserved)) + if (ShouldRequestWait( + _parameters, + EditorApplication.isCompiling, + Volatile.Read(ref _reloadSignalObserved))) { return true; } @@ -93,6 +104,29 @@ public void Dispose() return; } + if (Interlocked.Exchange(ref _isDisposed, 1) != 0) + { + return; + } + + if (MainThreadSwitcher.IsMainThread) + { + UnsubscribeFromEditorEvents(); + return; + } + + // Why: timeout continuations can resume off-thread, but UnityEditor event removal belongs on the editor thread. + UnsubscribeFromEditorEventsOnMainThreadAsync(CancellationToken.None).Forget(); + } + + private async Task UnsubscribeFromEditorEventsOnMainThreadAsync(CancellationToken ct) + { + await MainThreadSwitcher.SwitchToMainThread(ct); + UnsubscribeFromEditorEvents(); + } + + private void UnsubscribeFromEditorEvents() + { CompilationPipeline.compilationStarted -= OnCompilationStarted; AssemblyReloadEvents.beforeAssemblyReload -= OnBeforeAssemblyReload; } @@ -106,12 +140,12 @@ private static bool ShouldObserve(ExecuteDynamicCodeSchema parameters) private void OnCompilationStarted(object context) { - _reloadSignalObserved = true; + Volatile.Write(ref _reloadSignalObserved, true); } private void OnBeforeAssemblyReload() { - _reloadSignalObserved = true; + Volatile.Write(ref _reloadSignalObserved, true); } } } diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeTool.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeTool.cs index e2247b70fb..9a30468b94 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeTool.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeTool.cs @@ -19,7 +19,7 @@ protected override async Task ExecuteAsync( CancellationToken ct) { IExecuteDynamicCodeUseCase useCase = DynamicCodeServices.GetExecuteDynamicCodeUseCase(); - return await useCase.ExecuteAsync(parameters, ct); + return await useCase.ExecuteAsync(parameters, ct).ConfigureAwait(false); } } } diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs index c6bed3adac..3df0f1507c 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs @@ -80,7 +80,10 @@ public async Task ExecuteAsync( originalCode); response.SecurityLevel = editorLevel.ToString(); response.EmitTimingsInJsonResponse = parameters.IncludeTimings; - response.DomainReloadWaitRequired = await domainReloadWaitSignal.ShouldWaitAsync(cancellationToken); + // Why: domain-reload timeouts can complete while Unity's synchronization context is stalled. + bool domainReloadWaitRequired = + await domainReloadWaitSignal.ShouldWaitAsync(cancellationToken).ConfigureAwait(false); + response.DomainReloadWaitRequired = domainReloadWaitRequired; return response; } catch (OperationCanceledException) diff --git a/Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs b/Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs index fa6af887a3..f37e86e663 100644 --- a/Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs +++ b/Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs @@ -2,9 +2,12 @@ #nullable enable using System; using System.IO; +using System.Threading; +using System.Threading.Tasks; using UnityEditor; using UnityEngine; +using io.github.hatayama.UnityCliLoop.Application; using io.github.hatayama.UnityCliLoop.Runtime; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -249,7 +252,46 @@ private void StartRecording(int requestedDelaySeconds) int generation = ++_countdownGeneration; RecordInputOverlayState.StartCountdown(delaySeconds); int delayMilliseconds = delaySeconds * 1000; - _ = TimerDelay.WaitThenExecuteOnMainThread(delayMilliseconds, () => StartRecordingAfterCountdown(generation)); + StartDelayedRecordingAsync(delayMilliseconds, generation, CancellationToken.None).Forget(); + } + + private async Task StartDelayedRecordingAsync(int delayMilliseconds, int generation, CancellationToken ct) + { + bool waitCompleted = false; + + try + { + await TimerDelay.WaitThenExecuteOnMainThread( + delayMilliseconds, + () => StartRecordingAfterCountdown(generation), + ct).ConfigureAwait(false); + waitCompleted = true; + } + finally + { + if (!waitCompleted) + { + QueueCountdownCleanup(generation); + } + } + } + + private void QueueCountdownCleanup(int generation) + { + CleanupCountdownOnMainThreadAsync(generation, CancellationToken.None).Forget(); + } + + private async Task CleanupCountdownOnMainThreadAsync(int generation, CancellationToken ct) + { + await MainThreadSwitcher.SwitchToMainThread(ct); + + // Why: delayed recording timeouts can complete off-thread after the overlay entered Countdown. + if (!InputRecorder.IsRecording + && generation == _countdownGeneration + && RecordInputOverlayState.Phase == RecordInputOverlayPhase.Countdown) + { + RecordInputOverlayState.Clear(); + } } private void StartRecordingAfterCountdown(int generation) diff --git a/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputTool.cs b/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputTool.cs index 159ef81dc5..0f40e2d742 100644 --- a/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputTool.cs +++ b/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputTool.cs @@ -16,7 +16,8 @@ public class RecordInputTool : UnityCliLoopTool ExecuteAsync(RecordInputSchema parameters, CancellationToken ct) { RecordInputUseCase useCase = new(); - UnityCliLoopRecordInputResult result = await useCase.RecordInputAsync(ToRequest(parameters), ct); + UnityCliLoopRecordInputResult result = + await useCase.RecordInputAsync(ToRequest(parameters), ct).ConfigureAwait(false); return ToResponse(result); } diff --git a/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs b/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs index d914ef5705..f22f08adc3 100644 --- a/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs @@ -12,6 +12,7 @@ using Newtonsoft.Json; using Newtonsoft.Json.Serialization; +using io.github.hatayama.UnityCliLoop.Application; using io.github.hatayama.UnityCliLoop.Runtime; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -56,7 +57,9 @@ public async Task RecordInputAsync( switch (request.Action) { case RecordInputAction.Start: - response = await ExecuteStartAsync(request, ct); + // Why: delayed-start timeouts must propagate without recapturing Unity's stalled synchronization context. + response = await ExecuteStartAsync(request, ct).ConfigureAwait(false); + await MainThreadSwitcher.SwitchToMainThread(ct); break; case RecordInputAction.Stop: @@ -79,7 +82,9 @@ public async Task RecordInputAsync( } #if ULOOP_HAS_INPUT_SYSTEM - private static async Task ExecuteStartAsync(UnityCliLoopRecordInputRequest request, CancellationToken ct) + private static async Task ExecuteStartAsync( + UnityCliLoopRecordInputRequest request, + CancellationToken ct) { if (!EditorApplication.isPlaying) { @@ -142,33 +147,10 @@ private static async Task ExecuteStartAsync(Unity if (delaySeconds > 0) { - RecordInputOverlayState.StartCountdown(delaySeconds); - - try - { - await TimerDelay.WaitThenExecuteOnMainThread(delaySeconds * 1000, () => - { - if (!EditorApplication.isPlaying || RecordInputOverlayState.Phase != RecordInputOverlayPhase.Countdown) - { - RecordInputOverlayState.Clear(); - return; - } - - RecordInputOverlayState.StartRecording(); - InputRecorder.StartRecording(keyFilter); - }, ct); - } - finally - { - // Cancelled mid-countdown: clear stale countdown state so next Start isn't blocked - if (!InputRecorder.IsRecording && - RecordInputOverlayState.Phase == RecordInputOverlayPhase.Countdown) - { - RecordInputOverlayState.Clear(); - } - } - - if (!EditorApplication.isPlaying || !InputRecorder.IsRecording) + RecordInputDelayedStartOutcome delayedStartOutcome = + await ExecuteDelayedStartAsync(delaySeconds, keyFilter, ct) + .ConfigureAwait(false); + if (delayedStartOutcome != RecordInputDelayedStartOutcome.Started) { return new UnityCliLoopRecordInputResult { @@ -249,6 +231,66 @@ private static UnityCliLoopRecordInputResult ExecuteStop(UnityCliLoopRecordInput DurationSeconds = data.Metadata.DurationSeconds }; } + + private static async Task ExecuteDelayedStartAsync( + int delaySeconds, + HashSet? keyFilter, + CancellationToken ct) + { + RecordInputDelayedStartOutcome outcome = RecordInputDelayedStartOutcome.Cancelled; + bool waitCompleted = false; + RecordInputOverlayState.StartCountdown(delaySeconds); + + try + { + await TimerDelay.WaitThenExecuteOnMainThread(delaySeconds * 1000, () => + { + if (!EditorApplication.isPlaying || RecordInputOverlayState.Phase != RecordInputOverlayPhase.Countdown) + { + RecordInputOverlayState.Clear(); + outcome = RecordInputDelayedStartOutcome.Cancelled; + return; + } + + RecordInputOverlayState.StartRecording(); + InputRecorder.StartRecording(keyFilter); + outcome = RecordInputDelayedStartOutcome.Started; + }, ct).ConfigureAwait(false); + waitCompleted = true; + return outcome; + } + finally + { + if (!waitCompleted) + { + QueueCountdownCleanup(); + } + } + } + + private enum RecordInputDelayedStartOutcome + { + Cancelled = 0, + Started = 1 + } + + private static void QueueCountdownCleanup() + { + CleanupCountdownOnMainThreadAsync(CancellationToken.None).Forget(); + } + + private static async Task CleanupCountdownOnMainThreadAsync( + CancellationToken ct) + { + await MainThreadSwitcher.SwitchToMainThread(ct); + + // Why: timeout/cancellation can resume off-thread, so stale countdown state is cleared only on Unity's context. + if (!InputRecorder.IsRecording && + RecordInputOverlayState.Phase == RecordInputOverlayPhase.Countdown) + { + RecordInputOverlayState.Clear(); + } + } #endif } } diff --git a/Packages/src/Editor/FirstPartyTools/RecordInput/UnityCLILoop.FirstPartyTools.RecordInput.Editor.asmdef b/Packages/src/Editor/FirstPartyTools/RecordInput/UnityCLILoop.FirstPartyTools.RecordInput.Editor.asmdef index 1f6ab7d94a..7e6d993593 100644 --- a/Packages/src/Editor/FirstPartyTools/RecordInput/UnityCLILoop.FirstPartyTools.RecordInput.Editor.asmdef +++ b/Packages/src/Editor/FirstPartyTools/RecordInput/UnityCLILoop.FirstPartyTools.RecordInput.Editor.asmdef @@ -2,6 +2,7 @@ "name": "UnityCLILoop.FirstPartyTools.RecordInput.Editor", "rootNamespace": "io.github.hatayama.UnityCliLoop.FirstPartyTools", "references": [ + "GUID:214998e563c124e8a88199b2dd1f522d", "GUID:fc3fd32eddbee40e39c2d76dc184957b", "GUID:b21158f16291043699bc11600a45a9ee", "GUID:45d3617c96fa64d3e95383450e67f25a", diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs index 4568653b92..753debe5ec 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs @@ -299,7 +299,7 @@ private static void DestroyAnnotationOverlay( GameObject annotationOverlay, SynchronizationContext editorContext) { - if (annotationOverlay == null) + if (ReferenceEquals(annotationOverlay, null)) { return; } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiTool.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiTool.cs index 461dabce83..006cbe7a11 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiTool.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiTool.cs @@ -17,7 +17,7 @@ protected override async Task ExecuteAsync(SimulateMous { SimulateMouseUiUseCase useCase = new(); UnityCliLoopMouseUiSimulationResult result = - await useCase.SimulateMouseUiAsync(ToRequest(parameters), ct); + await useCase.SimulateMouseUiAsync(ToRequest(parameters), ct).ConfigureAwait(false); return ToResponse(result); } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.cs index 7454ad5e25..f863cade7f 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.cs @@ -22,12 +22,14 @@ public class SimulateMouseUiUseCase : IUnityCliLoopMouseUiSimulationService private const float EXPAND_DURATION = SimulateMouseUiAnimationConstants.EXPAND_DURATION; private const float EXPAND_START_SCALE = SimulateMouseUiAnimationConstants.EXPAND_START_SCALE; private const float DISSIPATE_DURATION = SimulateMouseUiAnimationConstants.DISSIPATE_DURATION; + private SynchronizationContext? _mainThreadContext; public async Task SimulateMouseUiAsync( UnityCliLoopMouseUiSimulationRequest request, CancellationToken ct) { ct.ThrowIfCancellationRequested(); + CaptureMainThreadContext(); MouseUiSimulationCommand parameters = MouseUiSimulationCommand.FromRequest(request); string correlationId = UnityCliLoopConstants.GenerateCorrelationId(); @@ -142,27 +144,27 @@ public async Task SimulateMouseUiAsync( switch (parameters.Action) { case MouseAction.Click: - response = await ExecuteClick(parameters, eventSystem, ct); + response = await ExecuteClick(parameters, eventSystem, ct).ConfigureAwait(false); break; case MouseAction.Drag: - response = await ExecuteDragOneShot(parameters, eventSystem, ct); + response = await ExecuteDragOneShot(parameters, eventSystem, ct).ConfigureAwait(false); break; case MouseAction.DragStart: - response = await ExecuteDragStart(parameters, eventSystem, ct); + response = await ExecuteDragStart(parameters, eventSystem, ct).ConfigureAwait(false); break; case MouseAction.DragMove: - response = await ExecuteDragMove(parameters, ct); + response = await ExecuteDragMove(parameters, ct).ConfigureAwait(false); break; case MouseAction.DragEnd: - response = await ExecuteDragEnd(parameters, ct); + response = await ExecuteDragEnd(parameters, ct).ConfigureAwait(false); break; case MouseAction.LongPress: - response = await ExecuteLongPress(parameters, eventSystem, ct); + response = await ExecuteLongPress(parameters, eventSystem, ct).ConfigureAwait(false); break; default: @@ -289,11 +291,19 @@ private async Task ExecuteClick( }; } + string? targetName = target?.name; + bool hitTarget = target != null; SimulateMouseUiOverlayState.Update( MouseAction.Click, inputPos, null, - target?.name, Handles.GetMainGameViewSize()); + targetName, Handles.GetMainGameViewSize()); - await PlayExpandAnimation(ct); + bool expandCompleted = await PlayExpandAnimation(ct).ConfigureAwait(false); + if (!expandCompleted) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.Click, inputPos, null, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); // Fire click events after expand animation so the user sees where the click lands if (rawTarget != null) @@ -311,21 +321,15 @@ private async Task ExecuteClick( } } - await PlayDissipateAnimation(ct); - - return new UnityCliLoopMouseUiSimulationResult + bool dissipateCompleted = await PlayDissipateAnimation(ct).ConfigureAwait(false); + if (!dissipateCompleted) { - Success = true, - Message = target != null - ? parameters.BypassRaycast - ? $"Bypass-clicked '{target.name}' at ({inputPos.x:F1}, {inputPos.y:F1}) via '{parameters.TargetPath}'" - : $"Clicked '{target.name}' at ({inputPos.x:F1}, {inputPos.y:F1})" - : $"Clicked at ({inputPos.x:F1}, {inputPos.y:F1}) - no UI element hit", - Action = MouseAction.Click.ToString(), - HitGameObjectName = target?.name, - PositionX = inputPos.x, - PositionY = inputPos.y - }; + QueueOverlayClear(); + return CreateClickResult(parameters, inputPos, targetName, hitTarget); + } + await MainThreadSwitcher.SwitchToMainThread(ct); + + return CreateClickResult(parameters, inputPos, targetName, hitTarget); } private async Task ExecuteLongPress( @@ -410,11 +414,20 @@ private async Task ExecuteLongPress( }; } + string? targetName = target?.name; + bool hitTarget = target != null; + bool shouldReleasePointer = rawTarget != null && target != null; SimulateMouseUiOverlayState.Update( MouseAction.LongPress, inputPos, null, - target?.name, Handles.GetMainGameViewSize()); + targetName, Handles.GetMainGameViewSize()); - await PlayExpandAnimation(ct); + bool expandCompleted = await PlayExpandAnimation(ct).ConfigureAwait(false); + if (!expandCompleted) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.LongPress, inputPos, null, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); if (rawTarget != null && target != null) { @@ -430,7 +443,13 @@ private async Task ExecuteLongPress( while (elapsed < parameters.Duration) { SimulateMouseUiOverlayState.UpdateLongPressElapsed(elapsed); - await WaitForEditorFrameOrThrow(ct); + bool frameReady = await WaitForEditorFrameAndSwitchToMainThreadAsync(ct).ConfigureAwait(false); + if (!frameReady) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.LongPress, inputPos, null, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); elapsed = Time.realtimeSinceStartup - startTime; } SimulateMouseUiOverlayState.UpdateLongPressElapsed(parameters.Duration); @@ -438,27 +457,22 @@ private async Task ExecuteLongPress( finally { // Ensure pointerUp fires even if the hold loop is cancelled - if (rawTarget != null && target != null) + if (shouldReleasePointer) { - ExecuteEvents.Execute(target, pointerData, ExecuteEvents.pointerUpHandler); + ExecuteCleanupOnMainThread( + () => ExecuteEvents.Execute(target!, pointerData, ExecuteEvents.pointerUpHandler)); } } - await PlayDissipateAnimation(ct); - - return new UnityCliLoopMouseUiSimulationResult + bool dissipateCompleted = await PlayDissipateAnimation(ct).ConfigureAwait(false); + if (!dissipateCompleted) { - Success = true, - Message = target != null - ? parameters.BypassRaycast - ? $"Bypass-long-pressed '{target.name}' at ({inputPos.x:F1}, {inputPos.y:F1}) via '{parameters.TargetPath}' for {parameters.Duration:F1}s" - : $"Long-pressed '{target.name}' at ({inputPos.x:F1}, {inputPos.y:F1}) for {parameters.Duration:F1}s" - : $"Long-pressed at ({inputPos.x:F1}, {inputPos.y:F1}) for {parameters.Duration:F1}s - no UI element hit", - Action = MouseAction.LongPress.ToString(), - HitGameObjectName = target?.name, - PositionX = inputPos.x, - PositionY = inputPos.y - }; + QueueOverlayClear(); + return CreateLongPressResult(parameters, inputPos, targetName, hitTarget); + } + await MainThreadSwitcher.SwitchToMainThread(ct); + + return CreateLongPressResult(parameters, inputPos, targetName, hitTarget); } private PointerEventData InitiateDrag( @@ -542,8 +556,21 @@ private async Task ExecuteDragOneShot( { SimulateMouseUiOverlayState.Update( MouseAction.Drag, inputStart, null, null, Handles.GetMainGameViewSize()); - await PlayExpandAnimation(ct); - await PlayDissipateAnimation(ct); + bool expandCompleted = await PlayExpandAnimation(ct).ConfigureAwait(false); + if (!expandCompleted) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.Drag, inputStart, inputEnd, null); + } + await MainThreadSwitcher.SwitchToMainThread(ct); + + bool dissipateCompleted = await PlayDissipateAnimation(ct).ConfigureAwait(false); + if (!dissipateCompleted) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.Drag, inputStart, inputEnd, null); + } + await MainThreadSwitcher.SwitchToMainThread(ct); return new UnityCliLoopMouseUiSimulationResult { @@ -564,38 +591,54 @@ private async Task ExecuteDragOneShot( ExecuteEvents.Execute(target, pointerData, ExecuteEvents.beginDragHandler); pointerData.dragging = true; + string targetName = target.name; SimulateMouseUiOverlayState.Update( - MouseAction.Drag, inputStart, inputStart, target.name, Handles.GetMainGameViewSize()); + MouseAction.Drag, inputStart, inputStart, targetName, Handles.GetMainGameViewSize()); try { - await PlayExpandAnimation(ct); - await InterpolateDragPosition(pointerData, target, screenEnd, parameters.DragSpeed, ct); - await WaitForEditorFrameOrThrow(ct); + bool expandCompleted = await PlayExpandAnimation(ct).ConfigureAwait(false); + if (!expandCompleted) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.Drag, inputStart, inputEnd, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); + + bool dragCompleted = await InterpolateDragPosition(pointerData, target, screenEnd, parameters.DragSpeed, ct) + .ConfigureAwait(false); + if (!dragCompleted) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.Drag, inputStart, inputEnd, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); + + bool frameReady = await WaitForEditorFrameAndSwitchToMainThreadAsync(ct).ConfigureAwait(false); + if (!frameReady) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.Drag, inputStart, inputEnd, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); } finally { - FinalizeDrag(pointerData, target, explicitDropTarget); + ExecuteCleanupOnMainThread(() => FinalizeDrag(pointerData, target, explicitDropTarget)); } SimulateMouseUiOverlayState.Update( - MouseAction.Drag, inputEnd, inputStart, target.name, Handles.GetMainGameViewSize()); - - await PlayDissipateAnimation(ct); + MouseAction.Drag, inputEnd, inputStart, targetName, Handles.GetMainGameViewSize()); - return new UnityCliLoopMouseUiSimulationResult + bool completedDissipate = await PlayDissipateAnimation(ct).ConfigureAwait(false); + if (!completedDissipate) { - Success = true, - Message = parameters.BypassRaycast - ? $"Bypass-dragged '{target.name}' from ({inputStart.x:F1}, {inputStart.y:F1}) to ({inputEnd.x:F1}, {inputEnd.y:F1}) via '{parameters.TargetPath}' at {parameters.DragSpeed:F0} px/s" - : $"Dragged '{target.name}' from ({inputStart.x:F1}, {inputStart.y:F1}) to ({inputEnd.x:F1}, {inputEnd.y:F1}) at {parameters.DragSpeed:F0} px/s", - Action = MouseAction.Drag.ToString(), - HitGameObjectName = target.name, - PositionX = inputStart.x, - PositionY = inputStart.y, - EndPositionX = inputEnd.x, - EndPositionY = inputEnd.y - }; + QueueOverlayClear(); + return CreateDragResult(parameters, inputStart, inputEnd, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); + + return CreateDragResult(parameters, inputStart, inputEnd, targetName); } // Lifecycle must match StandaloneInputModule: raycast → pointerUp → drop → endDrag @@ -642,7 +685,7 @@ private void UpdatePointerRaycast(PointerEventData pointerData) pointerData.pointerCurrentRaycast = fallback ?? new RaycastResult(); } - private async Task InterpolateDragPosition( + private async Task InterpolateDragPosition( PointerEventData pointerData, GameObject target, Vector2 endPos, @@ -657,7 +700,12 @@ private async Task InterpolateDragPosition( if (duration <= 0f) { - await WaitForEditorFrameOrThrow(ct); + bool frameReady = await WaitForEditorFrameAndSwitchToMainThreadAsync(ct).ConfigureAwait(false); + if (!frameReady) + { + return false; + } + await MainThreadSwitcher.SwitchToMainThread(ct); Vector2 previousPosition = pointerData.position; pointerData.position = endPos; @@ -665,30 +713,36 @@ private async Task InterpolateDragPosition( ExecuteEvents.Execute(target, pointerData, ExecuteEvents.dragHandler); SimulateMouseUiOverlayState.UpdatePosition(ScreenToInput(endPos)); + return true; } - else - { - float startTime = Time.realtimeSinceStartup; - float t; - do + float startTime = Time.realtimeSinceStartup; + float t; + + do + { + bool frameReady = await WaitForEditorFrameAndSwitchToMainThreadAsync(ct).ConfigureAwait(false); + if (!frameReady) { - await WaitForEditorFrameOrThrow(ct); + return false; + } + await MainThreadSwitcher.SwitchToMainThread(ct); - float elapsed = Time.realtimeSinceStartup - startTime; - t = Mathf.Clamp01(elapsed / duration); - Vector2 previousPosition = pointerData.position; - Vector2 currentPosition = Vector2.Lerp(startPos, endPos, t); + float elapsed = Time.realtimeSinceStartup - startTime; + t = Mathf.Clamp01(elapsed / duration); + Vector2 previousPosition = pointerData.position; + Vector2 currentPosition = Vector2.Lerp(startPos, endPos, t); - pointerData.position = currentPosition; - pointerData.delta = currentPosition - previousPosition; + pointerData.position = currentPosition; + pointerData.delta = currentPosition - previousPosition; - ExecuteEvents.Execute(target, pointerData, ExecuteEvents.dragHandler); + ExecuteEvents.Execute(target, pointerData, ExecuteEvents.dragHandler); - SimulateMouseUiOverlayState.UpdatePosition(ScreenToInput(currentPosition)); - } - while (t < 1.0f); + SimulateMouseUiOverlayState.UpdatePosition(ScreenToInput(currentPosition)); } + while (t < 1.0f); + + return true; } private async Task ExecuteDragStart( @@ -741,8 +795,21 @@ private async Task ExecuteDragStart( { SimulateMouseUiOverlayState.Update( MouseAction.DragStart, inputPos, null, null, Handles.GetMainGameViewSize()); - await PlayExpandAnimation(ct); - await PlayDissipateAnimation(ct); + bool expandCompleted = await PlayExpandAnimation(ct).ConfigureAwait(false); + if (!expandCompleted) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.DragStart, inputPos, null, null); + } + await MainThreadSwitcher.SwitchToMainThread(ct); + + bool dissipateCompleted = await PlayDissipateAnimation(ct).ConfigureAwait(false); + if (!dissipateCompleted) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.DragStart, inputPos, null, null); + } + await MainThreadSwitcher.SwitchToMainThread(ct); return new UnityCliLoopMouseUiSimulationResult { @@ -763,13 +830,20 @@ private async Task ExecuteDragStart( MouseDragState.Target = target; MouseDragState.PointerData = pointerData; + string targetName = target.name; SimulateMouseUiOverlayState.Update( - MouseAction.DragStart, inputPos, inputPos, target.name, Handles.GetMainGameViewSize()); + MouseAction.DragStart, inputPos, inputPos, targetName, Handles.GetMainGameViewSize()); bool animationCompleted = false; try { - await PlayExpandAnimation(ct); + animationCompleted = await PlayExpandAnimation(ct).ConfigureAwait(false); + if (!animationCompleted) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.DragStart, inputPos, null, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); animationCompleted = true; } finally @@ -777,17 +851,20 @@ private async Task ExecuteDragStart( // Cancellation during animation leaves beginDrag dispatched; clean up if (!animationCompleted) { - FinalizeDrag(pointerData, target, null); - MouseDragState.Clear(); + ExecuteCleanupOnMainThread(() => + { + FinalizeDrag(pointerData, target, null); + MouseDragState.Clear(); + }); } } return new UnityCliLoopMouseUiSimulationResult { Success = true, - Message = $"Drag started on '{target.name}' at ({inputPos.x:F1}, {inputPos.y:F1})", + Message = $"Drag started on '{targetName}' at ({inputPos.x:F1}, {inputPos.y:F1})", Action = MouseAction.DragStart.ToString(), - HitGameObjectName = target.name, + HitGameObjectName = targetName, PositionX = inputPos.x, PositionY = inputPos.y }; @@ -819,26 +896,35 @@ private async Task ExecuteDragMove( Vector2 inputEnd = new(parameters.X, parameters.Y); Vector2 screenEnd = InputToScreen(inputEnd); + PointerEventData pointerData = MouseDragState.PointerData!; + GameObject target = MouseDragState.Target!; + string targetName = target.name; SimulateMouseUiOverlayState.Update( MouseAction.DragMove, - ScreenToInput(MouseDragState.PointerData!.position), + ScreenToInput(pointerData.position), SimulateMouseUiOverlayState.DragStartPosition, - MouseDragState.Target!.name, Handles.GetMainGameViewSize()); + targetName, Handles.GetMainGameViewSize()); // Cancellation leaves drag state intact so the user can continue with DragMove/DragEnd - await InterpolateDragPosition( - MouseDragState.PointerData!, MouseDragState.Target!, screenEnd, - parameters.DragSpeed, ct); + bool dragCompleted = await InterpolateDragPosition( + pointerData, target, screenEnd, + parameters.DragSpeed, ct).ConfigureAwait(false); + if (!dragCompleted) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.DragMove, inputEnd, null, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); SimulateMouseUiOverlayState.AddWaypoint(inputEnd); return new UnityCliLoopMouseUiSimulationResult { Success = true, - Message = $"Drag moved on '{MouseDragState.Target!.name}' to ({inputEnd.x:F1}, {inputEnd.y:F1}) at {parameters.DragSpeed:F0} px/s", + Message = $"Drag moved on '{targetName}' to ({inputEnd.x:F1}, {inputEnd.y:F1}) at {parameters.DragSpeed:F0} px/s", Action = MouseAction.DragMove.ToString(), - HitGameObjectName = MouseDragState.Target.name, + HitGameObjectName = targetName, PositionX = inputEnd.x, PositionY = inputEnd.y }; @@ -870,7 +956,9 @@ private async Task ExecuteDragEnd( Vector2 inputEnd = new(parameters.X, parameters.Y); Vector2 screenEnd = InputToScreen(inputEnd); - string targetName = MouseDragState.Target!.name; + PointerEventData pointerData = MouseDragState.PointerData!; + GameObject target = MouseDragState.Target!; + string targetName = target.name; GameObject? explicitDropTarget = null; if (!TryResolveDropTargetPath( @@ -885,37 +973,51 @@ private async Task ExecuteDragEnd( SimulateMouseUiOverlayState.Update( MouseAction.DragEnd, - ScreenToInput(MouseDragState.PointerData!.position), + ScreenToInput(pointerData.position), SimulateMouseUiOverlayState.DragStartPosition, targetName, Handles.GetMainGameViewSize()); try { - await InterpolateDragPosition( - MouseDragState.PointerData!, MouseDragState.Target!, screenEnd, - parameters.DragSpeed, ct); - await WaitForEditorFrameOrThrow(ct); + bool dragCompleted = await InterpolateDragPosition( + pointerData, target, screenEnd, + parameters.DragSpeed, ct).ConfigureAwait(false); + if (!dragCompleted) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.DragEnd, inputEnd, null, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); + + bool frameReady = await WaitForEditorFrameAndSwitchToMainThreadAsync(ct).ConfigureAwait(false); + if (!frameReady) + { + QueueOverlayClear(); + return CreateFrameTimeoutResult(MouseAction.DragEnd, inputEnd, null, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); } finally { - FinalizeDrag(MouseDragState.PointerData!, MouseDragState.Target!, explicitDropTarget); - MouseDragState.Clear(); + ExecuteCleanupOnMainThread(() => + { + FinalizeDrag(pointerData, target, explicitDropTarget); + MouseDragState.Clear(); + }); } SimulateMouseUiOverlayState.Update( MouseAction.DragEnd, inputEnd, null, targetName, Handles.GetMainGameViewSize()); - await PlayDissipateAnimation(ct); - - return new UnityCliLoopMouseUiSimulationResult + bool dissipateCompleted = await PlayDissipateAnimation(ct).ConfigureAwait(false); + if (!dissipateCompleted) { - Success = true, - Message = $"Drag ended on '{targetName}' at ({inputEnd.x:F1}, {inputEnd.y:F1}) at {parameters.DragSpeed:F0} px/s", - Action = MouseAction.DragEnd.ToString(), - HitGameObjectName = targetName, - PositionX = inputEnd.x, - PositionY = inputEnd.y - }; + QueueOverlayClear(); + return CreateDragEndResult(parameters, inputEnd, targetName); + } + await MainThreadSwitcher.SwitchToMainThread(ct); + + return CreateDragEndResult(parameters, inputEnd, targetName); } // User input during a CLI drag can cause Unity's StandaloneInputModule to @@ -950,7 +1052,7 @@ await InterpolateDragPosition( return null; } - private static async Task PlayExpandAnimation(CancellationToken ct) + private static async Task PlayExpandAnimation(CancellationToken ct) { SimulateMouseUiOverlay overlay = OverlayCanvasFactory.VisualizationCanvas.MouseUiOverlay; @@ -963,13 +1065,19 @@ private static async Task PlayExpandAnimation(CancellationToken ct) { float t = elapsed / EXPAND_DURATION; overlay.SetCursorScale(Mathf.Lerp(EXPAND_START_SCALE, 1f, t)); - await WaitForEditorFrameOrThrow(ct); + bool frameReady = await WaitForEditorFrameAndSwitchToMainThreadAsync(ct).ConfigureAwait(false); + if (!frameReady) + { + return false; + } + await MainThreadSwitcher.SwitchToMainThread(ct); elapsed = Time.realtimeSinceStartup - startTime; } overlay.SetCursorScale(1f); + return true; } - private static async Task PlayDissipateAnimation(CancellationToken ct) + private static async Task PlayDissipateAnimation(CancellationToken ct) { SimulateMouseUiOverlay overlay = OverlayCanvasFactory.VisualizationCanvas.MouseUiOverlay; @@ -980,15 +1088,21 @@ private static async Task PlayDissipateAnimation(CancellationToken ct) float t = elapsed / DISSIPATE_DURATION; overlay.SetCursorScale(Mathf.Lerp(1f, 0f, t)); overlay.SetAlpha(Mathf.Lerp(1f, 0f, t)); - await WaitForEditorFrameOrThrow(ct); + bool frameReady = await WaitForEditorFrameAndSwitchToMainThreadAsync(ct).ConfigureAwait(false); + if (!frameReady) + { + return false; + } + await MainThreadSwitcher.SwitchToMainThread(ct); elapsed = Time.realtimeSinceStartup - startTime; } overlay!.SetCursorScale(0f); overlay!.SetAlpha(0f); SimulateMouseUiOverlayState.Clear(); + return true; } - private static async Task WaitForEditorFrameOrThrow(CancellationToken ct) + private static async Task WaitForEditorFrameAndSwitchToMainThreadAsync(CancellationToken ct) { bool frameReady = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( 1, @@ -996,11 +1110,145 @@ private static async Task WaitForEditorFrameOrThrow(CancellationToken ct) ct).ConfigureAwait(false); if (!frameReady) { - throw new TimeoutException( - $"Timed out after {UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS}ms while waiting for an editor frame."); + return false; } await MainThreadSwitcher.SwitchToMainThread(ct); + return true; + } + + private void CaptureMainThreadContext() + { + _mainThreadContext = SynchronizationContext.Current; + Debug.Assert(_mainThreadContext != null, "Main thread synchronization context must be captured."); + } + + private void QueueOverlayClear() + { + ExecuteCleanupOnMainThread(SimulateMouseUiOverlayState.Clear); + } + + private void ExecuteCleanupOnMainThread(Action cleanup) + { + Debug.Assert(cleanup != null, "cleanup must not be null"); + if (cleanup == null) + { + throw new ArgumentNullException(nameof(cleanup)); + } + + if (MainThreadSwitcher.IsMainThread) + { + cleanup(); + return; + } + + SynchronizationContext? context = _mainThreadContext; + Debug.Assert(context != null, "Main thread synchronization context must be captured before cleanup."); + if (context == null) + { + throw new InvalidOperationException("Main thread synchronization context was not captured."); + } + + // Why: timeout continuations can run on timer threads while Unity objects must still be cleaned up on the Editor thread. + context.Post(_ => cleanup(), null); + } + + private static UnityCliLoopMouseUiSimulationResult CreateFrameTimeoutResult( + MouseAction action, + Vector2 position, + Vector2? endPosition, + string? hitGameObjectName) + { + return new UnityCliLoopMouseUiSimulationResult + { + Success = false, + Message = $"Timed out after {UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS}ms while waiting for an editor frame.", + Action = action.ToString(), + HitGameObjectName = hitGameObjectName, + PositionX = position.x, + PositionY = position.y, + EndPositionX = endPosition.HasValue ? endPosition.Value.x : null, + EndPositionY = endPosition.HasValue ? endPosition.Value.y : null + }; + } + + private static UnityCliLoopMouseUiSimulationResult CreateClickResult( + MouseUiSimulationCommand parameters, + Vector2 inputPos, + string? targetName, + bool hitTarget) + { + return new UnityCliLoopMouseUiSimulationResult + { + Success = true, + Message = hitTarget + ? parameters.BypassRaycast + ? $"Bypass-clicked '{targetName}' at ({inputPos.x:F1}, {inputPos.y:F1}) via '{parameters.TargetPath}'" + : $"Clicked '{targetName}' at ({inputPos.x:F1}, {inputPos.y:F1})" + : $"Clicked at ({inputPos.x:F1}, {inputPos.y:F1}) - no UI element hit", + Action = MouseAction.Click.ToString(), + HitGameObjectName = targetName, + PositionX = inputPos.x, + PositionY = inputPos.y + }; + } + + private static UnityCliLoopMouseUiSimulationResult CreateLongPressResult( + MouseUiSimulationCommand parameters, + Vector2 inputPos, + string? targetName, + bool hitTarget) + { + return new UnityCliLoopMouseUiSimulationResult + { + Success = true, + Message = hitTarget + ? parameters.BypassRaycast + ? $"Bypass-long-pressed '{targetName}' at ({inputPos.x:F1}, {inputPos.y:F1}) via '{parameters.TargetPath}' for {parameters.Duration:F1}s" + : $"Long-pressed '{targetName}' at ({inputPos.x:F1}, {inputPos.y:F1}) for {parameters.Duration:F1}s" + : $"Long-pressed at ({inputPos.x:F1}, {inputPos.y:F1}) for {parameters.Duration:F1}s - no UI element hit", + Action = MouseAction.LongPress.ToString(), + HitGameObjectName = targetName, + PositionX = inputPos.x, + PositionY = inputPos.y + }; + } + + private static UnityCliLoopMouseUiSimulationResult CreateDragResult( + MouseUiSimulationCommand parameters, + Vector2 inputStart, + Vector2 inputEnd, + string targetName) + { + return new UnityCliLoopMouseUiSimulationResult + { + Success = true, + Message = parameters.BypassRaycast + ? $"Bypass-dragged '{targetName}' from ({inputStart.x:F1}, {inputStart.y:F1}) to ({inputEnd.x:F1}, {inputEnd.y:F1}) via '{parameters.TargetPath}' at {parameters.DragSpeed:F0} px/s" + : $"Dragged '{targetName}' from ({inputStart.x:F1}, {inputStart.y:F1}) to ({inputEnd.x:F1}, {inputEnd.y:F1}) at {parameters.DragSpeed:F0} px/s", + Action = MouseAction.Drag.ToString(), + HitGameObjectName = targetName, + PositionX = inputStart.x, + PositionY = inputStart.y, + EndPositionX = inputEnd.x, + EndPositionY = inputEnd.y + }; + } + + private static UnityCliLoopMouseUiSimulationResult CreateDragEndResult( + MouseUiSimulationCommand parameters, + Vector2 inputEnd, + string targetName) + { + return new UnityCliLoopMouseUiSimulationResult + { + Success = true, + Message = $"Drag ended on '{targetName}' at ({inputEnd.x:F1}, {inputEnd.y:F1}) at {parameters.DragSpeed:F0} px/s", + Action = MouseAction.DragEnd.ToString(), + HitGameObjectName = targetName, + PositionX = inputEnd.x, + PositionY = inputEnd.y + }; } private static RaycastResult? RaycastUI(Vector2 screenPosition, EventSystem eventSystem) diff --git a/Packages/src/Editor/ToolContracts/VibeLogger.cs b/Packages/src/Editor/ToolContracts/VibeLogger.cs index 59f3f8cdd6..e713103e40 100644 --- a/Packages/src/Editor/ToolContracts/VibeLogger.cs +++ b/Packages/src/Editor/ToolContracts/VibeLogger.cs @@ -3,6 +3,7 @@ using System.Diagnostics; using System.IO; using System.Linq; +using System.Threading; using Newtonsoft.Json; using UnityEditor; using UnityEngine; @@ -20,14 +21,19 @@ namespace io.github.hatayama.UnityCliLoop.ToolContracts /// public sealed class VibeLoggerService { - private readonly string _logDirectory = Path.Combine(UnityEngine.Application.dataPath, "..", UnityCliLoopConstants.OUTPUT_ROOT_DIR, UnityCliLoopConstants.VIBE_LOGS_DIR); private const string LOG_FILE_PREFIX = "unity_vibe"; private const int MAX_FILE_SIZE_MB = 10; private const int MAX_MEMORY_LOGS = 1000; private const int MAX_LOG_FILES = 3; + private const int UNINITIALIZED_MAIN_THREAD_ID = 0; + private const string DOMAIN_RELOAD_STATE_IN_PROGRESS = "InProgress"; + private const string DOMAIN_RELOAD_STATE_IDLE = "Idle"; + private const string DOMAIN_RELOAD_STATE_UNAVAILABLE_OFF_MAIN_THREAD = "UnavailableOffMainThread"; private readonly List _memoryLogs = new List(); private readonly object _lockObject = new object(); + private string _logDirectory = string.Empty; + private int _mainThreadId = UNINITIALIZED_MAIN_THREAD_ID; private bool _hasCleanedUpOnStartup = false; /// @@ -57,6 +63,15 @@ public class EnvironmentInfo { public string domain_reload_state; } + + /// + /// Captures Unity main-thread-only logger state during editor startup. + /// + public void InitializeForEditorLoad() + { + Volatile.Write(ref _mainThreadId, Thread.CurrentThread.ManagedThreadId); + CacheUnityProjectLogDirectory(); + } /// /// Log an info level message with structured context @@ -226,9 +241,11 @@ private void Log(string level, string operation, string message, object context, /// private void SaveLogToFile(VibeLogEntry logEntry) { - if (!Directory.Exists(_logDirectory)) + string logDirectory = GetLogDirectory(); + + if (!Directory.Exists(logDirectory)) { - Directory.CreateDirectory(_logDirectory); + Directory.CreateDirectory(logDirectory); } // Clean up old log files on first access only @@ -242,7 +259,7 @@ private void SaveLogToFile(VibeLogEntry logEntry) } string fileName = $"{LOG_FILE_PREFIX}_{DateTime.UtcNow:yyyyMMdd}.json"; - string filePath = Path.Combine(_logDirectory, fileName); + string filePath = Path.Combine(logDirectory, fileName); // Check file size and rotate if necessary if (File.Exists(filePath)) @@ -251,7 +268,7 @@ private void SaveLogToFile(VibeLogEntry logEntry) if (fileInfo.Length > MAX_FILE_SIZE_MB * 1024 * 1024) { string rotatedFileName = $"{LOG_FILE_PREFIX}_{DateTime.UtcNow:yyyyMMdd_HHmmss}.json"; - string rotatedFilePath = Path.Combine(_logDirectory, rotatedFileName); + string rotatedFilePath = Path.Combine(logDirectory, rotatedFileName); File.Move(filePath, rotatedFilePath); // Clean up old files after rotation @@ -308,13 +325,15 @@ private static bool IsFileSharingViolation(IOException ex) /// private void CleanupOldLogFiles() { + string logDirectory = GetLogDirectory(); + try { - if (!Directory.Exists(_logDirectory)) + if (!Directory.Exists(logDirectory)) return; // Get all vibe log files, sorted by creation time (newest first) - FileInfo[] logFiles = Directory.GetFiles(_logDirectory, $"{LOG_FILE_PREFIX}_*.json") + FileInfo[] logFiles = Directory.GetFiles(logDirectory, $"{LOG_FILE_PREFIX}_*.json") .Select(file => new FileInfo(file)) .OrderByDescending(file => file.CreationTime) .ToArray(); @@ -342,15 +361,67 @@ private void CleanupOldLogFiles() /// /// Get current environment information /// - private static EnvironmentInfo GetEnvironmentInfo() + private EnvironmentInfo GetEnvironmentInfo() { + if (!IsEditorMainThread) + { + return new EnvironmentInfo + { + domain_reload_state = DOMAIN_RELOAD_STATE_UNAVAILABLE_OFF_MAIN_THREAD + }; + } + + // Why: timeout continuations can run off-thread, so UnityEditor state is read only + // after the cached editor main-thread check above confirms this call is safe. bool isDomainReloadInProgress = EditorApplication.isCompiling; return new EnvironmentInfo { - domain_reload_state = isDomainReloadInProgress ? "InProgress" : "Idle" + domain_reload_state = isDomainReloadInProgress ? DOMAIN_RELOAD_STATE_IN_PROGRESS : DOMAIN_RELOAD_STATE_IDLE }; } + + private bool IsEditorMainThread + { + get + { + int mainThreadId = Volatile.Read(ref _mainThreadId); + return mainThreadId != UNINITIALIZED_MAIN_THREAD_ID + && Thread.CurrentThread.ManagedThreadId == mainThreadId; + } + } + + private string GetLogDirectory() + { + string logDirectory = Volatile.Read(ref _logDirectory); + if (!string.IsNullOrEmpty(logDirectory)) + { + return logDirectory; + } + + if (IsEditorMainThread) + { + return CacheUnityProjectLogDirectory(); + } + + return CreateLogDirectoryPath(Directory.GetCurrentDirectory()); + } + + private string CacheUnityProjectLogDirectory() + { + string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + string logDirectory = CreateLogDirectoryPath(projectRoot); + Volatile.Write(ref _logDirectory, logDirectory); + return logDirectory; + } + + private static string CreateLogDirectoryPath(string projectRoot) + { + return Path.Combine( + projectRoot, + UnityCliLoopConstants.OUTPUT_ROOT_DIR, + UnityCliLoopConstants.VIBE_LOGS_DIR); + } } /// @@ -360,6 +431,12 @@ public static class VibeLogger { private static readonly VibeLoggerService ServiceValue = new VibeLoggerService(); + [InitializeOnLoadMethod] + private static void InitializeForEditorLoad() + { + ServiceValue.InitializeForEditorLoad(); + } + [Conditional(UnityCliLoopConstants.ENV_KEY_ULOOP_DEBUG)] public static void LogInfo(string operation, string message, object context = null, string correlationId = null, string humanNote = null, string aiTodo = null, bool includeStackTrace = false) From 99ae2a2dab3788aec0645d800c845ae9fafd07ab Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 14 Jun 2026 14:39:57 +0900 Subject: [PATCH 4/5] Preserve screenshot timeout state Switch dynamic-code reload waiting back to the editor thread before reading EditorApplication state, and keep already captured screenshots in timeout responses so CLI output matches files written before a later timeout. --- .../Editor/StaticFacadeStateGuardTests.cs | 33 +++++++++++++++++++ .../DynamicCodeDomainReloadWaitSignal.cs | 8 ++++- .../Screenshot/ScreenshotUseCase.cs | 18 +++++++--- 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs b/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs index e487d9fce9..3839ff4409 100644 --- a/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs +++ b/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs @@ -163,6 +163,39 @@ public void ScreenshotUseCase_WhenDestroyingTimedOutAnnotationOverlay_UsesRefere Assert.That(source, Does.Not.Contain("annotationOverlay == null")); } + [Test] + public void ScreenshotUseCase_WhenWindowCaptureTimesOut_PreservesPartialScreenshots() + { + // Tests that window capture timeout results keep screenshots already written to disk. + string source = ReadSourceFile("Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs"); + + Assert.That(source, Does.Contain("return CreateTimedOutResult(\"EditorWindow capture\", correlationId, screenshots);")); + Assert.That(source, Does.Contain("Screenshots = screenshots,")); + } + + [Test] + public void DynamicCodeDomainReloadWaitSignal_WhenCheckingCompileState_SwitchesToMainThreadFirst() + { + // Tests that timeout continuations do not read UnityEditor compile state off the editor thread. + string source = ReadSourceFile( + "Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs"); + int methodIndex = source.IndexOf( + "public async Task ShouldWaitAsync(CancellationToken ct)", + System.StringComparison.Ordinal); + int mainThreadSwitchIndex = source.IndexOf( + "await MainThreadSwitcher.SwitchToMainThread(ct);", + methodIndex, + System.StringComparison.Ordinal); + int editorCompilingIndex = source.IndexOf( + "EditorApplication.isCompiling", + methodIndex, + System.StringComparison.Ordinal); + + Assert.That(methodIndex, Is.GreaterThanOrEqualTo(0)); + Assert.That(mainThreadSwitchIndex, Is.GreaterThan(methodIndex)); + Assert.That(editorCompilingIndex, Is.GreaterThan(mainThreadSwitchIndex)); + } + [Test] public void SimulateMouseUiUseCase_WhenReturningAfterTimeout_UsesCapturedUnityObjectState() { diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs index f193526ff9..07c4bb39e6 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs @@ -48,10 +48,16 @@ public async Task ShouldWaitAsync(CancellationToken ct) return false; } + if (Volatile.Read(ref _reloadSignalObserved)) + { + return true; + } + + await MainThreadSwitcher.SwitchToMainThread(ct); if (ShouldRequestWait( _parameters, EditorApplication.isCompiling, - Volatile.Read(ref _reloadSignalObserved))) + reloadSignalObserved: Volatile.Read(ref _reloadSignalObserved))) { return true; } diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs index 753debe5ec..3640dbd67e 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs @@ -99,7 +99,10 @@ private async Task CaptureRenderingAsync( ct).ConfigureAwait(false); if (!overlayFramesReady) { - return CreateTimedOutResult("annotation overlay render", correlationId); + return CreateTimedOutResult( + "annotation overlay render", + correlationId, + new List()); } await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); @@ -111,7 +114,10 @@ private async Task CaptureRenderingAsync( ct).ConfigureAwait(false); if (timedOut) { - return CreateTimedOutResult("GameView rendering capture", correlationId); + return CreateTimedOutResult( + "GameView rendering capture", + correlationId, + new List()); } await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); @@ -217,7 +223,7 @@ private async Task CaptureWindowsAsync( ct).ConfigureAwait(false); if (timedOut) { - return CreateTimedOutResult("EditorWindow capture", correlationId); + return CreateTimedOutResult("EditorWindow capture", correlationId, screenshots); } await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); @@ -277,7 +283,10 @@ private async Task CaptureWindowsAsync( return new UnityCliLoopScreenshotResult { Screenshots = screenshots }; } - private static UnityCliLoopScreenshotResult CreateTimedOutResult(string waitName, string correlationId) + private static UnityCliLoopScreenshotResult CreateTimedOutResult( + string waitName, + string correlationId, + List screenshots) { string message = $"Timed out after {UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS}ms while waiting for {waitName} frames."; @@ -292,6 +301,7 @@ private static UnityCliLoopScreenshotResult CreateTimedOutResult(string waitName { TimedOut = true, Message = message, + Screenshots = screenshots, }; } From cf9d261383f48e0b0d1c25d4eeafbb84f707c3d0 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 14 Jun 2026 14:48:16 +0900 Subject: [PATCH 5/5] Guard delayed editor continuations Add timeout-safe ConfigureAwait usage through the dynamic-code workflow, cancel the losing TimerDelay timeout after posted actions complete, and bind delayed record-input callbacks to their active countdown generation. Manual frame-wait checks now report timeout failures instead of logging completion after a timed-out wait. --- Assets/Editor/EditorDelayManualTests.cs | 12 ++++- .../Editor/StaticFacadeStateGuardTests.cs | 51 +++++++++++++++++++ .../ExecuteDynamicCodeUseCase.cs | 18 ++++--- .../RecordingsApplicationFacade.cs | 11 +++- .../RecordInput/RecordInputUseCase.cs | 25 +++++++-- .../src/Editor/ToolContracts/TimerDelay.cs | 10 +++- 6 files changed, 111 insertions(+), 16 deletions(-) diff --git a/Assets/Editor/EditorDelayManualTests.cs b/Assets/Editor/EditorDelayManualTests.cs index a60d52aa43..2474ee1b4a 100644 --- a/Assets/Editor/EditorDelayManualTests.cs +++ b/Assets/Editor/EditorDelayManualTests.cs @@ -17,12 +17,20 @@ public static class EditorFrameWaiterManualTests private static int testFrameStart; private static int testCounter = 0; - private static Task WaitFramesForManualTestAsync(int frameCount, CancellationToken ct = default) + private static async Task WaitFramesForManualTestAsync(int frameCount, CancellationToken ct = default) { - return EditorFrameWaiter.WaitFramesOrTimeoutAsync( + bool completed = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( frameCount, UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, ct); + if (completed) + { + return; + } + + Debug.LogWarning($"[WaitFramesForManualTestAsync] Timed out after {UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS}ms waiting for {frameCount} frame(s)."); + throw new TimeoutException( + $"Timed out after {UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS}ms waiting for {frameCount} frame(s)."); } [MenuItem("UnityCliLoop/Debug/EditorFrameWaiter Tests/Basic Frame Wait Tests")] diff --git a/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs b/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs index 3839ff4409..617def0f21 100644 --- a/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs +++ b/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs @@ -153,6 +153,42 @@ public void RecordingsApplicationFacade_WhenStartingDelayedRecording_DoesNotDisc Assert.That(source, Does.Contain("StartDelayedRecordingAsync(int delayMilliseconds, int generation, CancellationToken ct)")); } + [Test] + public void RecordInputDelayedStarts_WhenCallbackIsStale_DoNotClearCurrentCountdown() + { + // Tests that stale delayed-recording callbacks cannot clear a newer countdown instance. + string facadeSource = ReadSourceFile( + "Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs"); + string useCaseSource = ReadSourceFile( + "Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs"); + + Assert.That(facadeSource, Does.Contain("if (generation != _countdownGeneration)\n {\n return;\n }")); + Assert.That(useCaseSource, Does.Contain("int generation = Interlocked.Increment(ref _delayedStartGeneration);")); + Assert.That(useCaseSource, Does.Contain("QueueCountdownCleanup(generation);")); + Assert.That(useCaseSource, Does.Contain("if (!IsCurrentDelayedStartGeneration(generation))")); + } + + [Test] + public void TimerDelay_WhenMainThreadActionCompletes_CancelsTimeoutWait() + { + // Tests that successful posted actions do not leave the timeout timer running until expiry. + string source = ReadSourceFile("Packages/src/Editor/ToolContracts/TimerDelay.cs"); + + Assert.That(source, Does.Contain("CancellationTokenSource.CreateLinkedTokenSource(ct)")); + Assert.That(source, Does.Contain("timeoutCancellationSource.Cancel();")); + } + + [Test] + public void EditorFrameWaiterManualTests_WhenWaitTimesOut_ReportFailureBeforeCompletionLogs() + { + // Tests that manual frame-wait checks surface timeout failures instead of silently continuing. + string source = ReadSourceFile("Assets/Editor/EditorDelayManualTests.cs"); + + Assert.That(source, Does.Contain("private static async Task WaitFramesForManualTestAsync")); + Assert.That(source, Does.Contain("Debug.LogWarning($\"[WaitFramesForManualTestAsync] Timed out after")); + Assert.That(source, Does.Contain("throw new TimeoutException(")); + } + [Test] public void ScreenshotUseCase_WhenDestroyingTimedOutAnnotationOverlay_UsesReferenceNullCheck() { @@ -196,6 +232,21 @@ public void DynamicCodeDomainReloadWaitSignal_WhenCheckingCompileState_SwitchesT Assert.That(editorCompilingIndex, Is.GreaterThan(mainThreadSwitchIndex)); } + [Test] + public void ExecuteDynamicCodeUseCase_WhenAwaitingWorkflow_DoesNotCaptureEditorSynchronizationContext() + { + // Tests that timeout-sensitive dynamic-code awaits do not capture Unity's synchronization context. + string source = ReadSourceFile( + "Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs"); + + Assert.That(source, Does.Contain("await WarmForegroundExecutionPathIfNeededAsync(parameters, editorLevel, cancellationToken)\n .ConfigureAwait(false);")); + Assert.That(source, Does.Contain("await ExecuteRequestAsync(request, cancellationToken).ConfigureAwait(false);")); + Assert.That(source, Does.Contain("await RetryMissingReturnIfNeeded(\n executionResult,")); + Assert.That(source, Does.Contain("cancellationToken).ConfigureAwait(false);")); + Assert.That(source, Does.Contain("await _runtime.ExecuteAsync(request, cancellationToken).ConfigureAwait(false);")); + Assert.That(source, Does.Contain("await _runtime.TryExecuteIfIdleAsync(\n request,\n cancellationToken).ConfigureAwait(false);")); + } + [Test] public void SimulateMouseUiUseCase_WhenReturningAfterTimeout_UsesCapturedUnityObjectState() { diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs index 3df0f1507c..0115a091fa 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs @@ -47,8 +47,9 @@ public async Task ExecuteAsync( parameters.CompileOnly, editorLevel, parameters.YieldToForegroundRequests); - await WarmForegroundExecutionPathIfNeededAsync(parameters, editorLevel, cancellationToken); - ExecutionResult executionResult = await ExecuteRequestAsync(request, cancellationToken); + await WarmForegroundExecutionPathIfNeededAsync(parameters, editorLevel, cancellationToken) + .ConfigureAwait(false); + ExecutionResult executionResult = await ExecuteRequestAsync(request, cancellationToken).ConfigureAwait(false); ExecutionResult finalResult = await RetryMissingReturnIfNeeded( executionResult, @@ -57,7 +58,7 @@ public async Task ExecuteAsync( parameters.CompileOnly, editorLevel, parameters.YieldToForegroundRequests, - cancellationToken); + cancellationToken).ConfigureAwait(false); if (ShouldMarkExecutionPathWarm(parameters, finalResult)) { @@ -181,7 +182,8 @@ private async Task RetryMissingReturnIfNeeded( compileOnly, securityLevel, yieldToForegroundRequests); - ExecutionResult retryResult = await ExecuteRequestAsync(retryRequest, cancellationToken); + ExecutionResult retryResult = await ExecuteRequestAsync(retryRequest, cancellationToken) + .ConfigureAwait(false); if (retryResult.Success) { return retryResult; @@ -254,12 +256,12 @@ private async Task ExecuteRequestAsync( { if (!request.YieldToForegroundRequests) { - return await _runtime.ExecuteAsync(request, cancellationToken); + return await _runtime.ExecuteAsync(request, cancellationToken).ConfigureAwait(false); } (bool entered, ExecutionResult result) = await _runtime.TryExecuteIfIdleAsync( request, - cancellationToken); + cancellationToken).ConfigureAwait(false); if (entered) { return result; @@ -292,7 +294,7 @@ private async Task WarmForegroundExecutionPathIfNeededAsync( { completed = await ExecuteForegroundWarmupSequenceAsync( securityLevel, - cancellationToken); + cancellationToken).ConfigureAwait(false); if (completed) { DynamicCodeForegroundWarmupState.MarkCompleted(); @@ -315,7 +317,7 @@ private async Task ExecuteForegroundWarmupSequenceAsync( _runtime, securityLevel, yieldToForegroundRequests: false, - ct); + ct).ConfigureAwait(false); } private static bool ShouldWarmForegroundExecutionPath(ExecuteDynamicCodeSchema parameters) diff --git a/Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs b/Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs index f37e86e663..746ceb5860 100644 --- a/Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs +++ b/Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs @@ -284,10 +284,13 @@ private void QueueCountdownCleanup(int generation) private async Task CleanupCountdownOnMainThreadAsync(int generation, CancellationToken ct) { await MainThreadSwitcher.SwitchToMainThread(ct); + if (generation != _countdownGeneration) + { + return; + } // Why: delayed recording timeouts can complete off-thread after the overlay entered Countdown. if (!InputRecorder.IsRecording - && generation == _countdownGeneration && RecordInputOverlayState.Phase == RecordInputOverlayPhase.Countdown) { RecordInputOverlayState.Clear(); @@ -296,8 +299,12 @@ private async Task CleanupCountdownOnMainThreadAsync(int generation, Cancellatio private void StartRecordingAfterCountdown(int generation) { + if (generation != _countdownGeneration) + { + return; + } + if (!EditorApplication.isPlaying - || generation != _countdownGeneration || RecordInputOverlayState.Phase != RecordInputOverlayPhase.Countdown) { RecordInputOverlayState.Clear(); diff --git a/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs b/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs index f22f08adc3..ca5dfe728f 100644 --- a/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs @@ -82,6 +82,8 @@ public async Task RecordInputAsync( } #if ULOOP_HAS_INPUT_SYSTEM + private static int _delayedStartGeneration; + private static async Task ExecuteStartAsync( UnityCliLoopRecordInputRequest request, CancellationToken ct) @@ -180,6 +182,7 @@ private static UnityCliLoopRecordInputResult ExecuteStop(UnityCliLoopRecordInput { if (RecordInputOverlayState.Phase == RecordInputOverlayPhase.Countdown) { + Interlocked.Increment(ref _delayedStartGeneration); RecordInputOverlayState.Clear(); return new UnityCliLoopRecordInputResult { @@ -239,12 +242,18 @@ private static async Task ExecuteDelayedStartAsy { RecordInputDelayedStartOutcome outcome = RecordInputDelayedStartOutcome.Cancelled; bool waitCompleted = false; + int generation = Interlocked.Increment(ref _delayedStartGeneration); RecordInputOverlayState.StartCountdown(delaySeconds); try { await TimerDelay.WaitThenExecuteOnMainThread(delaySeconds * 1000, () => { + if (!IsCurrentDelayedStartGeneration(generation)) + { + return; + } + if (!EditorApplication.isPlaying || RecordInputOverlayState.Phase != RecordInputOverlayPhase.Countdown) { RecordInputOverlayState.Clear(); @@ -263,7 +272,7 @@ await TimerDelay.WaitThenExecuteOnMainThread(delaySeconds * 1000, () => { if (!waitCompleted) { - QueueCountdownCleanup(); + QueueCountdownCleanup(generation); } } } @@ -274,15 +283,20 @@ private enum RecordInputDelayedStartOutcome Started = 1 } - private static void QueueCountdownCleanup() + private static void QueueCountdownCleanup(int generation) { - CleanupCountdownOnMainThreadAsync(CancellationToken.None).Forget(); + CleanupCountdownOnMainThreadAsync(generation, CancellationToken.None).Forget(); } private static async Task CleanupCountdownOnMainThreadAsync( + int generation, CancellationToken ct) { await MainThreadSwitcher.SwitchToMainThread(ct); + if (!IsCurrentDelayedStartGeneration(generation)) + { + return; + } // Why: timeout/cancellation can resume off-thread, so stale countdown state is cleared only on Unity's context. if (!InputRecorder.IsRecording && @@ -291,6 +305,11 @@ private static async Task CleanupCountdownOnMainThreadAsync( RecordInputOverlayState.Clear(); } } + + private static bool IsCurrentDelayedStartGeneration(int generation) + { + return Volatile.Read(ref _delayedStartGeneration) == generation; + } #endif } } diff --git a/Packages/src/Editor/ToolContracts/TimerDelay.cs b/Packages/src/Editor/ToolContracts/TimerDelay.cs index fdaf9eafd8..1745aae148 100644 --- a/Packages/src/Editor/ToolContracts/TimerDelay.cs +++ b/Packages/src/Editor/ToolContracts/TimerDelay.cs @@ -106,7 +106,11 @@ private static async Task ExecuteOnSynchronizationContextOrTimeoutAsync( }, null); // Why: Unity may be paused or backgrounded after the wall-clock delay, so the posted action must not hang callers forever. - Task timeoutTask = Wait(UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, ct); + using CancellationTokenSource timeoutCancellationSource = + CancellationTokenSource.CreateLinkedTokenSource(ct); + Task timeoutTask = Wait( + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, + timeoutCancellationSource.Token); Task completedTask = await Task.WhenAny(completionSource.Task, timeoutTask).ConfigureAwait(false); if (completedTask == timeoutTask) { @@ -117,6 +121,10 @@ private static async Task ExecuteOnSynchronizationContextOrTimeoutAsync( $"Timed out after {UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS}ms while waiting for Unity main-thread action execution.")); } } + else + { + timeoutCancellationSource.Cancel(); + } await completionSource.Task.ConfigureAwait(false); }