Repository navigation
Bound app termination with a force-exit watchdog (#6758) - #6837
Merged
Merged
Conversation
cmux can hang the main thread for ~30s on Cmd+Q when a clipboard-history manager (Paste, Raycast, Maccy, …) is mid-read of cmux's promised pasteboard data: AppKit's will-terminate gauntlet runs CFPasteboardResolveAllPromisedData, which blocks on a stuck mach round-trip to the pasteboard server until the OS force-kills the app. This is the third "an observer blocks the main thread during quit" report (cf. #6415 PostHog flush, #6381 ghostty lock); the structural gap is that quit has no global "return within N seconds no matter what" guard. Add TerminationWatchdog plus its tests, with the watchdog deliberately inert (it never starts the firing thread) so the tests go red. The end-to- end pasteboard deadlock is not unit-testable — reproducing it requires the real pasteboard server and would wedge the test process — so the tests cover the watchdog mechanism that bounds it. The fix commit starts the thread and arms the watchdog from the terminate path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implement TerminationWatchdog.arm and arm it from the terminate path so a committed quit always returns within a bounded time, even when AppKit's will-terminate gauntlet wedges on an Apple-owned observer we don't control (CFPasteboardResolveAllPromisedData blocking on a stuck pasteboard-server round-trip while a clipboard-history manager reads cmux's promised data). The watchdog runs on a dedicated background thread with no run-loop, GCD, or main-actor dependency, so it fires even while the main thread is parked in mach_msg. It is armed in prepareForConfirmedAppTermination() — after the critical session/state save and before AppKit posts will-terminate — and, as a backstop, at the start of applicationWillTerminate(). Arming is idempotent, so the two sites and repeated quit attempts never stack threads. If the process has not exited within the deadline it force-exits cleanly, turning a ~30s hang into a bounded quit. This closes the structural gap shared with #6415 and #6381: quit now has a global "return within N seconds no matter what" guard. Fixes #6758 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a termination watchdog that arms a hard quit deadline, integrates it into AppDelegate’s termination paths, and includes atomic support, tests, and Xcode project wiring. ChangesTermination Watchdog
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (21 passed)
✨ Finishing Touches📝 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 |
Contributor
Greptile SummaryAdds
Confidence Score: 5/5Safe to merge. The change is well-scoped: a single AppDelegate-owned watchdog instance armed only after the critical state save, with a lock-free _exit firing path that avoids any Foundation or filesystem work while termination may already be wedged. The watchdog fires after 8 seconds on a dedicated OS thread with no dependency on GCD, run loops, or Foundation — exactly the infrastructure that may be stalled. The C11 atomic latch correctly prevents double-scheduling. Critical state is saved before arm() is called on both the primary and backstop paths. The raw-Thread-over-GCD choice is explicitly justified inline. Tests cover the mechanism deterministically with an injected scheduler. No new global state, no test seams in production source, and no unconditional blocking on interactive paths. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant AppDelegate
participant TerminationWatchdog
participant WatchdogThread
participant AppKit
User->>AppDelegate: Cmd+Q
AppDelegate->>AppDelegate: prepareForConfirmedAppTermination()
AppDelegate->>AppDelegate: saveSessionSnapshot() + flushPendingSaves()
AppDelegate->>TerminationWatchdog: arm() [latch: 0→1]
TerminationWatchdog->>WatchdogThread: spawn raw Thread (sleep 8s)
AppDelegate->>AppKit: allow termination
AppKit->>AppDelegate: applicationWillTerminate()
AppDelegate->>AppDelegate: saveSessionSnapshot() + flushPendingSaves()
AppDelegate->>TerminationWatchdog: arm() [latch already 1 → no-op]
AppKit-->>AppKit: CFPasteboardResolveAllPromisedData (may wedge main thread)
alt "Process exits normally (< 8s)"
AppKit-->>AppKit: terminates
WatchdogThread-->>WatchdogThread: thread reclaimed on exit
else "Wedged > 8s (deadline fires)"
WatchdogThread->>WatchdogThread: sleep(8s) elapses
WatchdogThread->>AppDelegate: _exit(EXIT_SUCCESS) [lock-free]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant AppDelegate
participant TerminationWatchdog
participant WatchdogThread
participant AppKit
User->>AppDelegate: Cmd+Q
AppDelegate->>AppDelegate: prepareForConfirmedAppTermination()
AppDelegate->>AppDelegate: saveSessionSnapshot() + flushPendingSaves()
AppDelegate->>TerminationWatchdog: arm() [latch: 0→1]
TerminationWatchdog->>WatchdogThread: spawn raw Thread (sleep 8s)
AppDelegate->>AppKit: allow termination
AppKit->>AppDelegate: applicationWillTerminate()
AppDelegate->>AppDelegate: saveSessionSnapshot() + flushPendingSaves()
AppDelegate->>TerminationWatchdog: arm() [latch already 1 → no-op]
AppKit-->>AppKit: CFPasteboardResolveAllPromisedData (may wedge main thread)
alt "Process exits normally (< 8s)"
AppKit-->>AppKit: terminates
WatchdogThread-->>WatchdogThread: thread reclaimed on exit
else "Wedged > 8s (deadline fires)"
WatchdogThread->>WatchdogThread: sleep(8s) elapses
WatchdogThread->>AppDelegate: _exit(EXIT_SUCCESS) [lock-free]
end
Reviews (12): Last reviewed commit: "Avoid growing AppDelegate termination pa..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- Codex/autoreview P1 (correctness): the watchdog's onFire logged a StartupBreadcrumbLog entry (flock + Foundation/file I/O) before _exit. If that logging stalled or contended during an already-wedged termination, the watchdog thread could block before reaching _exit and the quit hang would stay unbounded — defeating the guarantee. Drop the breadcrumb: the firing path is now an unconditional, lock-free _exit (the default onFire), which does zero Foundation/filesystem work before exiting. - Greptile P1 (no-ambient-global-state): replace the TerminationWatchdog.shared singleton with an AppDelegate-owned instance, next to the existing terminate-control state (terminateKillWatchdogTask). The type was already injectable, so this is a small wiring change. - Greptile P2: document why the deadline uses a raw Thread + Thread.sleep rather than a GCD timer (the wedged termination can sit on GCD/run-loop infrastructure, so the firing path must not depend on it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cmux-policy (Aziz concurrency) prefers actor isolation over locks for new runtime state. Rejected here with rationale recorded in-code: an actor would force `arm()` async, but it is called synchronously from the terminate delegate methods and the deadline fires on a raw Thread — and the watchdog must not depend on the Swift concurrency runtime, which may itself be wedged during the termination it guards against. This is the same sanctioned NSLock + nonisolated(unsafe) shape TerminalPasteboardService uses for synchronous- callback state. Comment-only change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d-hang-30s-on-cmd # Conflicts: # .github/swift-file-length-budget.tsv
CI's test-determinism gate (scripts/check-test-determinism.py --strict) flagged the prior tests for real sleeps / wall-clock timeouts (sleep-then-assert and assert-on-duration). Invert the time dependency per the gate's contract instead of allowlisting: extract the deadline scheduler as an injectable `DeadlineScheduler`. Production keeps the raw background Thread (`TerminationWatchdog.threadScheduler`); the tests inject a synchronous capturing scheduler and advance the deadline by hand. The tests now assert idempotency (three arms schedule the deadline exactly once) and exactly-once firing with zero real sleeps, timeouts, or wall-clock reads, so they are deterministic by construction. arm() is now a thin idempotent latch over scheduleDeadline(deadline, onFire); behavior is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
CI status noteAll substantive checks are green: The only red is
Re-running shard 2 once CI load subsides should clear it. |
# Conflicts: # .github/swift-file-length-budget.tsv
This comment has been minimized.
This comment has been minimized.
![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 `@Sources/AppDelegate.swift`:
- Around line 2001-2004: The termination flow in applicationWillTerminate(_:)
arms the backstop watchdog too late, leaving
saveSessionSnapshotIncludingProcessDetectedIndexes(includeScrollback:removeWhenEmpty:)
and ClosedItemHistoryStore.shared.flushPendingSaves() outside the deadline
window. Move terminationWatchdog.arm() to run before those I/O calls, keeping
the existing isTerminatingApp flag update and snapshot/flush sequence intact so
the backstop is active even when prepareForConfirmedAppTermination() was never
reached.
🪄 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
Run ID: 16ca1104-cf3c-468c-b463-326b12138ca9
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (6)
Sources/AppDelegate.swiftSources/TerminationWatchdog.swiftSources/TerminationWatchdogAtomic.cSources/TerminationWatchdogAtomic.hcmux-Bridging-Header.hcmux.xcodeproj/project.pbxproj
This was referenced Jul 13, 2026
This branch was successfully deployed
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.
Fixes #6758
Problem
cmux hangs the main thread for ~30s on Cmd+Q. The hang is AppKit's
will-terminate gauntlet running
CFPasteboardResolveAllPromisedData, whichflushes promised (lazy) pasteboard data with a blocking mach round-trip to the
pasteboard server (
pboard). When a clipboard-history manager (Paste, Raycast,Maccy, Pastebot, …) is mid-read of cmux's promised clipboard data, that
round-trip wedges until the OS force-kills the app.
Reported stack (every sample identical):
This is the third "an observer blocks the main thread during quit" report
(cf. #6415 PostHog flush, #6381 ghostty lock). As the reporter notes, the
structural gap is that quit has no global "return within N seconds no matter
what" guard — the blocking observer here is Apple-owned
(
CFPasteboardResolveAllPromisedData), so cmux cannot prevent it from runningonce
terminate:posts the notification.Why not "just write concrete data on copy"?
cmux already does. Its terminal/image clipboard writes are eager
(
clearContents()+setString/setData,writeObjectswith materializeditems), and every
declareTypes/addTypesusesowner: nil(no lazy owner).The promise being resolved is held by
pboardon cmux's behalf for theexternal reader; cmux can't clear it without the same blocking round-trip. So
the right fix is the missing structural guard.
Fix
Add
TerminationWatchdog: a one-shot, idempotent hard deadline on thecommitted-quit sequence.
Thread+Thread.sleep, notGCD) with no run-loop, GCD-queue, or main-actor dependency, so it fires even
while the main thread is parked in
mach_msg.prepareForConfirmedAppTermination()— after the criticalsession/state save and before AppKit posts will-terminate — and, as a
backstop, at the start of
applicationWillTerminate(). Arming is idempotent,so the two sites and repeated quit attempts never stack threads.
normal sub-second teardown, far under the OS's ~30s hang watchdog), it
force-exits via an unconditional, lock-free
_exit. The firing path doesno Foundation/filesystem work before exiting (the termination it guards
against may itself be wedged on exactly such a lock). The bytes that matter
are already on disk because the save runs before the watchdog is armed.
AppDelegate(next to the existingterminateKillWatchdogTask), nota global singleton.
Result: a wedged Apple observer turns a ~30s freeze into a bounded quit. This
closes the same structural gap behind #6415 and #6381.
Testing
Two-commit red → green per the repo's regression policy:
test: red regression …— addsTerminationWatchdog+ tests with thewatchdog deliberately inert (never starts the firing thread) → tests fail.
Bound app termination …— implementsarmand wires it into the terminatepath → tests pass.
The tests cover the watchdog mechanism (fires
onFireafter the deadlinefrom a background thread; idempotent so it fires exactly once). The end-to-end
pasteboard deadlock itself is not unit-testable — reproducing it needs the
real pasteboard server plus a clipboard-history reader and would wedge the test
process — so the unit tests bound what can be tested deterministically.
Review iteration (commit
Address review …)onFireoriginally logged aStartupBreadcrumbLogbreadcrumb (flock+ Foundation/file I/O) before_exit. If that stalled during an already-wedged termination, the watchdogitself could block and never exit — defeating the guarantee. Removed; the
firing path is now a bare, lock-free
_exit.TerminationWatchdog.sharedwith an
AppDelegate-owned instance.Thread-vs-GCD choice inline.Localization audit
No user-facing strings added or changed — code comments only. (The earlier
diagnostic breadcrumb string was removed in the review iteration above.) Nothing
to add to
Resources/Localizable.xcstringsor the web message catalogs.Summary by CodeRabbit
Bug Fixes
Tests