fix: Unity tools no longer hang while waiting for editor frames#1333
Merged
Conversation
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.
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.
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
Contributor
|
Warning Review limit reached
More reviews will be available in 20 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughReplaces ChangesTimeout-aware frame waiting and thread-safe tool execution
Sequence Diagram(s)sequenceDiagram
participant Tool as ScreenshotTool
participant UseCase as ScreenshotUseCase
participant EditorFrameWaiter
participant Utility as EditorWindowCaptureUtility
participant EditorContext as Editor SynchronizationContext
Tool->>UseCase: CaptureAsync(request).ConfigureAwait(false)
UseCase->>UseCase: capture SynchronizationContext
UseCase->>EditorFrameWaiter: WaitFramesOrTimeoutAsync(2, 5000ms, ct)
alt frames observed within timeout
EditorFrameWaiter-->>UseCase: true
UseCase->>EditorContext: SwitchToMainThread
UseCase->>Utility: CaptureWindowAsync(window, scale, 5000, ct)
Utility-->>UseCase: (texture, timedOut=false)
UseCase-->>Tool: Screenshots result
else timeout elapsed
EditorFrameWaiter-->>UseCase: false
UseCase->>EditorContext: Post(DestroyAnnotationOverlay)
UseCase-->>Tool: TimedOut=true, Message, partial Screenshots
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.cs`:
- Around line 51-54: The ShouldRequestWait method call is accessing
EditorApplication.isCompiling before switching to the main thread, which is
unsafe when the method can be entered off-thread. Move the
MainThreadSwitcher.SwitchToMainThread call to occur before the ShouldRequestWait
invocation (which includes the EditorApplication.isCompiling access) to ensure
main thread safety before any UnityEditor API access.
In
`@Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs`:
- Around line 83-86: The ExecuteAsync method has await expressions at lines 50,
51, and 53 (WarmForegroundExecutionPathIfNeededAsync and related awaits) that
lack ConfigureAwait(false), while ConfigureAwait(false) is only added later at
line 85 for ShouldWaitAsync. To ensure consistent timeout-safe behavior and
prevent capturing the editor's stalled SynchronizationContext throughout the
entire method execution, add ConfigureAwait(false) to all await expressions in
the ExecuteAsync method, starting with the early awaits at lines 50, 51, and 53,
and continuing through any other awaits in the method's execution flow.
In
`@Packages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.cs`:
- Around line 252-295: The delayed-start callbacks across
RecordingsApplicationFacade.cs (methods StartDelayedRecordingAsync,
CleanupCountdownOnMainThreadAsync, and StartRecordingAfterCountdown) and
RecordInputUseCase.cs (the delayed-start callback and cleanup path around lines
235-293) use generation checks to gate state mutations, but stale generations
can still execute cleanup or recording logic on the current overlay state. For
RecordingsApplicationFacade.cs, add an early-return guard in
CleanupCountdownOnMainThreadAsync so that if the generation is stale (not equal
to _countdownGeneration), the method returns immediately without clearing
overlay state; also ensure StartRecordingAfterCountdown checks generation before
starting recording. For RecordInputUseCase.cs, introduce an invocation-scoped
generation and cancellation token around the entire delayed-start callback and
cleanup path (similar to the facade's _countdownGeneration pattern) so that
earlier RecordInputAsync(Start) invocations cannot affect overlay state once a
newer countdown or recording path has taken control.
In `@Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs`:
- Around line 213-221: When the EditorWindow capture times out at the timedOut
check in ScreenshotUseCase.cs, the current code returns a fresh timeout result
via CreateTimedOutResult() which discards the screenshots already collected from
previous iterations. Instead, modify the timeout handling to preserve the
already-captured window metadata by including the screenshots list that has been
populated in earlier loop iterations in the result being returned, so the CLI
response accurately reflects the side effects (PNGs written to disk) even when a
later window hits the timeout.
In `@Packages/src/Editor/ToolContracts/TimerDelay.cs`:
- Around line 108-121: The timeoutTask created by the Wait method is never
canceled when completionSource.Task completes successfully, causing the
background timer to remain active for the full EDITOR_FRAME_WAIT_TIMEOUT_MS
duration. Add a cancel operation for timeoutTask on the non-timeout
path—specifically, when completedTask does not equal timeoutTask, cancel the
timeout wait so that the timer/callback is not left running unnecessarily. This
should be done before or after the final await of completionSource.Task,
mirroring the pattern used in EditorFrameWaiter.WaitFramesOrTimeoutAsync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ee8adaad-512f-4bc6-b656-3751297e470e
⛔ Files ignored due to path filters (4)
Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/UnityCLILoop.FirstPartyTools.ExecuteDynamicCode.Editor.asmdefis excluded by none and included by nonePackages/src/Editor/FirstPartyTools/RecordInput/UnityCLILoop.FirstPartyTools.RecordInput.Editor.asmdefis excluded by none and included by nonePackages/src/Editor/FirstPartyTools/Screenshot/UnityCLILoop.FirstPartyTools.Screenshot.Editor.asmdefis excluded by none and included by nonePackages/src/Editor/FirstPartyTools/SimulateMouseUi/UnityCLILoop.FirstPartyTools.SimulateMouseUi.Editor.asmdefis excluded by none and included by none
📒 Files selected for processing (26)
Assets/Editor/CaptureTest/EditorWindowCaptureTest.csAssets/Editor/EditorDelayManualTests.csAssets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeUseCaseTests.csAssets/Tests/Editor/StaticFacadeStateGuardTests.csAssets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.csPackages/src/Editor/Application/UnityCliLoopToolExecutionService.csPackages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemUpdateHelper.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeDomainReloadWaitSignal.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeTool.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.csPackages/src/Editor/FirstPartyTools/RecordInput/Application/RecordingsApplicationFacade.csPackages/src/Editor/FirstPartyTools/RecordInput/RecordInputTool.csPackages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.csPackages/src/Editor/FirstPartyTools/Screenshot/EditorWindowCaptureUtility.csPackages/src/Editor/FirstPartyTools/Screenshot/ScreenshotResponse.csPackages/src/Editor/FirstPartyTools/Screenshot/ScreenshotTool.csPackages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.csPackages/src/Editor/FirstPartyTools/Screenshot/UnityCliLoopScreenshotTypes.csPackages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.csPackages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiTool.csPackages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.csPackages/src/Editor/ToolContracts/EditorFrameWaiter.csPackages/src/Editor/ToolContracts/TimerDelay.csPackages/src/Editor/ToolContracts/UnityCliLoopConstants.csPackages/src/Editor/ToolContracts/UnityCliLoopTool.csPackages/src/Editor/ToolContracts/VibeLogger.cs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 30 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
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.
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.
Merged
This was referenced Jul 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
User Impact
Changes
Verification