From 42277445b484ec77f92c7e898868309734d09463 Mon Sep 17 00:00:00 2001 From: Gilles Dubuc Date: Tue, 14 Jul 2026 00:29:02 +0800 Subject: [PATCH 1/2] fix: remove startup key monitor in defer so it can't leak 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 --- Sources/Terminal/TerminalSurface.swift | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/Sources/Terminal/TerminalSurface.swift b/Sources/Terminal/TerminalSurface.swift index 14ec9f4..35b4e34 100644 --- a/Sources/Terminal/TerminalSurface.swift +++ b/Sources/Terminal/TerminalSurface.swift @@ -473,13 +473,19 @@ class TerminalSurface: NSObject, LocalProcessTerminalViewDelegate { } DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { [weak self] in guard let self else { return } - defer { self.terminalView.handlesPasteShortcuts = true } + // Always restore input handling and remove the window-wide key + // monitor, even if the pending input was consumed elsewhere — + // a leaked monitor silently swallows every keystroke for the + // window whenever this surface's view is attached. + defer { + self.terminalView.handlesPasteShortcuts = true + if let monitor = self.keyEventMonitor { + NSEvent.removeMonitor(monitor) + self.keyEventMonitor = nil + } + } guard let input = self.pendingInitialInput else { return } self.pendingInitialInput = nil - if let monitor = self.keyEventMonitor { - NSEvent.removeMonitor(monitor) - self.keyEventMonitor = nil - } self.sendInput(input) } } From 559da5812841fbc6b1dd50e6d3fdc2a024cc1aa4 Mon Sep 17 00:00:00 2001 From: Gilles Dubuc Date: Tue, 14 Jul 2026 01:54:02 +0800 Subject: [PATCH 2/2] fix: close remaining key-monitor leak vectors from review - 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 --- Sources/Terminal/TerminalSurface.swift | 38 ++++++++++++++++++++------ 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/Sources/Terminal/TerminalSurface.swift b/Sources/Terminal/TerminalSurface.swift index 35b4e34..d49e882 100644 --- a/Sources/Terminal/TerminalSurface.swift +++ b/Sources/Terminal/TerminalSurface.swift @@ -341,6 +341,15 @@ class TerminalSurface: NSObject, LocalProcessTerminalViewDelegate { name: .deckardScrollbackChanged, object: nil) } + deinit { + // Backstop for surfaces released without terminate(): a still-installed + // monitor would outlive the surface (its closure retains the view) and + // permanently swallow the window's keystrokes. + if let monitor = keyEventMonitor { + NSEvent.removeMonitor(monitor) + } + } + /// Apply a color scheme to this terminal. func applyColorScheme(_ scheme: TerminalColorScheme) { scheme.apply(to: terminalView) @@ -465,22 +474,34 @@ class TerminalSurface: NSObject, LocalProcessTerminalViewDelegate { if let initialInput { pendingInitialInput = initialInput terminalView.handlesPasteShortcuts = false + // Overwriting keyEventMonitor would orphan the previous monitor + // inside AppKit — permanently swallowing every keystroke for the + // window — so remove any existing one first. + if let existing = keyEventMonitor { + NSEvent.removeMonitor(existing) + keyEventMonitor = nil + } let view = terminalView - keyEventMonitor = NSEvent.addLocalMonitorForEvents(matching: .keyDown) { event in + let monitor = NSEvent.addLocalMonitorForEvents(matching: .keyDown) { event in // Swallow key events targeting our terminal view's window. if event.window === view.window { return nil } return event } - DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { [weak self] in + keyEventMonitor = monitor + DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { [weak self, monitor] in + // If the surface was deallocated, deinit removes the monitor. guard let self else { return } - // Always restore input handling and remove the window-wide key - // monitor, even if the pending input was consumed elsewhere — - // a leaked monitor silently swallows every keystroke for the - // window whenever this surface's view is attached. + // Always restore input handling and remove the monitor this + // call installed on every exit path — but only if it is still + // the active one (terminate() or a later startShell may have + // removed or replaced it already; removing a monitor twice is + // not allowed). defer { self.terminalView.handlesPasteShortcuts = true - if let monitor = self.keyEventMonitor { - NSEvent.removeMonitor(monitor) + if let current = self.keyEventMonitor, + let monitor, + (current as AnyObject) === (monitor as AnyObject) { + NSEvent.removeMonitor(current) self.keyEventMonitor = nil } } @@ -502,6 +523,7 @@ class TerminalSurface: NSObject, LocalProcessTerminalViewDelegate { func terminate() { guard !processExited else { return } processExited = true + pendingInitialInput = nil if let monitor = keyEventMonitor { NSEvent.removeMonitor(monitor) keyEventMonitor = nil