Skip to content

fix: remove startup key monitor in defer so it can't leak - #101

Merged
gi11es merged 2 commits into
masterfrom
fix/key-event-monitor-leak
Jul 29, 2026
Merged

fix: remove startup key monitor in defer so it can't leak#101
gi11es merged 2 commits into
masterfrom
fix/key-event-monitor-leak

Conversation

@gi11es

@gi11es gi11es commented Jul 13, 2026

Copy link
Copy Markdown
Owner

The +0.3s initial-input closure in TerminalSurface.startShell early-returned before removing the window-wide keystroke-swallowing NSEvent monitor when pendingInitialInput was nil. A leaked monitor silently eats every keystroke for the window whenever the surface's view is attached (latent bug surfaced by the #99 investigation — ruled out as that freeze's cause by live state inspection, but real).

Fix: move monitor removal and handlesPasteShortcuts restore into a single defer so every exit path cleans up.

🤖 Generated with Claude Code

gi11es and others added 2 commits July 14, 2026 00:29
The +0.3s initial-input closure early-returned before removing the
window-wide keystroke-swallowing NSEvent monitor when
pendingInitialInput was nil. A leaked monitor silently eats every
keystroke for the window whenever the surface's view is attached.
Move monitor removal (and handlesPasteShortcuts restore) into defer.

Found while investigating #99.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- remove any existing monitor before installing a new one (overwrite
  orphaned the old token inside AppKit)
- the delayed closure now removes only the monitor it installed, gated
  on identity, so it can never remove a later call's monitor or
  double-remove one terminate() already handled
- add deinit backstop for surfaces released without terminate()
- clear pendingInitialInput in terminate() so a tab closed inside the
  0.3s window doesn't send its initial command to a dead PTY

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@gi11es

gi11es commented Jul 13, 2026

Copy link
Copy Markdown
Owner Author

Subagent review follow-up (pushed): the original defer fix guarded a path that's currently unreachable — the real leak vectors were (1) overwriting keyEventMonitor on a second startShell orphaning the old token inside AppKit, and (2) surfaces released without terminate() having no cleanup backstop. Now: existing monitor removed before install, the delayed closure removes only its own token (identity-gated, no double-remove), deinit backstop added, terminate() clears pendingInitialInput so a tab closed inside the 0.3s window doesn't write its initial command to a dead PTY.

@gi11es
gi11es merged commit 93c1cdd into master Jul 29, 2026
3 checks passed
@gi11es
gi11es deleted the fix/key-event-monitor-leak branch July 29, 2026 18:32
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