Fix sidebar-triggered main-actor terminal teardown hang - #9358
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesTerminal surface teardown now uses two bounded concurrent close-teardown slots. Native freeing and resource release remain coordinated by Terminal teardown
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TeardownTest
participant TerminalSurface
participant RuntimeTeardown
participant GhosttySurface
participant MainActor
TeardownTest->>GhosttySurface: Enable blocking for target surface
TeardownTest->>TerminalSurface: Call teardownSurface()
TerminalSurface->>RuntimeTeardown: Enqueue runtime teardown
RuntimeTeardown->>GhosttySurface: Run ghostty_surface_free(...)
TeardownTest->>MainActor: Run responsiveness probe
TeardownTest->>GhosttySurface: Release and reset blocking gate
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c">
<violation number="1" location="Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c:60">
P2: The condition waits in wait_until_started and the gated ghostty_surface_free have no timeout, so a regression that never routes the surface free to this stub (or never releases the gate) will hang the whole test process indefinitely rather than fail with a clear signal. Consider bounding both waits (e.g. pthread_cond_timedwait with a deadline) and returning an error/asserting so CI fails instead of stalling.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceTeardownCallbackLifetimeTests.swift`:
- Around line 8-19: Replace the four `@_silgen_name` declarations in
TerminalSurfaceTeardownCallbackLifetimeTests with a normal import of the
GhosttyRuntimeTestStubs module, then call the exposed C bindings directly.
Preserve the existing binding names and test behavior while removing the manual
Swift symbol wrappers.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: d9cbdb34-22f9-4527-85f3-5cb3c3d7aabe
📒 Files selected for processing (4)
Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+RuntimeLifecycle.swiftPackages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceTeardownCallbackLifetimeTests.swiftPackages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.cPackages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.h
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swift">
<violation number="1" location="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swift:167">
P2: The new `stuckCloseFreeDoesNotStrandLaterCloses` test asserts that later closes complete within one second while the first native free is gate-blocked, but the coordinator's default `.serializedClose` lane runs all requests through a single serialized worker. When the first request's `freeSurface` blocks, that worker is stuck and cannot dequeue the later tickets, so `await ticket.wait(timeout: .seconds(1))` returns false and the test's `#expect` fails (it also burn ~2s of wall time). The concurrency the test wants only exists on the isolated-hibernation lane. The test should either use the isolated-hibernation lane (mirroring `stuckHibernationFreeDoesNotStrandAnotherAdmissionOrClose`) or flip its expectations to assert that a stuck close in the serialized lane does strand later closes, otherwise this added test is red.</violation>
<violation number="2" location="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swift:173">
P3: Read freed pointers via `recorder.waitForFreeCount(laterTickets.count)` before the equality check, since the free closures record through spawned `Task { await recorder.record(bits) }` and ticket.wait can resolve before those Tasks run. The file's sibling test already uses this helper for the same reason; checking `recorder.freed` directly here risks a flaky partial-set comparison.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c">
<violation number="1" location="Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c:41">
P3: cmux_test_ghostty_runtime_stubs_reset() does not clear the new surface-free blocking state, so a test that tears down without running blocking_reset (e.g. a test failure before the defer, or a future test that forgets it) leaves should_block=true and makes unrelated tests in the same process block 5s per free. Consider folding the blocking-state reset into the general reset, or having ghostty_surface_free auto-clear the flag after it unblocks, to keep failure isolation for the rest of the suite.</violation>
</file>
<file name="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceTeardownCallbackLifetimeTests.swift">
<violation number="1" location="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceTeardownCallbackLifetimeTests.swift:41">
P2: This test will fail every run: the ghostty test stub's free gate is global (cmux_test_surface_free_should_block), so ghostty_surface_free(0x7542) gets blocked for the 5s condvar deadline and the `< .seconds(1)` assertion fails, adding a 5s stall to the serialized @MainActor suite. The 'does not intercept another surface' premise only holds if the gate is made surface-aware (gate the specific teardown surface), otherwise the assertion can never pass. Please make blocking_start take the expected surface and only block ghostty_surface_free for that pointer, then this test validates the intended behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/macOS/CmuxTerminal/Sources/CmuxTerminal/Lifecycle/TerminalSurfaceRuntimeTeardownCoordinator.swift`:
- Around line 227-261: Update the bounded close-teardown flow around
startAvailableCloseTeardowns and finishCloseTeardown to expose when
queuedCloseRequests grows beyond a configured threshold, such as by reporting
the queue depth through the existing logging or telemetry mechanism. Preserve
the current slot behavior and main-actor responsiveness; make systemic
native-free hangs observable rather than silently leaving later requests queued
indefinitely.
In
`@Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceTeardownCallbackLifetimeTests.swift`:
- Around line 31-48: Replace the ContinuousClock elapsed-time assertion in
surfaceFreeGateDoesNotInterceptAnotherRuntimeSurface with a completion signal
and deadline-bounded wait, following the existing pattern in
teardownSurfaceKeepsMainActorResponsiveWhileNativeFreeIsBlocked. Signal
completion after ghostty_surface_free(unrelatedSurface) returns, then assert the
signal completes before the deadline while preserving the test’s cleanup
behavior.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 8f28fce9-3b4b-4cf5-b3af-43d07339aaa7
📒 Files selected for processing (8)
Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Lifecycle/TerminalSurfaceRuntimeTeardownCoordinator.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Lifecycle/TerminalSurfaceRuntimeTeardownExecutionLane.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Lifecycle/TerminalSurfaceRuntimeTeardownTicket.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Runtime/TerminalSurfaceRuntimeDependencies.swiftPackages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swiftPackages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceTeardownCallbackLifetimeTests.swiftPackages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.cPackages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.h
There was a problem hiding this comment.
3 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c">
<violation number="1" location="Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c:48">
P3: The 5s blocking-gate timeout is computed from CLOCK_REALTIME, so a wall-clock adjustment (NTP step, manual/timezone change) on the CI builder can arbitrarily extend the fallback wait (clock stepped backward) or fire it early (stepped forward). The deadlock guard is exactly the safety net this PR relies on for teardown hangs, so it shouldn't be at the mercy of wall-clock jumps. Consider creating the condvar with a CLOCK_MONOTONIC attr (pthread_condattr_setclock) and using CLOCK_MONOTONIC in the deadline helper; the monotonic deadline also avoids being distorted across the wait loops.</violation>
</file>
<file name="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swift">
<violation number="1" location="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swift:174">
P2: The final freed-set assertion can race: later tickets complete when the coordinator calls completion.finish(), which is independent of the `Task { await recorder.record(bits) }` actor hops spawned inside each freeSurface. If those record tasks haven't run yet, `Set(recorder.freed)` may be partial, making this test flaky. Await the recorder before asserting, matching the sibling test which uses `recorder.waitForFreeCount(...)`.</violation>
</file>
<file name="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceTeardownCallbackLifetimeTests.swift">
<violation number="1" location="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceTeardownCallbackLifetimeTests.swift:44">
P2: This new test measures wall-clock latency (`clock.now - start < .seconds(1)`) around `ghostty_surface_free(unrelatedSurface)` to prove the call wasn't intercepted by the blocking gate. On a loaded CI runner a correctly non-blocked call could exceed 1 second and fail for the wrong reason, and the assertion tests a timing threshold rather than the logical invariant (the gate didn't intercept this surface). Consider using a completion signal with a deadline-bounded wait instead, matching the pattern already used in `teardownSurfaceKeepsMainActorResponsiveWhileNativeFreeIsBlocked` below.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
5 issues found across 9 files
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c">
<violation number="1" location="Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c:48">
P3: The gate's 5s deadline is derived from CLOCK_REALTIME, which can be stepped forward or backward (NTP sync, manual time changes). A forward step makes pthread_cond_timedwait return ETIMEDOUT early and the gate flake as a false failure; a backward step can delay the watchdog well beyond 5s. Use CLOCK_MONOTONIC for a stable wait timeout; since the default-initialized condvar is CLOCK_REALTIME, initialize it via pthread_condattr_setclock(CLOCK_MONOTONIC) so the monotonic abs_timeout is valid.</violation>
<violation number="2" location="Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c:72">
P3:</violation>
</file>
<file name="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swift">
<violation number="1" location="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swift:129">
P3: The new test duplicates the ~15-line stuck-free harness (AsyncStream<Void>.makeStream + DispatchSemaphore + makeAsyncIterator/next + defer cleanup) already used by stuckHibernationFreeDoesNotStrandAnotherAdmissionOrClose. Extracting a small helper (e.g. a 'stuckFree' that returns a StartedStream + release() cleans up) would remove the duplicated scaffolding and keep the teardown tests focused on their lane-specific assertions.</violation>
</file>
<file name="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceTeardownCallbackLifetimeTests.swift">
<violation number="1" location="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceTeardownCallbackLifetimeTests.swift:39">
P2: This new test measures wall-clock elapsed time (`ContinuousClock().now` diff under 1 second) to assert the surface-free gate didn't intercept an unrelated surface. Wall-clock latency thresholds are flaky under CI load — a correctly non-blocked call can still take over 1 second on a busy runner, failing the test for the wrong reason. Use a completion signal with a deadline-bounded wait instead (as done in `teardownSurfaceKeepsMainActorResponsiveWhileNativeFreeIsBlocked` right below), so the test asserts the actual invariant (the call didn't block on the gate) rather than a timing threshold.</violation>
<violation number="2" location="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceTeardownCallbackLifetimeTests.swift:76">
P3: The `teardownSurfaceKeepsMainActorResponsiveWhileNativeFreeIsBlocked` test decides pass/fail on a fixed 1-second wall-clock window for the main-actor probe. A genuinely healthy teardown can be scheduled late under CI load, so this can flake false-fail even though the fix is correct. Consider making the timeout configurable/injected (or polling the probe with a deadline rather than a single 1s gate) so the watchdog bounds deadlock without coupling the pass/fail decision to absolute timing.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swift (1)
274-274: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMake the test prove the bounded-close fallback.
The isolated lane is idle when the stale-reservation ticket runs. An incorrect implementation could still use that lane and pass this test. Start one blocked isolated teardown before enqueueing the stale ticket. Require the stale ticket and its free callback to complete while the isolated teardown remains blocked. Release the blocked teardown afterward.
🤖 Prompt for 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. In `@Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swift` at line 274, Update staleIsolatedReservationFallsBackToBoundedClose to start and block an isolated teardown before enqueueing the stale-reservation ticket, ensuring the isolated lane is occupied. Await completion of both the stale ticket and its free callback while the blocking teardown remains pending, then release the blocked teardown afterward so the test proves the bounded-close fallback rather than reuse of an idle isolated lane.
🤖 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.
Outside diff comments:
In
`@Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swift`:
- Line 274: Update staleIsolatedReservationFallsBackToBoundedClose to start and
block an isolated teardown before enqueueing the stale-reservation ticket,
ensuring the isolated lane is occupied. Await completion of both the stale
ticket and its free callback while the blocking teardown remains pending, then
release the blocked teardown afterward so the test proves the bounded-close
fallback rather than reuse of an idle isolated lane.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8b28b2b1-d94e-43c4-b12d-0eb18e8ac4f5
📒 Files selected for processing (1)
Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swift
…ick-hang # Conflicts: # Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.h
3 of 6 tasks
6 tasks done
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
TerminalSurfaceRuntimeTeardownCoordinatorCloses #9220.
Root cause and symbolication chain
The attachment is a 12.79-second hang stackshot, not a crash log. Its shipped cmux image UUID is
6FD8C9CB-CBA6-3651-9733-027FBC2490CB, loaded at0x100c44000. In all 17 samples the main actor is blocked in__ulock_waitfrom the Swift concurrency job whose cmux return address is0x104943628(unslid0x103cff628, image offset0x3cff628). The turnstile owner is Ghostty renderer thread0x6598191.The public release does not include dSYMs, and a same-commit rebuild produced a different UUID, so that nonmatching dSYM was not used as false symbolication. Instead, the shipped arm64 instructions at
0x103cff628were compared with the pinned Ghosttybb30526carchive object. The instruction sequence is byte-identical to_Surface.deinit + 156; the sampled address is the return site of the firstpthread_join. Ghostty source atsrc/Surface.zig:847-867identifies that first join asself.renderer_thr.join().The complete blocking chain is:
@MainActor Swift Task→ghostty_surface_free→ GhosttySurface.deinit→renderer_thr.join()→ renderer thread does not finish → main actor hangs.Release
TerminalSurface.teardownSurface()scheduledghostty_surface_freeinsideTask { @MainActor in ... }. Current main already routed deinit and agent-hibernation frees throughTerminalSurfaceRuntimeTeardownCoordinator; explicit teardown was the remaining production bypass. Sidebar selection/restore churn can reach that explicit close path, making the sidebar click the visible trigger.Fix
TerminalSurface.teardownSurface()now detaches all main-actor ownership first, then submits the pointer and its retained callback userdata to the injected teardown coordinator. Native free runs away from the main actor, and callback userdata is released only after Ghostty's renderer/IO joins return.The coordinator now owns two independently startable close/deinit execution slots. A request holds its slot until native free, userdata release, and ticket completion are all finished. One permanently blocked Ghostty join therefore leaves the other slot able to drain later closes. Hibernation admission and its two isolated queues remain a separate bounded pool.
Invariants after this change:
ghostty_surface_freeor renderer-thread join runs on the main actorRelated PR #8905 independently targets this hang class but also contains unrelated hook/environment work; this PR is the self-contained terminal teardown fix and regression.
Regression proof
The regression commits are intentionally split from their fixes:
0a42b05f6badds the main-actor responsiveness regression. On the AWS macOS 15.7.4 builder it failed in 1.053s before77ab34b3a4routed explicit teardown through the coordinator.7fcc434fe6adds the stuck-close isolation regression. It failed with both later close tickets timing out behind the first blocked free befored5c5c3e8adintroduced bounded close slots.40578022e7adds the surface-scoped gate regression. It failed after delaying an unrelated surface by 5.119s befored5c5c3e8adscoped the gate by pointer identity.AWS verification after the fix:
TerminalSurfaceRuntimeTeardownCoordinatorTests: 7/7 passedTerminalSurfaceTeardownCallbackLifetimeTests: 10/10 passedNo local
xcodebuild testor XCUITest was run.Localization audit
No user-facing strings, settings, menus, schema text, docs, or help text changed; no localization catalog update is required.
Summary by CodeRabbit
Performance
Bug Fixes
Tests
Note
High Risk
Changes terminal native teardown and threading around ghostty_surface_free on critical UI paths; regressions could cause hangs, leaks, or use-after-free on IO callbacks, though ordering and concurrency are heavily tested.
Overview
Fixes main-actor hangs when closing terminals (e.g. sidebar churn) by removing the last production path that called
ghostty_surface_freeon the main actor insideteardownSurface(). Explicit teardown now matches deinit and agent-hibernation: detach ownership on the main actor, then enqueue native free onTerminalSurfaceRuntimeTeardownCoordinator.Close/deinit teardown is no longer fully serialized on one utility worker. The coordinator runs up to two concurrent close slots (
maximumConcurrentCloseTeardownCount), each on its own utilityDispatchQueue, so one stuck Ghostty renderer join cannot block every later close. The execution laneserializedCloseis renamedboundedClose.Tests add a stuck-close regression, a main-actor responsiveness check while native free is blocked, and surface-scoped blocking stubs so the test gate does not intercept unrelated frees.
Reviewed by Cursor Bugbot for commit f6bd09f. Bugbot is set up for automated code reviews on this repo. Configure here.