From fdb516c8dcc9e4e1f029d6e74dd7751603b476ee Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 29 Oct 2026 16:11:39 +0900 Subject: [PATCH 1/5] Fail a command waiting for a blocked main thread with MAIN_THREAD_BLOCKED A command sent while a modal dialog or a long operation held the Editor main thread took the command slot and waited silently for the thread to resume, up to the CLI's 5-minute limit, even though status and BUSY already reported the stall within seconds. A watchdog now polls the main-thread stall once a second while a request waits for its first main-thread switch. At 30 seconds it cancels the lease through the same path as uloop cancel and the request fails with main_thread_blocked, carrying the tool name and stall seconds; no tool code has run, so retrying is safe. The first check waits one poll so an idle Editor whose loop was only asleep is not judged. The lease keeps an entry state that the watchdog, uloop cancel, and the tool entry all move with a CAS from Waiting, and the service decides by that state rather than by the token: a claimer can win before it has cancelled the token, and the main thread may resume the switch in that window. A tool that has entered is never stopped by the watchdog. --- ...seFactoryWireShapeCharacterizationTests.cs | 17 + .../Editor/MainThreadEntryWatchdogTests.cs | 175 ++++++++++ .../MainThreadEntryWatchdogTests.cs.meta | 11 + .../Tests/Editor/ToolExecutionSessionTests.cs | 120 +++++++ .../UnityCliLoopToolExecutionServiceTests.cs | 321 ++++++++++++++++++ .../Application/IMainThreadEntryWatchdog.cs | 17 + .../IMainThreadEntryWatchdog.cs.meta | 11 + .../Application/ImprovedErrorHandler.cs | 13 + .../Application/MainThreadEntryWatchdog.cs | 77 +++++ .../MainThreadEntryWatchdog.cs.meta | 11 + .../Application/MainThreadLivenessPort.cs | 12 + .../MainThreadLivenessPort.cs.meta | 11 + .../Application/NoMainThreadEntryWatchdog.cs | 25 ++ .../NoMainThreadEntryWatchdog.cs.meta | 11 + .../UnityCliLoopMainThreadBlockedException.cs | 29 ++ ...yCliLoopMainThreadBlockedException.cs.meta | 11 + .../UnityCliLoopToolExecutionService.cs | 52 ++- .../UnityCliLoopApplicationRegistration.cs | 8 +- .../src/Editor/Domain/ToolExecutionSession.cs | 63 +++- .../Infrastructure/Api/JsonRpcErrorTypes.cs | 1 + .../Api/JsonRpcRequestProcessor.cs | 7 +- .../Api/JsonRpcResponseFactory.cs | 7 + .../Api/MainThreadBlockedErrorData.cs | 22 ++ .../Api/MainThreadBlockedErrorData.cs.meta | 11 + .../Threading/EditorMainThreadLivenessPort.cs | 15 + .../EditorMainThreadLivenessPort.cs.meta | 11 + 26 files changed, 1063 insertions(+), 6 deletions(-) create mode 100644 Assets/Tests/Editor/MainThreadEntryWatchdogTests.cs create mode 100644 Assets/Tests/Editor/MainThreadEntryWatchdogTests.cs.meta create mode 100644 Packages/src/Editor/Application/IMainThreadEntryWatchdog.cs create mode 100644 Packages/src/Editor/Application/IMainThreadEntryWatchdog.cs.meta create mode 100644 Packages/src/Editor/Application/MainThreadEntryWatchdog.cs create mode 100644 Packages/src/Editor/Application/MainThreadEntryWatchdog.cs.meta create mode 100644 Packages/src/Editor/Application/MainThreadLivenessPort.cs create mode 100644 Packages/src/Editor/Application/MainThreadLivenessPort.cs.meta create mode 100644 Packages/src/Editor/Application/NoMainThreadEntryWatchdog.cs create mode 100644 Packages/src/Editor/Application/NoMainThreadEntryWatchdog.cs.meta create mode 100644 Packages/src/Editor/Application/UnityCliLoopMainThreadBlockedException.cs create mode 100644 Packages/src/Editor/Application/UnityCliLoopMainThreadBlockedException.cs.meta create mode 100644 Packages/src/Editor/Infrastructure/Api/MainThreadBlockedErrorData.cs create mode 100644 Packages/src/Editor/Infrastructure/Api/MainThreadBlockedErrorData.cs.meta create mode 100644 Packages/src/Editor/Infrastructure/Threading/EditorMainThreadLivenessPort.cs create mode 100644 Packages/src/Editor/Infrastructure/Threading/EditorMainThreadLivenessPort.cs.meta diff --git a/Assets/Tests/Editor/JsonRpcResponseFactoryWireShapeCharacterizationTests.cs b/Assets/Tests/Editor/JsonRpcResponseFactoryWireShapeCharacterizationTests.cs index 705ea27ebf..046fd51dca 100644 --- a/Assets/Tests/Editor/JsonRpcResponseFactoryWireShapeCharacterizationTests.cs +++ b/Assets/Tests/Editor/JsonRpcResponseFactoryWireShapeCharacterizationTests.cs @@ -148,6 +148,23 @@ public void CreateErrorResponse_ForBusyException_IncludesRunningToolPhaseOnlyWhe Assert.That(unknownPhaseData["runningToolPhase"], Is.Null); } + /// + /// Verifies a blocked-main-thread failure reaches the wire as main_thread_blocked data with the tool name and stall seconds. + /// + [Test] + public void CreateErrorResponse_ForMainThreadBlockedException_ProducesMainThreadBlockedData() + { + UnityCliLoopMainThreadBlockedException blockedException = new("get-hierarchy", 45); + + string response = JsonRpcResponseFactory.CreateErrorResponse(1, blockedException); + + JToken dataToken = JObject.Parse(response)["error"]!["data"]!; + Assert.That(dataToken["type"]!.Value(), Is.EqualTo("main_thread_blocked")); + Assert.That(dataToken["runningToolName"]!.Value(), Is.EqualTo("get-hierarchy")); + Assert.That(dataToken["stallSeconds"]!.Value(), Is.EqualTo(45)); + Assert.That(dataToken["message"]!.Value(), Is.Not.Empty); + } + [Test] public async Task ProcessRequest_WhenProtocolVersionIsTooOld_ProducesFrozenMismatchJson() { diff --git a/Assets/Tests/Editor/MainThreadEntryWatchdogTests.cs b/Assets/Tests/Editor/MainThreadEntryWatchdogTests.cs new file mode 100644 index 0000000000..f67f20e9a8 --- /dev/null +++ b/Assets/Tests/Editor/MainThreadEntryWatchdogTests.cs @@ -0,0 +1,175 @@ +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; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Tests the watchdog that fails a request still waiting for a blocked Editor main thread. + /// + public sealed class MainThreadEntryWatchdogTests + { + /// + /// Verifies a stall below the threshold leaves the waiting request alone. + /// + [Test] + public void Observe_WhenStallBelowThreshold_LeavesLeaseAlone() + { + ToolExecutionLease lease = new ToolExecutionSession().TryEnter("tool").Lease; + ManualPolls polls = new ManualPolls(); + MainThreadEntryWatchdog watchdog = new MainThreadEntryWatchdog(new ScriptedLiveness(29.9), polls.WaitOnePoll); + + bool cancelled = watchdog.Observe(lease); + + Assert.That(cancelled, Is.False); + Assert.That(lease.ExecutionToken.IsCancellationRequested, Is.False); + lease.Dispose(); + } + + /// + /// Verifies a stall at the threshold cancels the waiting request and records the stall it saw. + /// + [Test] + public void Observe_WhenStallAtThreshold_CancelsLeaseWithStallSeconds() + { + ToolExecutionLease lease = new ToolExecutionSession().TryEnter("tool").Lease; + ManualPolls polls = new ManualPolls(); + MainThreadEntryWatchdog watchdog = new MainThreadEntryWatchdog(new ScriptedLiveness(30.0), polls.WaitOnePoll); + + bool cancelled = watchdog.Observe(lease); + + Assert.That(cancelled, Is.True); + Assert.That(lease.WasCancelledForBlockedMainThread, Is.True); + Assert.That(lease.BlockedMainThreadStallSeconds, Is.EqualTo(30)); + lease.Dispose(); + } + + /// + /// Verifies the first stall check waits for one poll, so an idle Editor that only slept is never judged. + /// + [Test] + public void GuardAsync_WhenStoppedBeforeFirstPoll_NeverReadsLiveness() + { + ToolExecutionLease lease = new ToolExecutionSession().TryEnter("tool").Lease; + ManualPolls polls = new ManualPolls(); + ScriptedLiveness liveness = new ScriptedLiveness(45); + MainThreadEntryWatchdog watchdog = new MainThreadEntryWatchdog(liveness, polls.WaitOnePoll); + CancellationTokenSource stop = new CancellationTokenSource(); + SynchronizationContext originalContext = SynchronizationContext.Current; + SynchronizationContext.SetSynchronizationContext(null); + + try + { + Task guardTask = watchdog.GuardAsync(lease, stop.Token); + + Assert.That(polls.Count, Is.EqualTo(1)); + Assert.That(liveness.Reads, Is.EqualTo(0)); + + stop.Cancel(); + + Assert.That(guardTask.Status, Is.EqualTo(TaskStatus.RanToCompletion)); + Assert.That(liveness.Reads, Is.EqualTo(0)); + Assert.That(lease.ExecutionToken.IsCancellationRequested, Is.False); + } + finally + { + SynchronizationContext.SetSynchronizationContext(originalContext); + stop.Dispose(); + lease.Dispose(); + } + } + + /// + /// Verifies the watchdog keeps polling while the stall grows and cancels the request once it reaches the threshold. + /// + [Test] + public void GuardAsync_PollsUntilTheStallReachesTheThreshold() + { + ToolExecutionLease lease = new ToolExecutionSession().TryEnter("tool").Lease; + ManualPolls polls = new ManualPolls(); + ScriptedLiveness liveness = new ScriptedLiveness(10, 20, 45); + MainThreadEntryWatchdog watchdog = new MainThreadEntryWatchdog(liveness, polls.WaitOnePoll); + CancellationTokenSource stop = new CancellationTokenSource(); + SynchronizationContext originalContext = SynchronizationContext.Current; + SynchronizationContext.SetSynchronizationContext(null); + + try + { + Task guardTask = watchdog.GuardAsync(lease, stop.Token); + + polls.CompleteLatest(); + Assert.That(lease.ExecutionToken.IsCancellationRequested, Is.False); + Assert.That(polls.Count, Is.EqualTo(2)); + + polls.CompleteLatest(); + Assert.That(lease.ExecutionToken.IsCancellationRequested, Is.False); + Assert.That(polls.Count, Is.EqualTo(3)); + + polls.CompleteLatest(); + Assert.That(lease.WasCancelledForBlockedMainThread, Is.True); + Assert.That(guardTask.Status, Is.EqualTo(TaskStatus.RanToCompletion)); + Assert.That(polls.Count, Is.EqualTo(3)); + } + finally + { + SynchronizationContext.SetSynchronizationContext(originalContext); + stop.Cancel(); + stop.Dispose(); + lease.Dispose(); + } + } + } + + // Stands in for the one-second poll: each poll is a pending task the test completes by hand, + // and stopping the watchdog cancels the pending one so no background work is left behind. + internal sealed class ManualPolls + { + private readonly List> _polls = new List>(); + + internal int Count => _polls.Count; + + internal Task Latest => _polls[_polls.Count - 1].Task; + + internal Task WaitOnePoll(CancellationToken ct) + { + TaskCompletionSource poll = new TaskCompletionSource(); + ct.Register(() => poll.TrySetCanceled()); + _polls.Add(poll); + return poll.Task; + } + + internal void CompleteLatest() + { + _polls[_polls.Count - 1].SetResult(true); + } + } + + // Returns the scripted stall values in order (repeating the last one) and runs an optional hook + // at each read, so a test can let the main thread resume while the watchdog is deciding. + internal sealed class ScriptedLiveness : IMainThreadLivenessPort + { + private readonly double[] _values; + + internal ScriptedLiveness(params double[] values) + { + _values = values; + } + + internal int Reads { get; private set; } + + internal Action OnRead { get; set; } + + public double SecondsSinceLastMainThreadTick() + { + OnRead?.Invoke(); + double value = _values[Math.Min(Reads, _values.Length - 1)]; + Reads++; + return value; + } + } +} diff --git a/Assets/Tests/Editor/MainThreadEntryWatchdogTests.cs.meta b/Assets/Tests/Editor/MainThreadEntryWatchdogTests.cs.meta new file mode 100644 index 0000000000..c4307ee574 --- /dev/null +++ b/Assets/Tests/Editor/MainThreadEntryWatchdogTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 12f14c32255a04a309cf9b45108a9c1a +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/ToolExecutionSessionTests.cs b/Assets/Tests/Editor/ToolExecutionSessionTests.cs index 054d055c49..2f259f7904 100644 --- a/Assets/Tests/Editor/ToolExecutionSessionTests.cs +++ b/Assets/Tests/Editor/ToolExecutionSessionTests.cs @@ -852,6 +852,126 @@ public void TryEnter_WhenDynamicCodeHolderIsCancelledByUserPastGrace_ShouldLetOt holderLease.Dispose(); } + [Test] + public void TryCancelForBlockedMainThread_WhileWaiting_CancelsExecutionTokenAndRecordsStall() + { + // Tests that a blocked-main-thread cancel before tool entry cancels the token and is not reported as a user cancel. + ToolExecutionSession session = new ToolExecutionSession(); + ToolExecutionLease lease = session.TryEnter("tool").Lease; + + bool cancelled = lease.TryCancelForBlockedMainThread(45); + + Assert.That(cancelled, Is.True); + Assert.That(lease.ExecutionToken.IsCancellationRequested, Is.True); + Assert.That(lease.WasCancelledForBlockedMainThread, Is.True); + Assert.That(lease.WasCancelledByUser, Is.False); + Assert.That(lease.BlockedMainThreadStallSeconds, Is.EqualTo(45)); + + lease.Dispose(); + } + + [Test] + public void TryCancelForBlockedMainThread_AfterToolEntered_ReturnsFalseAndLeavesTokenAlive() + { + // Tests that once the tool entered, the blocked-main-thread cancel loses and leaves the running tool alone. + ToolExecutionSession session = new ToolExecutionSession(); + ToolExecutionLease lease = session.TryEnter("tool").Lease; + + bool entered = lease.MarkToolEntered(); + bool cancelled = lease.TryCancelForBlockedMainThread(45); + + Assert.That(entered, Is.True); + Assert.That(cancelled, Is.False); + Assert.That(lease.ExecutionToken.IsCancellationRequested, Is.False); + Assert.That(lease.WasCancelledForBlockedMainThread, Is.False); + + lease.Dispose(); + } + + [Test] + public void TryCancelForBlockedMainThread_AfterUserCancel_ReturnsFalseAndKeepsUserReason() + { + // Tests that a uloop cancel recorded first keeps its reason when the blocked-main-thread cancel comes later. + ToolExecutionSession session = new ToolExecutionSession(); + ToolExecutionLease lease = session.TryEnter("tool").Lease; + + lease.CancelByUser(); + bool cancelled = lease.TryCancelForBlockedMainThread(45); + + Assert.That(cancelled, Is.False); + Assert.That(lease.WasCancelledByUser, Is.True); + Assert.That(lease.WasCancelledForBlockedMainThread, Is.False); + + lease.Dispose(); + } + + [Test] + public void CancelByUser_AfterBlockedCancel_KeepsBlockedReason() + { + // Tests that a uloop cancel arriving after the blocked-main-thread cancel does not overwrite the blocked reason. + ToolExecutionSession session = new ToolExecutionSession(); + ToolExecutionLease lease = session.TryEnter("tool").Lease; + + lease.TryCancelForBlockedMainThread(45); + lease.CancelByUser(); + + Assert.That(lease.WasCancelledByUser, Is.False); + Assert.That(lease.WasCancelledForBlockedMainThread, Is.True); + + lease.Dispose(); + } + + [Test] + public void CancelByUser_AfterToolEntered_ReportsUserCancel() + { + // Tests that uloop cancel on a running tool is still reported as a user cancel. + ToolExecutionSession session = new ToolExecutionSession(); + ToolExecutionLease lease = session.TryEnter("tool").Lease; + + lease.MarkToolEntered(); + lease.CancelByUser(); + + Assert.That(lease.WasCancelledByUser, Is.True); + + lease.Dispose(); + } + + [Test] + public void MarkToolEntered_AfterBlockedClaim_ReturnsFalseBeforeTheTokenIsCancelled() + { + // Tests that a claimed lease refuses tool entry even in the moment before the claimer cancels the token. + ToolExecutionSession session = new ToolExecutionSession(); + ToolExecutionLease lease = session.TryEnter("tool").Lease; + + bool claimed = lease.TryClaimBlockedMainThread(45); + bool entered = lease.MarkToolEntered(); + + Assert.That(claimed, Is.True); + Assert.That(entered, Is.False); + Assert.That(lease.ExecutionToken.IsCancellationRequested, Is.False); + Assert.That(lease.WasCancelledForBlockedMainThread, Is.True); + Assert.That(lease.BlockedMainThreadStallSeconds, Is.EqualTo(45)); + + lease.Dispose(); + } + + [Test] + public void MarkToolEntered_Twice_ReturnsFalseTheSecondTime() + { + // Tests that tool entry is claimed once and a second claim does not change the entered state. + ToolExecutionSession session = new ToolExecutionSession(); + ToolExecutionLease lease = session.TryEnter("tool").Lease; + + bool first = lease.MarkToolEntered(); + bool second = lease.MarkToolEntered(); + + Assert.That(first, Is.True); + Assert.That(second, Is.False); + Assert.That(lease.WasCancelledByUser, Is.False); + + lease.Dispose(); + } + private static long GraceTicks => ToolExecutionSession.CancelledLeaseGraceSeconds * Stopwatch.Frequency; private static UnityCliLoopToolRegistry CreateRegistry(InMemoryToolSettingsPort settingsPort, IUnityCliLoopTool[] tools) diff --git a/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs b/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs index 90575c3970..09f018cb88 100644 --- a/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs @@ -501,6 +501,289 @@ public void ScreenshotUseCase_CreateTimedOutResult_PreservesTimeoutDetails() Assert.That(response.ScreenshotCount, Is.EqualTo(1)); } + /// + /// Verifies a request still waiting for a main thread that stays blocked fails with the blocked-main-thread error, frees the slot, and never runs its tool. + /// + [Test] + public void ExecuteToolAsync_WhenMainThreadStaysBlockedBeforeToolEntry_ThrowsMainThreadBlockedAndReleasesSlot() + { + AssertBlockedBeforeToolEntryThrowsMainThreadBlocked(cancellableByUser: true); + } + + /// + /// Verifies the blocked-main-thread failure also applies to requests uloop cancel may not stop, because no tool code has run yet. + /// + [Test] + public void ExecuteToolAsync_WhenNotCancellableByUserAndMainThreadBlocked_StillThrowsMainThreadBlocked() + { + AssertBlockedBeforeToolEntryThrowsMainThreadBlocked(cancellableByUser: false); + } + + /// + /// Verifies a main thread that resumes before the stall reaches the threshold runs the tool and stops the watchdog. + /// + [Test] + public void ExecuteToolAsync_WhenMainThreadResumesBeforeThreshold_RunsTool() + { + ToolExecutionSession session = new ToolExecutionSession(); + UnityCliLoopToolRegistry registry = ToolRegistryTestFactory.Create(); + ImmediateTool tool = new ImmediateTool(ImmediateTool.HolderName); + registry.RegisterTool(tool); + ManualPolls polls = new ManualPolls(); + UnityCliLoopToolExecutionService executionService = CreateServiceWithWatchdog(session, new ScriptedLiveness(10), polls); + + RunWithQueueingDispatcherAndNoContext(dispatcher => + { + Task task = executionService.ExecuteToolAsync( + registry, + ImmediateTool.HolderName, + null, + CancellationToken.None); + polls.CompleteLatest(); + + Assert.That(task.IsCompleted, Is.False); + + dispatcher.RunQueued(); + + Assert.That(task.Status, Is.EqualTo(TaskStatus.RanToCompletion)); + Assert.That(tool.ExecutionCount, Is.EqualTo(1)); + Assert.That(polls.Latest.IsCanceled, Is.True); + }); + } + + /// + /// Verifies the watchdog stops once the tool code runs, so a tool that waits for the main thread itself is never failed as blocked. + /// + [Test] + public void ExecuteToolAsync_WhenToolCodeWaitsForMainThreadAfterEntry_WatchdogIsStopped() + { + ToolExecutionSession session = new ToolExecutionSession(); + UnityCliLoopToolRegistry registry = ToolRegistryTestFactory.Create(); + registry.RegisterTool(new MainThreadSwitchingTool()); + ManualPolls polls = new ManualPolls(); + UnityCliLoopToolExecutionService executionService = CreateServiceWithWatchdog(session, new ScriptedLiveness(45), polls); + + RunWithQueueingDispatcherAndNoContext(dispatcher => + { + Task holderTask = executionService.ExecuteToolAsync( + registry, + MainThreadSwitchingTool.ToolNameValue, + null, + CancellationToken.None); + // Resumes the switch before the tool, so the tool code runs and queues its own switch. + dispatcher.RunQueued(); + + Assert.That(polls.Latest.IsCanceled, Is.True); + Assert.That(holderTask.IsCompleted, Is.False); + + dispatcher.RunQueued(); + + Assert.That(holderTask.Status, Is.EqualTo(TaskStatus.RanToCompletion)); + }); + } + + /// + /// Verifies a lease claimed for a blocked main thread refuses the tool even when the main thread resumes before the claimer cancels the token. + /// + [Test] + public void ExecuteToolAsync_WhenWatchdogClaimsTheLeaseBeforeTheMainThreadResumes_ThrowsMainThreadBlockedWithoutRunningTool() + { + ToolExecutionSession session = new ToolExecutionSession(); + UnityCliLoopToolRegistry registry = ToolRegistryTestFactory.Create(); + ImmediateTool tool = new ImmediateTool(ImmediateTool.HolderName); + registry.RegisterTool(tool); + CapturingWatchdog capturing = new CapturingWatchdog(null); + UnityCliLoopToolExecutionService executionService = + new UnityCliLoopToolExecutionService(new NoOpEditorRuntimeStatePort(), session, capturing); + + RunWithQueueingDispatcherAndNoContext(dispatcher => + { + Task task = executionService.ExecuteToolAsync( + registry, + ImmediateTool.HolderName, + null, + CancellationToken.None); + + Assert.That(capturing.Lease.TryClaimBlockedMainThread(45), Is.True); + Assert.That(capturing.Lease.ExecutionToken.IsCancellationRequested, Is.False); + + dispatcher.RunQueued(); + + UnityCliLoopMainThreadBlockedException exception = + Assert.ThrowsAsync(() => task); + Assert.That(exception.StallSeconds, Is.EqualTo(45)); + Assert.That(tool.ExecutionCount, Is.EqualTo(0)); + ToolExecutionSessionEnterResult nextEnter = session.TryEnter("other-tool"); + Assert.That(nextEnter.IsEntered, Is.True); + nextEnter.Lease.Dispose(); + }); + } + + /// + /// Verifies a main thread that resumes while the watchdog is deciding wins, so the tool runs and is not failed as blocked. + /// + [Test] + public void ExecuteToolAsync_WhenMainThreadResumesWhileTheWatchdogIsDeciding_RunsToolAndIsNotCancelled() + { + ToolExecutionSession session = new ToolExecutionSession(); + UnityCliLoopToolRegistry registry = ToolRegistryTestFactory.Create(); + ImmediateTool tool = new ImmediateTool(ImmediateTool.HolderName); + registry.RegisterTool(tool); + ManualPolls polls = new ManualPolls(); + ScriptedLiveness liveness = new ScriptedLiveness(45); + CapturingWatchdog capturing = new CapturingWatchdog(new MainThreadEntryWatchdog(liveness, polls.WaitOnePoll)); + UnityCliLoopToolExecutionService executionService = + new UnityCliLoopToolExecutionService(new NoOpEditorRuntimeStatePort(), session, capturing); + + RunWithQueueingDispatcherAndNoContext(dispatcher => + { + liveness.OnRead = dispatcher.RunQueued; + Task task = executionService.ExecuteToolAsync( + registry, + ImmediateTool.HolderName, + null, + CancellationToken.None); + + polls.CompleteLatest(); + + Assert.That(task.Status, Is.EqualTo(TaskStatus.RanToCompletion)); + Assert.That(tool.ExecutionCount, Is.EqualTo(1)); + Assert.That(liveness.Reads, Is.EqualTo(1)); + Assert.That(capturing.Lease.WasCancelledForBlockedMainThread, Is.False); + }); + } + + /// + /// Verifies a uloop cancel that reaches a waiting request before the watchdog keeps its cancelled outcome and stops the watchdog. + /// + [Test] + public void ExecuteToolAsync_WhenUserCancelsBeforeTheWatchdog_ThrowsToolCancelled() + { + ToolExecutionSession session = new ToolExecutionSession(); + UnityCliLoopToolRegistry registry = ToolRegistryTestFactory.Create(); + registry.RegisterTool(new ImmediateTool(ImmediateTool.HolderName)); + ManualPolls polls = new ManualPolls(); + UnityCliLoopToolExecutionService executionService = CreateServiceWithWatchdog(session, new ScriptedLiveness(45), polls); + + RunWithQueueingDispatcherAndNoContext(dispatcher => + { + Task task = executionService.ExecuteToolAsync( + registry, + ImmediateTool.HolderName, + null, + CancellationToken.None); + + executionService.CancelRunningTool(); + + Assert.ThrowsAsync(() => task); + Assert.That(polls.Latest.IsCanceled, Is.True); + }); + } + + /// + /// Verifies that when an execute-dynamic-code run blocks the main thread from inside its code, only the request sharing its slot and still waiting fails as blocked. + /// + [Test] + public void ExecuteToolAsync_WhenASharedSlotHolderBlocksTheMainThread_FailsTheWaitingSharerAndKeepsTheHolder() + { + ToolExecutionSession session = new ToolExecutionSession(); + UnityCliLoopToolRegistry registry = ToolRegistryTestFactory.Create(); + registry.RegisterTool(new DynamicCodeMainThreadSwitchingTool()); + ManualPolls polls = new ManualPolls(); + UnityCliLoopToolExecutionService executionService = CreateServiceWithWatchdog(session, new ScriptedLiveness(45), polls); + + RunWithQueueingDispatcherAndNoContext(dispatcher => + { + Task holderTask = executionService.ExecuteToolAsync( + registry, + UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE, + null, + CancellationToken.None); + dispatcher.RunQueued(); + Assert.That(polls.Latest.IsCanceled, Is.True); + + Task sharerTask = executionService.ExecuteToolAsync( + registry, + UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE, + null, + CancellationToken.None); + polls.CompleteLatest(); + + Assert.ThrowsAsync(() => sharerTask); + Assert.That(holderTask.IsCompleted, Is.False); + + dispatcher.RunQueued(); + + Assert.That(holderTask.Status, Is.EqualTo(TaskStatus.RanToCompletion)); + }); + } + + private static void AssertBlockedBeforeToolEntryThrowsMainThreadBlocked(bool cancellableByUser) + { + ToolExecutionSession session = new ToolExecutionSession(); + UnityCliLoopToolRegistry registry = ToolRegistryTestFactory.Create(); + ImmediateTool tool = new ImmediateTool(ImmediateTool.HolderName); + registry.RegisterTool(tool); + ManualPolls polls = new ManualPolls(); + UnityCliLoopToolExecutionService executionService = CreateServiceWithWatchdog(session, new ScriptedLiveness(45), polls); + + RunWithQueueingDispatcherAndNoContext(dispatcher => + { + Task task = executionService.ExecuteToolAsync( + registry, + ImmediateTool.HolderName, + null, + CancellationToken.None, + cancellableByUser); + + Assert.That(task.IsCompleted, Is.False); + + polls.CompleteLatest(); + + UnityCliLoopMainThreadBlockedException exception = + Assert.ThrowsAsync(() => task); + Assert.That(exception.ToolName, Is.EqualTo(ImmediateTool.HolderName)); + Assert.That(exception.StallSeconds, Is.EqualTo(45)); + ToolExecutionSessionEnterResult nextEnter = session.TryEnter("other-tool"); + Assert.That(nextEnter.IsEntered, Is.True); + nextEnter.Lease.Dispose(); + dispatcher.RunQueued(); + Assert.That(tool.ExecutionCount, Is.EqualTo(0)); + }); + } + + private static UnityCliLoopToolExecutionService CreateServiceWithWatchdog( + ToolExecutionSession session, + ScriptedLiveness liveness, + ManualPolls polls) + { + return new UnityCliLoopToolExecutionService( + new NoOpEditorRuntimeStatePort(), + session, + new MainThreadEntryWatchdog(liveness, polls.WaitOnePoll)); + } + + // Why clear the context: with no context every continuation in the service and the watchdog + // runs synchronously on the thread that completes the poll or runs the queue, so each step's + // outcome is visible right after it without waiting or leaving pool work behind. + private static void RunWithQueueingDispatcherAndNoContext(Action body) + { + QueueingMainThreadDispatcher dispatcher = new QueueingMainThreadDispatcher(); + MainThreadSwitcher.RegisterService(dispatcher); + SynchronizationContext originalContext = SynchronizationContext.Current; + SynchronizationContext.SetSynchronizationContext(null); + + try + { + body(dispatcher); + } + finally + { + SynchronizationContext.SetSynchronizationContext(originalContext); + RestoreEditorMainThreadDispatcher(); + } + } + private static void RestoreEditorMainThreadDispatcher() { EditorMainThreadDispatcher dispatcher = new EditorMainThreadDispatcher(); @@ -585,8 +868,11 @@ public ImmediateTool(string toolName) public ToolParameterSchema ParameterSchema => new(); + public int ExecutionCount { get; private set; } + public Task ExecuteAsync(JToken paramsToken, CancellationToken ct) { + ExecutionCount++; return Task.FromResult(new PendingTypedResponse()); } } @@ -607,6 +893,41 @@ public async Task ExecuteAsync(JToken paramsToken, Can } } + // Shares the execute-dynamic-code slot and awaits the main thread inside its code, as a + // snippet that keeps the main thread busy would. + private sealed class DynamicCodeMainThreadSwitchingTool : IUnityCliLoopTool + { + public string ToolName => UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE; + + public ToolParameterSchema ParameterSchema => new(); + + public async Task ExecuteAsync(JToken paramsToken, CancellationToken ct) + { + await MainThreadSwitcher.SwitchToMainThread(ct); + return new PendingTypedResponse(); + } + } + + // Hands the test the lease the service guards, so the test can claim it at a chosen moment or + // inspect it after the real watchdog it wraps has decided. + private sealed class CapturingWatchdog : IMainThreadEntryWatchdog + { + private readonly IMainThreadEntryWatchdog _inner; + + public CapturingWatchdog(IMainThreadEntryWatchdog inner) + { + _inner = inner; + } + + public ToolExecutionLease Lease { get; private set; } + + public Task GuardAsync(ToolExecutionLease lease, CancellationToken stop) + { + Lease = lease; + return _inner == null ? Task.CompletedTask : _inner.GuardAsync(lease, stop); + } + } + private sealed class ImmediateMainThreadDispatcher : IMainThreadDispatcher { public bool IsMainThread => true; diff --git a/Packages/src/Editor/Application/IMainThreadEntryWatchdog.cs b/Packages/src/Editor/Application/IMainThreadEntryWatchdog.cs new file mode 100644 index 0000000000..a6878925b3 --- /dev/null +++ b/Packages/src/Editor/Application/IMainThreadEntryWatchdog.cs @@ -0,0 +1,17 @@ +using System.Threading; +using System.Threading.Tasks; + +using io.github.hatayama.UnityCliLoop.Domain; + +namespace io.github.hatayama.UnityCliLoop.Application +{ + /// + /// Guards one request between taking the command slot and entering its tool code: the guard + /// may cancel the lease while the request still waits for the Editor main thread, and must + /// stop doing anything once the stop token is cancelled. + /// + internal interface IMainThreadEntryWatchdog + { + Task GuardAsync(ToolExecutionLease lease, CancellationToken stop); + } +} diff --git a/Packages/src/Editor/Application/IMainThreadEntryWatchdog.cs.meta b/Packages/src/Editor/Application/IMainThreadEntryWatchdog.cs.meta new file mode 100644 index 0000000000..911232832a --- /dev/null +++ b/Packages/src/Editor/Application/IMainThreadEntryWatchdog.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a104cad559fa44dd88c4bef30dd3730d +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Application/ImprovedErrorHandler.cs b/Packages/src/Editor/Application/ImprovedErrorHandler.cs index b67a3ac36f..80a63b3e9c 100644 --- a/Packages/src/Editor/Application/ImprovedErrorHandler.cs +++ b/Packages/src/Editor/Application/ImprovedErrorHandler.cs @@ -92,6 +92,19 @@ public TranslationOutput TranslateFromException(Exception exception) }; } + if (exception is UnityCliLoopMainThreadBlockedException blockedException) + { + return new TranslationOutput + { + FriendlyMessage = blockedException.Message, + Explanation = blockedException.Message, + Solutions = new List + { + "Check the Unity window for a modal dialog or a long operation, then run the command once more" + } + }; + } + if (exception is TimeoutException) { return new TranslationOutput diff --git a/Packages/src/Editor/Application/MainThreadEntryWatchdog.cs b/Packages/src/Editor/Application/MainThreadEntryWatchdog.cs new file mode 100644 index 0000000000..b64bb56d98 --- /dev/null +++ b/Packages/src/Editor/Application/MainThreadEntryWatchdog.cs @@ -0,0 +1,77 @@ +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; + +using io.github.hatayama.UnityCliLoop.Domain; + +namespace io.github.hatayama.UnityCliLoop.Application +{ + /// + /// Cancels a request that is still waiting for its first main-thread switch once the Editor + /// main thread has stalled for BlockedStallSeconds, so the CLI gets an answer instead of a + /// silent wait. Never touches a request whose tool code has started. + /// + internal sealed class MainThreadEntryWatchdog : IMainThreadEntryWatchdog + { + // Matches the CLI's stall notice threshold and compile's silent-wait warning: the longest + // ordinary Editor work (a scene save, a small import) stays under it, a dialog does not. + internal const double BlockedStallSeconds = 30; + internal static readonly TimeSpan PollInterval = TimeSpan.FromSeconds(1); + + private readonly IMainThreadLivenessPort _liveness; + private readonly Func _waitOnePoll; + + internal MainThreadEntryWatchdog(IMainThreadLivenessPort liveness, Func waitOnePoll) + { + Debug.Assert(liveness != null, "liveness must not be null"); + Debug.Assert(waitOnePoll != null, "waitOnePoll must not be null"); + + _liveness = liveness ?? throw new ArgumentNullException(nameof(liveness)); + _waitOnePoll = waitOnePoll ?? throw new ArgumentNullException(nameof(waitOnePoll)); + } + + // Returns true when the lease was cancelled for a blocked main thread. + internal bool Observe(ToolExecutionLease lease) + { + Debug.Assert(lease != null, "lease must not be null"); + + double stallSeconds = _liveness.SecondsSinceLastMainThreadTick(); + if (stallSeconds < BlockedStallSeconds) + { + return false; + } + + return lease.TryCancelForBlockedMainThread(stallSeconds); + } + + // Why the first Observe comes after the first poll: an idle Editor's loop sleeps and its stall + // counter grows; the switch request woke it, and one poll interval is enough for it to tick. + public async Task GuardAsync(ToolExecutionLease lease, CancellationToken stop) + { + Debug.Assert(lease != null, "lease must not be null"); + + while (!stop.IsCancellationRequested) + { + try + { + await _waitOnePoll(stop).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + return; + } + + if (stop.IsCancellationRequested) + { + return; + } + + if (Observe(lease)) + { + return; + } + } + } + } +} diff --git a/Packages/src/Editor/Application/MainThreadEntryWatchdog.cs.meta b/Packages/src/Editor/Application/MainThreadEntryWatchdog.cs.meta new file mode 100644 index 0000000000..be8b87b11e --- /dev/null +++ b/Packages/src/Editor/Application/MainThreadEntryWatchdog.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 679e8c8190e3a435f9ff498495ea4c0b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Application/MainThreadLivenessPort.cs b/Packages/src/Editor/Application/MainThreadLivenessPort.cs new file mode 100644 index 0000000000..837fd7583b --- /dev/null +++ b/Packages/src/Editor/Application/MainThreadLivenessPort.cs @@ -0,0 +1,12 @@ +namespace io.github.hatayama.UnityCliLoop.Application +{ + /// + /// Reads how long the Unity Editor main thread has gone without a tick, so Application code can + /// tell a blocked Editor from a busy one without depending on Editor platform types. Safe to read + /// from any thread. + /// + public interface IMainThreadLivenessPort + { + double SecondsSinceLastMainThreadTick(); + } +} diff --git a/Packages/src/Editor/Application/MainThreadLivenessPort.cs.meta b/Packages/src/Editor/Application/MainThreadLivenessPort.cs.meta new file mode 100644 index 0000000000..2e3e01fd5e --- /dev/null +++ b/Packages/src/Editor/Application/MainThreadLivenessPort.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 10a81dbacf62f4de89ff8d0b96a400a7 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Application/NoMainThreadEntryWatchdog.cs b/Packages/src/Editor/Application/NoMainThreadEntryWatchdog.cs new file mode 100644 index 0000000000..b2dea4d28d --- /dev/null +++ b/Packages/src/Editor/Application/NoMainThreadEntryWatchdog.cs @@ -0,0 +1,25 @@ +using System.Threading; +using System.Threading.Tasks; + +using io.github.hatayama.UnityCliLoop.Domain; + +namespace io.github.hatayama.UnityCliLoop.Application +{ + /// + /// Leaves every request waiting for the main thread as long as it takes. Tests build the + /// execution service without a watchdog; only the composition root wires the real one. + /// + internal sealed class NoMainThreadEntryWatchdog : IMainThreadEntryWatchdog + { + internal static readonly NoMainThreadEntryWatchdog Instance = new(); + + private NoMainThreadEntryWatchdog() + { + } + + public Task GuardAsync(ToolExecutionLease lease, CancellationToken stop) + { + return Task.CompletedTask; + } + } +} diff --git a/Packages/src/Editor/Application/NoMainThreadEntryWatchdog.cs.meta b/Packages/src/Editor/Application/NoMainThreadEntryWatchdog.cs.meta new file mode 100644 index 0000000000..d6fd2a919c --- /dev/null +++ b/Packages/src/Editor/Application/NoMainThreadEntryWatchdog.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 08cd383391cdf45e48e593a4b555726b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Application/UnityCliLoopMainThreadBlockedException.cs b/Packages/src/Editor/Application/UnityCliLoopMainThreadBlockedException.cs new file mode 100644 index 0000000000..d937ef2331 --- /dev/null +++ b/Packages/src/Editor/Application/UnityCliLoopMainThreadBlockedException.cs @@ -0,0 +1,29 @@ +using System; + +namespace io.github.hatayama.UnityCliLoop.Application +{ + /// + /// Reports that a request stopped waiting for a blocked Editor main thread before any of its tool + /// code ran, so the protocol layer can answer the waiting CLI with a main_thread_blocked error. + /// Why not an OperationCanceledException: the request pipeline treats that as a disconnect and + /// writes no response. + /// + public sealed class UnityCliLoopMainThreadBlockedException : Exception + { + public UnityCliLoopMainThreadBlockedException(string toolName, double stallSeconds) + : base(CreateMessage(toolName, stallSeconds)) + { + ToolName = toolName; + StallSeconds = stallSeconds; + } + + public string ToolName { get; } + + public double StallSeconds { get; } + + private static string CreateMessage(string toolName, double stallSeconds) + { + return $"'{toolName}' was not run: the Unity Editor main thread has not run for {stallSeconds:F0}s, so uloop stopped waiting and released the command slot."; + } + } +} diff --git a/Packages/src/Editor/Application/UnityCliLoopMainThreadBlockedException.cs.meta b/Packages/src/Editor/Application/UnityCliLoopMainThreadBlockedException.cs.meta new file mode 100644 index 0000000000..e3e9d12869 --- /dev/null +++ b/Packages/src/Editor/Application/UnityCliLoopMainThreadBlockedException.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 1fe15fcf201624709883f793916de892 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs b/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs index ec49fe3816..6f17c4411d 100644 --- a/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs +++ b/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs @@ -16,15 +16,18 @@ internal sealed class UnityCliLoopToolExecutionService { private readonly IEditorRuntimeStatePort _editorRuntimeStatePort; private readonly ToolExecutionSession _executionSession; + private readonly IMainThreadEntryWatchdog _watchdog; internal UnityCliLoopToolExecutionService( IEditorRuntimeStatePort editorRuntimeStatePort, - ToolExecutionSession executionSession = null) + ToolExecutionSession executionSession = null, + IMainThreadEntryWatchdog watchdog = null) { Debug.Assert(editorRuntimeStatePort != null, "editorRuntimeStatePort must not be null"); _editorRuntimeStatePort = editorRuntimeStatePort ?? throw new ArgumentNullException(nameof(editorRuntimeStatePort)); _executionSession = executionSession ?? new ToolExecutionSession(); + _watchdog = watchdog ?? NoMainThreadEntryWatchdog.Instance; } internal async Task ExecuteToolAsync( @@ -59,7 +62,30 @@ internal async Task ExecuteToolAsync( // execute-dynamic-code reading the pause state), and only the switch itself sees that. // The value set here flows into the tool and is restored for this method's caller. MainThreadWaitObservation.SetCurrent(new ToolExecutionLeaseWaitObserver(lease)); - await MainThreadSwitcher.SwitchToMainThread(executionToken); + // Why never disposed: the guard may still be between its stop check and its next poll when + // the switch returns, and Task.Delay on a disposed token throws; a CTS without a timer holds + // nothing to leak (same reasoning as the lease's explicit cancellation source). + CancellationTokenSource watchdogStop = new CancellationTokenSource(); + // Not awaited: Cancel() below ends it, and awaiting it on the main thread would add a hop. + // GuardAsync never throws, so the discarded task cannot hide a failure. + _ = _watchdog.GuardAsync(lease, watchdogStop.Token); + try + { + await MainThreadSwitcher.SwitchToMainThread(executionToken); + } + finally + { + watchdogStop.Cancel(); + } + + // CAS against the watchdog: whoever moves the state first decides whether the tool runs. + // Why the state and not the token: a claimer that won the CAS may not have cancelled the + // token yet, and the main thread can resume the switch from the Editor queue in that window. + if (!lease.MarkToolEntered()) + { + throw CreateNotEnteredException(lease, toolName, ct); + } + executionToken.ThrowIfCancellationRequested(); UnityCliLoopEditorStateGuard.Validate(toolName, _editorRuntimeStatePort); @@ -71,6 +97,10 @@ internal async Task ExecuteToolAsync( return response; } + catch (OperationCanceledException) when (lease.WasCancelledForBlockedMainThread && !ct.IsCancellationRequested) + { + throw new UnityCliLoopMainThreadBlockedException(toolName, lease.BlockedMainThreadStallSeconds); + } // Why convert: an explicit uloop cancel must reach the waiting CLI as its own error, while a // disconnect (the request token) still drops the connection because nobody is left to answer. catch (OperationCanceledException) when (lease.WasCancelledByUser && !ct.IsCancellationRequested) @@ -83,6 +113,24 @@ internal async Task ExecuteToolAsync( } } + // Builds the outcome for a lease that the watchdog or uloop cancel claimed before the tool + // entered; both claimers may still be about to cancel the token, so the state decides. + private static Exception CreateNotEnteredException(ToolExecutionLease lease, string toolName, CancellationToken ct) + { + if (ct.IsCancellationRequested) + { + // Client gone: the plain cancellation drops the connection, as for any disconnect. + return new OperationCanceledException(ct); + } + + if (lease.WasCancelledForBlockedMainThread) + { + return new UnityCliLoopMainThreadBlockedException(toolName, lease.BlockedMainThreadStallSeconds); + } + + return new UnityCliLoopToolCancelledException(toolName); + } + // Why convert here: Infrastructure cannot see the Domain result type, so the outcome leaves // in an Application type, the same way GetExecutionStatus does. internal UnityCliLoopCancelRequestResult CancelRunningTool() diff --git a/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs b/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs index 1e8b67a2bb..a021a9a5ad 100644 --- a/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs +++ b/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs @@ -1,3 +1,5 @@ +using System.Threading.Tasks; + using io.github.hatayama.UnityCliLoop.Application; using io.github.hatayama.UnityCliLoop.Domain; using io.github.hatayama.UnityCliLoop.Infrastructure; @@ -49,7 +51,11 @@ internal UnityCliLoopApplicationServices Register() UnityCliLoopToolRegistrarService toolRegistrarService = new( new SkillInstallLayoutInternalToolNameProvider(), toolSettingsPort, - new UnityCliLoopToolExecutionService(editorRuntimeStateService), + new UnityCliLoopToolExecutionService( + editorRuntimeStateService, + watchdog: new MainThreadEntryWatchdog( + new EditorMainThreadLivenessPort(), + ct => Task.Delay(MainThreadEntryWatchdog.PollInterval, ct))), UnityCliLoopToolDiscovery.DiscoverTools); ApplicationRegistrar.RegisterService(toolRegistrarService); ToolContractsRegistrar.RegisterService(toolRegistrarService); diff --git a/Packages/src/Editor/Domain/ToolExecutionSession.cs b/Packages/src/Editor/Domain/ToolExecutionSession.cs index b67ccf7827..e6a2722b72 100644 --- a/Packages/src/Editor/Domain/ToolExecutionSession.cs +++ b/Packages/src/Editor/Domain/ToolExecutionSession.cs @@ -313,6 +313,14 @@ internal sealed class ToolExecutionLease : IDisposable private long? _cancellationObservedTimestamp; // A count, not a flag: one request can have several switches queued at once (WhenAll). private int _pendingMainThreadWaits; + // Why a state and not a flag: the watchdog and the main thread race for the moment the tool + // enters; a CAS from Waiting decides who won, so a tool that started is never reported as blocked. + private const int EntryStateWaiting = 0; + private const int EntryStateEntered = 1; + private const int EntryStateBlockedMainThread = 2; + private const int EntryStateCancelledByUserBeforeEntry = 3; + private int _entryState = EntryStateWaiting; + private double _blockedMainThreadStallSeconds; internal ToolExecutionLease( ToolExecutionSession session, @@ -341,7 +349,25 @@ internal ToolExecutionLease( // fires this lease. Tools run with this token instead of the raw request token. internal CancellationToken ExecutionToken => _executionCancellation.Token; - internal bool WasCancelledByUser => _explicitCancellation.IsCancellationRequested; + // A blocked-main-thread cancel uses the same token but is not a user cancel. The state is read + // too because a uloop cancel can have claimed the lease before its Cancel() has run. + internal bool WasCancelledByUser + { + get + { + int state = Volatile.Read(ref _entryState); + if (state == EntryStateBlockedMainThread) + { + return false; + } + + return state == EntryStateCancelledByUserBeforeEntry || _explicitCancellation.IsCancellationRequested; + } + } + + internal bool WasCancelledForBlockedMainThread => Volatile.Read(ref _entryState) == EntryStateBlockedMainThread; + + internal double BlockedMainThreadStallSeconds => Volatile.Read(ref _blockedMainThreadStallSeconds); internal string ToolName { get; } @@ -368,7 +394,42 @@ internal void MarkMainThreadWaitEnded() // session lock or inside an activity hold. internal void CancelByUser() { + Interlocked.CompareExchange(ref _entryState, EntryStateCancelledByUserBeforeEntry, EntryStateWaiting); + _explicitCancellation.Cancel(); + } + + // Called by the execution service right after the first main-thread switch returns. False means + // the lease was claimed first (blocked main thread or uloop cancel) and the tool must not run; + // the caller decides by WasCancelledForBlockedMainThread / WasCancelledByUser, not by the token, + // which the claimer may not have cancelled yet. + internal bool MarkToolEntered() + { + return Interlocked.CompareExchange(ref _entryState, EntryStateEntered, EntryStateWaiting) == EntryStateWaiting; + } + + // The decision half of TryCancelForBlockedMainThread: records the stall and claims the lease. + // Returns false when the tool already entered or uloop cancel already claimed the lease. + // Split from the cancel so the moment between the claim and the cancel exists as a unit. + internal bool TryClaimBlockedMainThread(double stallSeconds) + { + Debug.Assert(stallSeconds >= 0, "stallSeconds must not be negative"); + + // Written before the CAS: the CAS is a full fence, so whoever sees the claim sees the value. + Volatile.Write(ref _blockedMainThreadStallSeconds, stallSeconds); + return Interlocked.CompareExchange(ref _entryState, EntryStateBlockedMainThread, EntryStateWaiting) == EntryStateWaiting; + } + + // Runs other requests' continuations synchronously on the calling thread (same note as + // CancelByUser), so never call it under the session lock or inside an activity hold. + internal bool TryCancelForBlockedMainThread(double stallSeconds) + { + if (!TryClaimBlockedMainThread(stallSeconds)) + { + return false; + } + _explicitCancellation.Cancel(); + return true; } public void Dispose() diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcErrorTypes.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcErrorTypes.cs index 37baeefccb..9ea00abd58 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcErrorTypes.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcErrorTypes.cs @@ -10,5 +10,6 @@ public static class JsonRpcErrorTypes public const string CliUpdateRequired = "cli_update_required"; public const string ServerBusy = "server_busy"; public const string ToolCancelled = "tool_cancelled"; + public const string MainThreadBlocked = "main_thread_blocked"; } } diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs index bf6d6a0e5c..77a951d6ca 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs @@ -201,8 +201,11 @@ await earlyResponseWriter( private static void LogRpcExceptionIfNeeded(Exception ex) { - // Why not log these: busy and uloop cancel are expected outcomes the CLI reports itself. - if (ex is UnityCliLoopToolBusyException || ex is UnityCliLoopToolCancelledException) + // Why not log these: busy, uloop cancel, and a blocked main thread are expected outcomes the + // CLI reports itself. + if (ex is UnityCliLoopToolBusyException + || ex is UnityCliLoopToolCancelledException + || ex is UnityCliLoopMainThreadBlockedException) { return; } diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs index 3b32203f35..8b4a573261 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs @@ -90,6 +90,13 @@ internal static string CreateErrorResponse(object id, Exception ex) cancelledEx.ToolName, exceptionResponse.Explanation ?? ex.Message); } + else if (ex is UnityCliLoopMainThreadBlockedException blockedEx) + { + errorData = new MainThreadBlockedErrorData( + blockedEx.ToolName, + blockedEx.StallSeconds, + exceptionResponse.Explanation ?? ex.Message); + } else { errorData = new InternalErrorData(exceptionResponse.Explanation ?? ex.Message); diff --git a/Packages/src/Editor/Infrastructure/Api/MainThreadBlockedErrorData.cs b/Packages/src/Editor/Infrastructure/Api/MainThreadBlockedErrorData.cs new file mode 100644 index 0000000000..f37be79cbb --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Api/MainThreadBlockedErrorData.cs @@ -0,0 +1,22 @@ +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Tells the CLI that its command stopped waiting for a blocked Editor main thread before any of + /// the tool's code ran, so the CLI can report a safe-to-retry failure instead of a silent wait. + /// + public class MainThreadBlockedErrorData : JsonRpcErrorData + { + public override string type => JsonRpcErrorTypes.MainThreadBlocked; + + public string runningToolName { get; } + + public double stallSeconds { get; } + + public MainThreadBlockedErrorData(string runningToolName, double stallSeconds, string message) + : base(message) + { + this.runningToolName = runningToolName; + this.stallSeconds = stallSeconds; + } + } +} diff --git a/Packages/src/Editor/Infrastructure/Api/MainThreadBlockedErrorData.cs.meta b/Packages/src/Editor/Infrastructure/Api/MainThreadBlockedErrorData.cs.meta new file mode 100644 index 0000000000..2a456fb7c3 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Api/MainThreadBlockedErrorData.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 9a76a10f5959544fa9464102392c215f +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/Threading/EditorMainThreadLivenessPort.cs b/Packages/src/Editor/Infrastructure/Threading/EditorMainThreadLivenessPort.cs new file mode 100644 index 0000000000..9c80a38d90 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Threading/EditorMainThreadLivenessPort.cs @@ -0,0 +1,15 @@ +using io.github.hatayama.UnityCliLoop.Application; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Exposes the Editor main-thread tick tracker to Application code through its port. + /// + internal sealed class EditorMainThreadLivenessPort : IMainThreadLivenessPort + { + public double SecondsSinceLastMainThreadTick() + { + return EditorMainThreadLivenessTracker.SecondsSinceLastMainThreadTick(); + } + } +} diff --git a/Packages/src/Editor/Infrastructure/Threading/EditorMainThreadLivenessPort.cs.meta b/Packages/src/Editor/Infrastructure/Threading/EditorMainThreadLivenessPort.cs.meta new file mode 100644 index 0000000000..5a2067784c --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Threading/EditorMainThreadLivenessPort.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 11256e26bd1164aa99e67b49af4af465 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From 97ec24e70953a0493a33b26726390f44f4c121ab Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 29 Oct 2026 16:23:21 +0900 Subject: [PATCH 2/5] Report MAIN_THREAD_BLOCKED in the CLI and document the fail-fast Unity now answers a command that stopped waiting for a blocked main thread with a main_thread_blocked error. Without a matching case the CLI showed it as a generic UNITY_RPC_ERROR, hiding that nothing ran and that running the command again is safe. The CLI maps the type to MAIN_THREAD_BLOCKED (retryable, safe to retry) with the stall seconds and tool name in Details, and points to the dialog check and uloop status instead of a retry loop. The status skill and the cancel reference say when the fail-fast applies, and ADR 0020 records the 30-second threshold and what it does not cover. --- .agents/skills/uloop-status/SKILL.md | 1 + .../skills/uloop-status/references/cancel.md | 1 + .claude/skills/uloop-status/SKILL.md | 1 + .../skills/uloop-status/references/cancel.md | 1 + .../CliOnlyTools~/Status/Skill/SKILL.md | 1 + .../Status/Skill/references/cancel.md | 1 + cli/common/errors/error_envelope.go | 1 + .../errors/error_envelope_classification.go | 2 + cli/common/errors/error_envelope_test.go | 74 +++++++++++++++++++ .../errors/main_thread_blocked_error.go | 54 ++++++++++++++ cli/common/errors/rpc_error_data.go | 15 ++++ cli/dispatcher/shared-inputs-stamp.json | 2 +- cli/project-runner/shared-inputs-stamp.json | 2 +- ...d-main-thread-fails-the-waiting-command.md | 62 ++++++++++++++++ 14 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 cli/common/errors/main_thread_blocked_error.go create mode 100644 docs/adr/0020-blocked-main-thread-fails-the-waiting-command.md diff --git a/.agents/skills/uloop-status/SKILL.md b/.agents/skills/uloop-status/SKILL.md index 360a337bc7..0c754c954b 100644 --- a/.agents/skills/uloop-status/SKILL.md +++ b/.agents/skills/uloop-status/SKILL.md @@ -57,3 +57,4 @@ Play Mode does not change the state: a paused Play Mode session is `Ready`, with - A sandboxed shell that cannot open the Unix socket gets an error, not NotRunning; run it outside the sandbox. - uloop cannot detect Safe Mode directly; ServerUnavailable that does not clear is the signal. - Before reporting MainThreadBlocked it wakes the Editor's main loop and asks again about a second later, so an Editor that is only idle in the background reads as Ready. +- A command sent while the main thread is blocked waits for it. Once the stall reaches 30 seconds before any of the command's code has run, Unity stops the wait and the command fails with `MAIN_THREAD_BLOCKED` (nothing ran, so running it again after the block clears is safe). A command whose own code blocks the main thread is not stopped; the CLI prints a stall notice on stderr every 60 seconds instead and gives up after 5 minutes with `UNITY_EDITOR_UNRESPONSIVE`. diff --git a/.agents/skills/uloop-status/references/cancel.md b/.agents/skills/uloop-status/references/cancel.md index e21d0d0717..a8987797e5 100644 --- a/.agents/skills/uloop-status/references/cancel.md +++ b/.agents/skills/uloop-status/references/cancel.md @@ -5,6 +5,7 @@ Cancel the uloop command that holds the Editor's command slot, from another term ## When to Use - Every command is rejected with `UNITY_SERVER_BUSY`, `uloop status` keeps reporting `Busy`, and the running command shows no sign of finishing. + `MAIN_THREAD_BLOCKED` means the command never ran and the slot is already free; `uloop cancel` is not needed. - A command you started does not return, for example an `execute-dynamic-code` snippet that awaits something that never completes. ## Usage diff --git a/.claude/skills/uloop-status/SKILL.md b/.claude/skills/uloop-status/SKILL.md index 360a337bc7..0c754c954b 100644 --- a/.claude/skills/uloop-status/SKILL.md +++ b/.claude/skills/uloop-status/SKILL.md @@ -57,3 +57,4 @@ Play Mode does not change the state: a paused Play Mode session is `Ready`, with - A sandboxed shell that cannot open the Unix socket gets an error, not NotRunning; run it outside the sandbox. - uloop cannot detect Safe Mode directly; ServerUnavailable that does not clear is the signal. - Before reporting MainThreadBlocked it wakes the Editor's main loop and asks again about a second later, so an Editor that is only idle in the background reads as Ready. +- A command sent while the main thread is blocked waits for it. Once the stall reaches 30 seconds before any of the command's code has run, Unity stops the wait and the command fails with `MAIN_THREAD_BLOCKED` (nothing ran, so running it again after the block clears is safe). A command whose own code blocks the main thread is not stopped; the CLI prints a stall notice on stderr every 60 seconds instead and gives up after 5 minutes with `UNITY_EDITOR_UNRESPONSIVE`. diff --git a/.claude/skills/uloop-status/references/cancel.md b/.claude/skills/uloop-status/references/cancel.md index e21d0d0717..a8987797e5 100644 --- a/.claude/skills/uloop-status/references/cancel.md +++ b/.claude/skills/uloop-status/references/cancel.md @@ -5,6 +5,7 @@ Cancel the uloop command that holds the Editor's command slot, from another term ## When to Use - Every command is rejected with `UNITY_SERVER_BUSY`, `uloop status` keeps reporting `Busy`, and the running command shows no sign of finishing. + `MAIN_THREAD_BLOCKED` means the command never ran and the slot is already free; `uloop cancel` is not needed. - A command you started does not return, for example an `execute-dynamic-code` snippet that awaits something that never completes. ## Usage diff --git a/Packages/src/Editor/CliOnlyTools~/Status/Skill/SKILL.md b/Packages/src/Editor/CliOnlyTools~/Status/Skill/SKILL.md index 360a337bc7..0c754c954b 100644 --- a/Packages/src/Editor/CliOnlyTools~/Status/Skill/SKILL.md +++ b/Packages/src/Editor/CliOnlyTools~/Status/Skill/SKILL.md @@ -57,3 +57,4 @@ Play Mode does not change the state: a paused Play Mode session is `Ready`, with - A sandboxed shell that cannot open the Unix socket gets an error, not NotRunning; run it outside the sandbox. - uloop cannot detect Safe Mode directly; ServerUnavailable that does not clear is the signal. - Before reporting MainThreadBlocked it wakes the Editor's main loop and asks again about a second later, so an Editor that is only idle in the background reads as Ready. +- A command sent while the main thread is blocked waits for it. Once the stall reaches 30 seconds before any of the command's code has run, Unity stops the wait and the command fails with `MAIN_THREAD_BLOCKED` (nothing ran, so running it again after the block clears is safe). A command whose own code blocks the main thread is not stopped; the CLI prints a stall notice on stderr every 60 seconds instead and gives up after 5 minutes with `UNITY_EDITOR_UNRESPONSIVE`. diff --git a/Packages/src/Editor/CliOnlyTools~/Status/Skill/references/cancel.md b/Packages/src/Editor/CliOnlyTools~/Status/Skill/references/cancel.md index e21d0d0717..a8987797e5 100644 --- a/Packages/src/Editor/CliOnlyTools~/Status/Skill/references/cancel.md +++ b/Packages/src/Editor/CliOnlyTools~/Status/Skill/references/cancel.md @@ -5,6 +5,7 @@ Cancel the uloop command that holds the Editor's command slot, from another term ## When to Use - Every command is rejected with `UNITY_SERVER_BUSY`, `uloop status` keeps reporting `Busy`, and the running command shows no sign of finishing. + `MAIN_THREAD_BLOCKED` means the command never ran and the slot is already free; `uloop cancel` is not needed. - A command you started does not return, for example an `execute-dynamic-code` snippet that awaits something that never completes. ## Usage diff --git a/cli/common/errors/error_envelope.go b/cli/common/errors/error_envelope.go index 25904bf206..f6767708d8 100644 --- a/cli/common/errors/error_envelope.go +++ b/cli/common/errors/error_envelope.go @@ -24,6 +24,7 @@ const ( errorCodeUnityRPCError = "UNITY_RPC_ERROR" errorCodeUnityServerBusy = "UNITY_SERVER_BUSY" errorCodeToolCancelled = "TOOL_CANCELLED" + errorCodeMainThreadBlocked = "MAIN_THREAD_BLOCKED" ErrorCodeCLIUpdateRequired = "CLI_UPDATE_REQUIRED" ErrorCodeV2ProjectDetected = "V2_PROJECT_DETECTED" ErrorCodeToolDisabled = "TOOL_DISABLED" diff --git a/cli/common/errors/error_envelope_classification.go b/cli/common/errors/error_envelope_classification.go index 2c56309d5a..1ffeb79220 100644 --- a/cli/common/errors/error_envelope_classification.go +++ b/cli/common/errors/error_envelope_classification.go @@ -145,6 +145,8 @@ func classifyRPCError(rpcErr *unityipc.RPCError, context ErrorContext) CLIError return unityServerBusyError(rpcErr, details, decodeServerBusyErrorData(rpcErr.Data), context) case "tool_cancelled": return toolCancelledError(rpcErr, details, decodeToolCancelledErrorData(rpcErr.Data), context) + case "main_thread_blocked": + return mainThreadBlockedError(rpcErr, details, decodeMainThreadBlockedErrorData(rpcErr.Data), context) default: return genericRPCError(rpcErr, details, context) } diff --git a/cli/common/errors/error_envelope_test.go b/cli/common/errors/error_envelope_test.go index 94269c083b..5aa3ecde45 100644 --- a/cli/common/errors/error_envelope_test.go +++ b/cli/common/errors/error_envelope_test.go @@ -650,3 +650,77 @@ func TestDisconnectedAfterAcceptKeepsNextActionsForOtherCommands(t *testing.T) { t.Fatalf("compile should keep the fixed next actions: %#v", envelope.NextActions) } } + +func TestClassifyMainThreadBlockedRPCError(t *testing.T) { + // Verifies main_thread_blocked RPC failures become a safe-to-retry MAIN_THREAD_BLOCKED error with the stall seconds and recovery guidance. + err := &unityipc.RPCError{ + Code: -32603, + Message: "'get-hierarchy' was not run: the Unity Editor main thread has not run for 45s, so uloop stopped waiting and released the command slot.", + Data: json.RawMessage( + `{"type":"main_thread_blocked","runningToolName":"get-hierarchy","stallSeconds":45,"message":"'get-hierarchy' was not run."}`), + } + + cliErr := ClassifyError(err, ErrorContext{ProjectRoot: "/tmp/MyProject", Command: "get-hierarchy"}) + if cliErr.ErrorCode != "MAIN_THREAD_BLOCKED" { + t.Fatalf("error code mismatch: %#v", cliErr) + } + if cliErr.Phase != ErrorPhaseResponseWaiting { + t.Fatalf("phase mismatch: %#v", cliErr) + } + if !cliErr.Retryable || !cliErr.SafeToRetry { + t.Fatalf("retry flags mismatch: %#v", cliErr) + } + if !strings.HasPrefix(cliErr.Message, "'get-hierarchy' was not run: Unity's main thread has not run for 45s") { + t.Fatalf("message mismatch: %s", cliErr.Message) + } + if !containsSubstring(cliErr.NextActions, "`uloop status`") { + t.Fatalf("next actions do not mention uloop status: %#v", cliErr.NextActions) + } + if cliErr.Details["StallSeconds"] != 45.0 { + t.Fatalf("stall seconds mismatch: %#v", cliErr.Details) + } + if cliErr.Details["RunningToolName"] != "get-hierarchy" { + t.Fatalf("running tool name mismatch: %#v", cliErr.Details) + } +} + +func TestClassifyMainThreadBlockedRPCError_WhenStallMissing_SaysThirtySecondsOrMore(t *testing.T) { + // Verifies a payload without stallSeconds still names the threshold and leaves StallSeconds out of Details. + err := &unityipc.RPCError{ + Code: -32603, + Message: "blocked", + Data: json.RawMessage(`{"type":"main_thread_blocked","runningToolName":"get-hierarchy"}`), + } + + cliErr := ClassifyError(err, ErrorContext{ProjectRoot: "/tmp/MyProject", Command: "get-hierarchy"}) + if !strings.Contains(cliErr.Message, "30s or more") { + t.Fatalf("message mismatch: %s", cliErr.Message) + } + if _, ok := cliErr.Details["StallSeconds"]; ok { + t.Fatalf("StallSeconds should be absent: %#v", cliErr.Details) + } +} + +func TestClassifyMainThreadBlockedRPCError_WhenToolNameMissing_UsesCommand(t *testing.T) { + // Verifies a payload without runningToolName names the command the CLI sent. + err := &unityipc.RPCError{ + Code: -32603, + Message: "blocked", + Data: json.RawMessage(`{"type":"main_thread_blocked","stallSeconds":31}`), + } + + cliErr := ClassifyError(err, ErrorContext{ProjectRoot: "/tmp/MyProject", Command: "get-logs"}) + if !strings.HasPrefix(cliErr.Message, "'get-logs' was not run") { + t.Fatalf("message mismatch: %s", cliErr.Message) + } +} + +// Reports whether any of the lines contains the substring. +func containsSubstring(lines []string, substring string) bool { + for _, line := range lines { + if strings.Contains(line, substring) { + return true + } + } + return false +} diff --git a/cli/common/errors/main_thread_blocked_error.go b/cli/common/errors/main_thread_blocked_error.go new file mode 100644 index 0000000000..b808532737 --- /dev/null +++ b/cli/common/errors/main_thread_blocked_error.go @@ -0,0 +1,54 @@ +package clierrors + +import ( + "fmt" + "strconv" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// mainThreadBlockedError reports that Unity stopped waiting for its blocked main thread before +// the tool's code ran. Why safe to retry: the tool never started, so nothing happened twice. +func mainThreadBlockedError( + rpcErr *unityipc.RPCError, + details map[string]any, + data mainThreadBlockedErrorData, + context ErrorContext, +) CLIError { + toolName := firstNonEmpty(data.RunningToolName, context.Command) + message := fmt.Sprintf( + "'%s' was not run: Unity's main thread has not run for %s, so uloop stopped waiting and released the command slot. A modal dialog, a long import, or running code is blocking it.", + toolName, + stallText(data.StallSeconds)) + details["RunningToolName"] = toolName + if data.StallSeconds != nil { + details["StallSeconds"] = *data.StallSeconds + } + + return CLIError{ + ErrorCode: errorCodeMainThreadBlocked, + Phase: ErrorPhaseResponseWaiting, + Message: message, + Retryable: true, + SafeToRetry: true, + ProjectRoot: context.ProjectRoot, + Command: context.Command, + NextActions: []string{ + "Check the Unity window for a modal dialog or a long operation that is blocking the Editor main thread.", + ApiUpdateConsentModalNextAction, + "Run `uloop focus-window` if Unity is hidden behind another window.", + "Run `uloop status`; when it reports Ready, run the command once more. Do not retry in a loop while it reports MainThreadBlocked.", + "If no dialog is shown and the Editor stays blocked, restart it with `uloop launch -r`.", + }, + Details: details, + } +} + +// stallText names the stall Unity reported, or the threshold it stops at when an older payload +// carried no value. +func stallText(stallSeconds *float64) string { + if stallSeconds == nil { + return "30s or more" + } + return strconv.FormatFloat(*stallSeconds, 'f', 0, 64) + "s" +} diff --git a/cli/common/errors/rpc_error_data.go b/cli/common/errors/rpc_error_data.go index 124a926f15..a889d23af9 100644 --- a/cli/common/errors/rpc_error_data.go +++ b/cli/common/errors/rpc_error_data.go @@ -35,6 +35,21 @@ type toolCancelledErrorData struct { RunningToolName string `json:"runningToolName"` } +// Mirrors Packages/src/Editor/Infrastructure/Api/MainThreadBlockedErrorData.cs public properties. +type mainThreadBlockedErrorData struct { + Type string `json:"type"` + Message string `json:"message"` + RunningToolName string `json:"runningToolName"` + StallSeconds *float64 `json:"stallSeconds"` +} + +func decodeMainThreadBlockedErrorData(data json.RawMessage) mainThreadBlockedErrorData { + var decoded mainThreadBlockedErrorData + // Why ignore: keep the previous missing-key = zero-value fallback. + _ = json.Unmarshal(data, &decoded) + return decoded +} + func decodeToolCancelledErrorData(data json.RawMessage) toolCancelledErrorData { var decoded toolCancelledErrorData // Why ignore: keep the previous missing-key = zero-value fallback. diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index b040c259e1..e853a304fe 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "292bd6079496d0865f852146f0a3829d8492dfd0" + "sharedInputsHash": "691e61712e55bcec7bf2fa6f1d24a5a5ecd57769" } diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index be37531b5e..6a008bbef6 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "1db5b8eb574726147b628d2832ceac7a691426ba" + "sharedInputsHash": "0733801075f46926118a89fc02d6ca5fb7513794" } diff --git a/docs/adr/0020-blocked-main-thread-fails-the-waiting-command.md b/docs/adr/0020-blocked-main-thread-fails-the-waiting-command.md new file mode 100644 index 0000000000..bd1068c928 --- /dev/null +++ b/docs/adr/0020-blocked-main-thread-fails-the-waiting-command.md @@ -0,0 +1,62 @@ +# A Blocked Main Thread Fails the Waiting Command + +Date: 2026-10-29 + +## Decision + +A command that holds the command slot but has not entered its tool code yet (it is still waiting +for its first switch to the Editor main thread) is failed by Unity once the main thread has stalled +for 30 seconds. Unity cancels the wait through the same lease cancellation as `uloop cancel`, +releases the slot, and answers `main_thread_blocked` with the tool name and the stall seconds; the +CLI reports it as `MAIN_THREAD_BLOCKED`, safe to retry, because no tool code ran. + +- The stall is the time since the main thread last ticked, not how long the command waited. +- The first check happens one poll (1 second) after the wait starts, then once a second. An idle + Editor's loop sleeps and its stall counter keeps growing; the switch request wakes it, and one + poll interval is enough for it to tick. +- A command whose tool code has started is never stopped this way. Its stall stays visible through + the CLI's 30-second notice (on stderr every 60 seconds when no spinner is shown) and the CLI gives + up after 5 minutes with `UNITY_EDITOR_UNRESPONSIVE`, as before. +- Requests `uloop cancel` may not stop (a compile with a domain reload, run-tests that respects the + Enter Play Mode settings) are failed too: before the tool code runs there is nothing to break. +- 30 seconds is the same "longest ordinary busy" limit the CLI already uses for its stall notice and + for compile's silent-wait warning. + +## Context + +Issue #3411: with a modal dialog holding the main thread, `uloop status` answered +`MainThreadBlocked` within seconds and a second command got `UNITY_SERVER_BUSY` saying the holder +was waiting for a main thread that had not responded for N seconds, but the command holding the slot +waited silently. In the report's 60-second block it returned success after 52 seconds with nothing +printed in between; without anyone dismissing the dialog it would have waited for the CLI's +5-minute limit. The stall was detected, just never used for the waiting command itself. + +## Considered Options + +- Give up on the CLI side: rejected. A request sent with `cancelOnClientDisconnect=false` keeps + running in Unity after the CLI leaves, so the command could still run later, after the agent has + moved on, and the slot would stay taken in the meantime. +- 5 seconds (the BUSY report threshold): rejected. Scene saves, small imports, and entering Play + Mode routinely exceed it, so ordinary commands would fail and be sent again. +- 15 seconds: rejected for the same reason at a lower rate, and it matches no other limit the user + already meets. +- 60 seconds: rejected. A dialog would be reported a minute late, which is longer than the wrappers + agents put around commands (about 30 seconds in the report). + +## Consequences + +- A command sent during a synchronous main-thread operation longer than 30 seconds (a large import) + fails once and has to be run again; `SafeToRetry` says so. +- When an `execute-dynamic-code` snippet blocks the main thread for 30 seconds or more, another + `execute-dynamic-code` waiting behind it in the shared slot fails with `MAIN_THREAD_BLOCKED`. + Before, it ran after the first snippet finished. +- The error is an additive `type`; older CLIs show it as `UNITY_RPC_ERROR`, so the protocol version + does not change. + +## Revisit When + +- (a) In real use, main-thread work that is not a dialog and lasts over 30 seconds causes + `MAIN_THREAD_BLOCKED` several times a day: raise the threshold. +- (b) The main complaint is that dialogs are detected too late: lower it. + +Both are the single constant `MainThreadEntryWatchdog.BlockedStallSeconds`. From 2670f2f71ac194070e012dec32f23566c42e443a Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 29 Oct 2026 16:33:05 +0900 Subject: [PATCH 3/5] Raise the dead-code public candidate ceiling for the new error data type MainThreadBlockedErrorData.type is the wire discriminator the CLI reads as error.data.type, like the type override on every other JsonRpcErrorData subclass, so it stays a PublicCandidate under the documented group. --- .github/workflows/dead-code.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/dead-code.yml b/.github/workflows/dead-code.yml index 3c514ad03b..01e88d9537 100644 --- a/.github/workflows/dead-code.yml +++ b/.github/workflows/dead-code.yml @@ -49,4 +49,4 @@ jobs: --include-kept false --format table --fail-on high-confidence - --max-public-candidates 38 + --max-public-candidates 39 From b0b12e47009b4dea1220b3132c5f61d2b669b25d Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 29 Oct 2026 16:59:13 +0900 Subject: [PATCH 4/5] Check the task finished before asserting its exception in the watchdog tests Assert.ThrowsAsync waits on the task, so a regression that leaves a blocked or cancelled request pending would freeze the Editor instead of failing the test. Each test now asserts the task is faulted first. --- .../Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs b/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs index 09f018cb88..530b2a0c8d 100644 --- a/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs @@ -609,6 +609,8 @@ public void ExecuteToolAsync_WhenWatchdogClaimsTheLeaseBeforeTheMainThreadResume dispatcher.RunQueued(); + // Checked first: ThrowsAsync waits on a pending task and would freeze the Editor. + Assert.That(task.IsFaulted, Is.True); UnityCliLoopMainThreadBlockedException exception = Assert.ThrowsAsync(() => task); Assert.That(exception.StallSeconds, Is.EqualTo(45)); @@ -675,6 +677,8 @@ public void ExecuteToolAsync_WhenUserCancelsBeforeTheWatchdog_ThrowsToolCancelle executionService.CancelRunningTool(); + // Checked first: ThrowsAsync waits on a pending task and would freeze the Editor. + Assert.That(task.IsFaulted, Is.True); Assert.ThrowsAsync(() => task); Assert.That(polls.Latest.IsCanceled, Is.True); }); @@ -709,6 +713,8 @@ public void ExecuteToolAsync_WhenASharedSlotHolderBlocksTheMainThread_FailsTheWa CancellationToken.None); polls.CompleteLatest(); + // Checked first: ThrowsAsync waits on a pending task and would freeze the Editor. + Assert.That(sharerTask.IsFaulted, Is.True); Assert.ThrowsAsync(() => sharerTask); Assert.That(holderTask.IsCompleted, Is.False); @@ -740,6 +746,8 @@ private static void AssertBlockedBeforeToolEntryThrowsMainThreadBlocked(bool can polls.CompleteLatest(); + // Checked first: ThrowsAsync waits on a pending task and would freeze the Editor. + Assert.That(task.IsFaulted, Is.True); UnityCliLoopMainThreadBlockedException exception = Assert.ThrowsAsync(() => task); Assert.That(exception.ToolName, Is.EqualTo(ImmediateTool.HolderName)); From 4915a040795d07feb62d3761e0ff604bfaaaaa7c Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 29 Oct 2026 17:21:38 +0900 Subject: [PATCH 5/5] Say MAIN_THREAD_BLOCKED frees only the failed command's hold on the slot An execute-dynamic-code run sharing the slot can still hold it, so status may stay Busy; the cancel reference claimed the slot was free. --- .agents/skills/uloop-status/references/cancel.md | 2 +- .claude/skills/uloop-status/references/cancel.md | 2 +- .../src/Editor/CliOnlyTools~/Status/Skill/references/cancel.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.agents/skills/uloop-status/references/cancel.md b/.agents/skills/uloop-status/references/cancel.md index a8987797e5..d14a4ddb5f 100644 --- a/.agents/skills/uloop-status/references/cancel.md +++ b/.agents/skills/uloop-status/references/cancel.md @@ -5,7 +5,7 @@ Cancel the uloop command that holds the Editor's command slot, from another term ## When to Use - Every command is rejected with `UNITY_SERVER_BUSY`, `uloop status` keeps reporting `Busy`, and the running command shows no sign of finishing. - `MAIN_THREAD_BLOCKED` means the command never ran and the slot is already free; `uloop cancel` is not needed. + `MAIN_THREAD_BLOCKED` means the command never ran and released its hold on the slot, so `uloop cancel` is not needed for it (an `execute-dynamic-code` run sharing the slot may still hold it). - A command you started does not return, for example an `execute-dynamic-code` snippet that awaits something that never completes. ## Usage diff --git a/.claude/skills/uloop-status/references/cancel.md b/.claude/skills/uloop-status/references/cancel.md index a8987797e5..d14a4ddb5f 100644 --- a/.claude/skills/uloop-status/references/cancel.md +++ b/.claude/skills/uloop-status/references/cancel.md @@ -5,7 +5,7 @@ Cancel the uloop command that holds the Editor's command slot, from another term ## When to Use - Every command is rejected with `UNITY_SERVER_BUSY`, `uloop status` keeps reporting `Busy`, and the running command shows no sign of finishing. - `MAIN_THREAD_BLOCKED` means the command never ran and the slot is already free; `uloop cancel` is not needed. + `MAIN_THREAD_BLOCKED` means the command never ran and released its hold on the slot, so `uloop cancel` is not needed for it (an `execute-dynamic-code` run sharing the slot may still hold it). - A command you started does not return, for example an `execute-dynamic-code` snippet that awaits something that never completes. ## Usage diff --git a/Packages/src/Editor/CliOnlyTools~/Status/Skill/references/cancel.md b/Packages/src/Editor/CliOnlyTools~/Status/Skill/references/cancel.md index a8987797e5..d14a4ddb5f 100644 --- a/Packages/src/Editor/CliOnlyTools~/Status/Skill/references/cancel.md +++ b/Packages/src/Editor/CliOnlyTools~/Status/Skill/references/cancel.md @@ -5,7 +5,7 @@ Cancel the uloop command that holds the Editor's command slot, from another term ## When to Use - Every command is rejected with `UNITY_SERVER_BUSY`, `uloop status` keeps reporting `Busy`, and the running command shows no sign of finishing. - `MAIN_THREAD_BLOCKED` means the command never ran and the slot is already free; `uloop cancel` is not needed. + `MAIN_THREAD_BLOCKED` means the command never ran and released its hold on the slot, so `uloop cancel` is not needed for it (an `execute-dynamic-code` run sharing the slot may still hold it). - A command you started does not return, for example an `execute-dynamic-code` snippet that awaits something that never completes. ## Usage