Skip to content

Fix orphaned modal sheet deadlock when parent window is destroyed - #699

Closed
Ranxi wants to merge 1 commit into
gnachman:masterfrom
Ranxi:fix/orphaned-modal-sheet-deadlock
Closed

Fix orphaned modal sheet deadlock when parent window is destroyed#699
Ranxi wants to merge 1 commit into
gnachman:masterfrom
Ranxi:fix/orphaned-modal-sheet-deadlock

Conversation

@Ranxi

@Ranxi Ranxi commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Environment

  • iTerm2 version: 3.6.9 (Build 21.32.14)
  • macOS: 15.3.1 (24D70), Apple Silicon (ARM64)
  • Xcode: 16.2 (16C5032a)
  • Configuration: Multiple tabs per window, "Confirm closing multiple sessions" enabled (default setting)

Steps to Reproduce

  1. SSH into a remote host, start tmux and open vim inside one tab
  2. Ensure the current iTerm2 window has at least 2 tabs (so the close-confirmation dialog will trigger)
  3. With vim focused, the Touch Bar shows a temporary button (icon starting with "i")
  4. Tap that Touch Bar button — a new temporary iTerm2 window opens
  5. Immediately click the red close button (X) on that new window
  6. A "Confirm closing multiple sessions" sheet appears. Do not click any button.
  7. Wait a few seconds — the confirmation sheet and the temporary window auto-dismiss together
  8. Return to the original working window: the app is now frozen

Actual Behavior

The entire iTerm2 application becomes unresponsive:

  • Menu bar cannot be interacted with (mouse clicks ignored)
  • Cmd+N / menu items have no effect (cannot create new windows)
  • Cannot select text with mouse in any existing tab
  • Mouse cursor invisible in terminal sessions (modal loop captures all events)
  • Keyboard input is buffered/delayed

The app process does not crash — it runs indefinitely with the main thread stuck in a nested runModalForWindow: event loop. Only force-quitting via Cmd+Option+Esc recovers, losing all session state including remote tmux/SSH connections.

Root Cause

-[NSAlert(iTerm) runSheetModalForWindow:] uses beginSheetModalForWindow:completionHandler: paired with a synchronous runModalForWindow:. When the parent window is destroyed (the Touch Bar temporary window auto-dismisses), the sheet is torn down with it, but the completion handler never fires — so stopModalWithCode: is never called. The main thread remains permanently blocked in the modal run loop.

Fix

Added an NSWindowWillCloseNotification observer in runSheetModalForWindow:. If the parent window closes before the user dismisses the sheet, the observer calls [NSApp abortModal], which forces the modal loop to unwind. A __block BOOL stopped flag prevents the observer and the normal completion handler from racing.

Changes in sources/Categories/NSAlert+iTerm.m only (+14 lines).

Test Plan

  • Deterministic unit test: NSAlert_iTermTests/test_modalReturnsWhenParentWindowCloses closes the window from a background thread while the modal is running, verifying the call returns instead of blocking
  • Manual reproduction: followed the exact steps above on macOS 15.3.1 / iTerm2 built from source — confirmed the hang no longer occurs
  • All 9 call sites of runSheetModalForWindow: benefit from this fix (single method-level change)

-[NSAlert(iTerm) runSheetModalForWindow:] uses beginSheetModalForWindow:
with a synchronous runModalForWindow:. If the parent window is
destroyed before the user dismisses the sheet, the completion handler
never fires so stopModalWithCode: is never called, leaving the main
thread permanently stuck in the modal run loop.

This can happen when a temporary window (e.g. Touch Bar popup) is
closed while a confirm-close sheet is showing: the window auto-
dismisses, the sheet is destroyed with it, but the modal loop
continues running. The entire app becomes unresponsive — menu bar,
mouse input, keyboard, new windows all blocked.

The fix adds an NSWindowWillCloseNotification observer that calls
[NSApp abortModal] if the parent window closes before the sheet is
dismissed. A stopped flag ensures the observer and completion handler
do not race.

Added a deterministic test that closes the window from a background
thread while the modal is running, verifying the call returns instead
of blocking.
@gnachman

Copy link
Copy Markdown
Owner

Thank you, this is a real fix for a nasty deadlock. I verified it end to end: reproduced the orphaned-modal hang (dead menu bar, Force-Quit only) with a timer-driven window close during a sheet, and confirmed your observer approach cleanly aborts the modal instead of wedging. Merged (squashed) as 1b0ab6d, with you as the author.

A couple of adjustments I made while reviewing:

  • Remove the observer on every exit path, so a modal unwound by something other than the two blocks (e.g. a third party calling abortModal during termination) can't leave a stale observer that later aborts an unrelated modal.
  • Reworked the regression test to close the window on the main queue with no sleep-based timing.

One follow-up worth noting: the same orphaned-modal deadlock exists in several other hand-rolled beginSheet: + runModalForWindow: sites that do not go through this method (coprocess panel, paste-special, session parameter panel, password manager, Add Account panel, split panel). Those are actually more dangerous, since a custom sheet has no always-clickable default button to recover with. I plan to generalize this guard into a shared helper those call sites can use. Thanks again for the fix and the clear write-up.

@gnachman gnachman closed this Jul 25, 2026
@Ranxi

Ranxi commented Jul 25, 2026 via email

Copy link
Copy Markdown
Contributor Author

@gnachman

Copy link
Copy Markdown
Owner

Follow-up: I landed the generalization mentioned above as 9158a7c. It factors this parent-window-close guard out of the alert category into a shared iTermRunModalForWindowAbortingIfParentCloses() and routes the five other hand-rolled beginSheet: + runModalForWindow: sheets through it (paste-special, coprocess, session parameter, split, and add-account panels), so the same deadlock is closed across all of them, not just alerts.

While generalizing I also hardened the guard beyond the original alert version:

  • Nesting: -abortModal only ends the topmost modal session, so a nested modal (alert, open/save panel, another sheet) stacked over the sheet would be aborted instead of ours, leaving us blocked. It now re-aborts on each run-loop pass until our own session ends.
  • nil parent: skip the observer when the parent window is nil (object:nil would abort on any window closing).
  • Deferred completion: beginSheet:'s completion runs a run-loop turn later, after we may have aborted, so each stopModal is gated on NSApp.modalWindow being the sheet.
  • The session parameter panel treats an abort as a cancel, so closing the window during a $$name$$ prompt no longer launches with an empty substitution.

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.

2 participants