Skip to content

fix: dynamic-code cancel and Editor shutdown no longer hang on stuck work#1753

Merged
hatayama merged 4 commits into
v3-betafrom
feature/dynamic-code-cancellation-hardening
Jul 13, 2026
Merged

fix: dynamic-code cancel and Editor shutdown no longer hang on stuck work#1753
hatayama merged 4 commits into
v3-betafrom
feature/dynamic-code-cancellation-hardening

Conversation

@hatayama

@hatayama hatayama commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Summary

  • Dynamic-code cancellation and Editor shutdown no longer hang on sync Execute wrappers, unbounded scheduler shutdown waits, or shared Roslyn worker ReadLine holding the session lock
  • Dead sync CommandRunner.Execute(ExecutionContext) entry point removed; raw sync Execute fallback via ExecuteInternal remains

User Impact

  • Before: Cancel/shutdown could stall on a sync Execute path, wait forever for stuck user code, or block domain reload/quit behind a stuck worker ReadLine (up to ~30s)
  • After: Cancellation reaches async wrappers; scheduler shutdown is bounded; worker conversations stay serial while shutdown can kill the process without waiting on the compile gate

Sub-PRs (already merged into this branch)

PR Change
#1744 Remove unused sync Execute() from WrapperTemplate so cancellation is not dropped
#1745 Delete dead CommandRunner.Execute(ExecutionContext) only
#1746 ShutdownAsync 5s timeout; timeout completes shutdown and logs a warning
#1747 Async Roslyn worker reads + SemaphoreSlim compile gate; shutdown bypasses gate

Follow-ups (not in this PR)

  • CommandRunner.Cancel() — pre-existing unused public candidate; leave unless requested
  • SharedRoslynCompilerWorkerAssemblyBuilder sync WaitForExit (may run on main thread during compile/prewarm)
  • Post-shutdown retry may briefly resurrect a worker; stdin EOF self-exits (no lasting leak)
  • Reset-crossing in-flight execute-dynamic-code can fail with ObjectDisposedException on a disposed scheduler; consider retryable "server restarting" mapping later

Verification

Test plan

  • CI Compile Check (Linux) green
  • Spot-check: cancel an in-flight execute-dynamic-code; Editor remains responsive
  • Spot-check: domain reload during/after dynamic-code work does not stick on worker ReadLine

Review in cubic

hatayama and others added 4 commits July 13, 2026 15:53
…ute path (#1744)

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…1747)

Compile request send stays inside the retryable IO try; shutdown bypasses the async compile gate.
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@hatayama, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 52512c90-744a-4ebc-8357-4924629e09df

📥 Commits

Reviewing files that changed from the base of the PR and between b348ad5 and cbe2662.

⛔ Files ignored due to path filters (4)
  • Assets/Tests/Editor/DynamicCodeToolTests/WrapperTemplateTests.cs.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerLineReader.cs.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSessionCoordination.cs.meta is excluded by none and included by none
  • tests/SharedRoslynCompilerWorker.UnitTests/SharedRoslynCompilerWorker.UnitTests.csproj is excluded by none and included by none
📒 Files selected for processing (16)
  • .github/workflows/unity-compile-check-and-test-runner.yml
  • .github/workflows/unity-editmode-tests.yml
  • Assets/Tests/Editor/DynamicCodeToolTests/WrapperTemplateTests.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerBackend.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerHost.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerLineReader.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerProtocol.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSession.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerSessionCoordination.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrapperTemplate.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionFacade.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionScheduler.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionSchedulerHooks.cs
  • tests/DynamicCodeExecutionScheduler.UnitTests/DynamicCodeExecutionSchedulerTests.cs
  • tests/SharedRoslynCompilerWorker.UnitTests/SharedRoslynCompilerWorkerLineReaderTests.cs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/dynamic-code-cancellation-hardening

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@hatayama

Copy link
Copy Markdown
Owner Author

LGTM (merge after required CI is green). Verified this umbrella contains exactly the four reviewed sub-PR squash merges (#17444506a7c, #1745343e478, #174626d48e9, #1747cbe2662) and the diff against v3-beta matches the sum of the reviewed sub-PR diffs — no extra changes. The follow-ups section correctly carries all four umbrella notes (CommandRunner.Cancel, AssemblyBuilder sync WaitForExit with the main-thread call-site note, post-shutdown worker resurrection, reset-crossing ObjectDisposedException race). Pure unit tests were re-verified locally at each sub-PR head (scheduler 9/9, SharedRoslynCompilerWorker 8/8).

@hatayama hatayama merged commit bc1233a into v3-beta Jul 13, 2026
10 checks passed
@hatayama hatayama deleted the feature/dynamic-code-cancellation-hardening branch July 13, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant