50 KiB
Fix per-session PTY fd leak in the current daemon
Status: resolved (2026-05-01)
Native-side root cause confirmed and patched. E2E validation under both Node and Electron ABIs: 50 spawn/kill cycles, ptmx count stays at baseline (0). Before the patch, the same harness reproduced a 1-fd-per-spawn linear leak (5 → 55 ptmx fds over 50 cycles).
Root cause (actual)
The leak is a native-side off-by-one in node-pty@1.1.0's pty_posix_spawn cleanup loop on macOS. In src/unix/pty.cc:
- Allocation (lines 697-701) walks
low_fds[0..2], allocating viaposix_openpt(O_RDWR). The loopbreaks at the firstlow_fds[count] >= STDERR_FILENO, socountholds the index of the last allocated slot (0, 1, or 2). These are decoy/dev/ptmxhandles used as a workaround for a macOS pty race condition that ensures the real master fd lands aboveSTDERR_FILENO. - Cleanup (lines 781-783, buggy):
When the typical case triggers (for (; count > 0; count--) { close(low_fds[count]); }breakatcount=0, i.e. the very firstposix_openptreturned an fd ≥ 2), the loop body never executes —low_fds[0]is never closed. Every spawn leaks one ptmx handle.
Upstream fixed this in commit af053f2 (PR #882, 2026-01-28), but version 1.1.0 (pinned here) predates that fix.
Applied fix: minimal 3-line backport in config/patches/node-pty@1.1.0.patch:
- for (; count > 0; count--) {
- close(low_fds[count]);
+ for (size_t i = 0; i <= count && i < 3; i++) {
+ close(low_fds[i]);
}
The i < 3 bound is an over-cautious defense against the pathological case where all three posix_openpt calls returned fds < STDERR_FILENO (loop exhausts to count=3, which would otherwise read low_fds[3] — UB in upstream's fix too, though unreachable in practice).
Why the JS-side destroy() discipline (below) is still correct
The native patch alone closes the leak. But the JS-side changes landed in pty-subprocess.ts, session.ts, terminal-host.ts, local-pty-provider.ts, and relay/pty-handler.ts remain load-bearing for separate reasons:
- SIGHUP-to-recycled-pid hazard.
UnixTerminal.destroy()registers_socket.once('close', () => this.kill('SIGHUP')). After the child is reaped, its pid can be recycled to an unrelated user process (Chrome tab, editor, etc.) before the socket close event fires. Neutralizingproc.killon POSIX beforedestroy()prevents delivering SIGHUP to a stranger. forceKill/signalguard against recycled pid.process.kill(proc.pid, …)afteronExittargets a potentially-recycled pid. The internaldeadguard converts these into no-ops.- Daemon shutdown determinism.
TerminalHost.dispose()→Session.forceKillAndDisposeSubprocess()reaps stubborn children via SIGKILL and releases the master fd synchronously, bypassing the 5sKILL_TIMEOUT_MSfallback. - Belt-and-suspenders fd release. Calling
destroy()on every teardown path remains the contract that will keep holding if the pinned node-pty is ever replaced or upgraded to a build without the bug.
Original problem statement (for historical context)
Problem
The current daemon-v4 is leaking PTY master file descriptors (ptmx) at a rate roughly 20× the live session count. In a real reported case, the live v4 daemon was holding 269 ptmx fds for only 13 active terminals — every closed terminal should have returned its ptmx to the OS, but the daemon keeps accumulating them until it hits macOS's kern.tty.ptmx_max=511 cap and every new terminal (Orca or otherwise) fails with "cannot allocate any more pty devices."
This is a distinct bug from the orphaned-daemon issue handled in auto-kill-old-daemons.md:
| Bug | Fd leak unit | Lifetime | Fix surface |
|---|---|---|---|
| Orphaned daemons | Whole daemon process | Across app upgrades | daemon-init.ts, daemon-pty-router.ts |
| This doc | Per-PTY ptmx |
Within any node-pty-hosting process | pty-subprocess.ts (daemon), local-pty-provider.ts (legacy), relay/pty-handler.ts (SSH) + wiring |
Both bugs compound each other (stale daemons × leaky sessions), but either one is sufficient to exhaust the cap in a few days of normal use. Fixing orphan daemons alone would only slow down the bleed — the PTY wrappers themselves must stop leaking.
The same missing destroy() call exists in all three node-pty-hosting code paths (daemon, legacy local provider, SSH relay), so the fix is scoped to all three.
Root cause
The master ptmx fd is owned by a tty.ReadStream that node-pty stores on the UnixTerminal instance as _socket (see node_modules/node-pty/lib/unixTerminal.js). That socket wraps the fd via libuv. The fd only gets closed when the socket is destroyed, and the socket is only destroyed by one of:
UnixTerminal.destroy()— calls_close(), then_socket.destroy(), then_writeStream.dispose().- The socket's own
'close'/'error'event firing, which hits theself._socket.on('close', ...)wiring inside node-pty's constructor.
None of our current teardown paths reliably trigger either. Specifically:
forceKillvia rawprocess.kill(pid, 'SIGKILL')(pty-subprocess.ts:159-168) kills the child but does nothing to the libuv socket on the parent side. The slave fd closing in the kernel does not, by itself, cause the parent's read-side socket to emit'close'on all platforms / node versions — and even when it does, it races GC.dead = trueafter a thrown native error (pty-subprocess.ts:127-157) leaves the_socketfully alive. We just stop touchingproc; the read stream keeps the fd.- Session
dispose()/handleSubprocessExit(session.ts:189-228,session.ts:250-271) never call anything that reaches_socket.destroy(). They kill the child and move on.
So the fd leaks until the JS wrapper is GC'd and libuv finalizes the stream — which may be much later, or never, within the daemon's lifetime. Evidence from the reported case is consistent with this: 269 ptmx − 13 live ≈ 256 orphaned fds from terminals that were opened and closed during the daemon's lifetime, which matches a few days of create/kill churn at ~50-100 terminal ops/day.
node-pty already exposes the fix: UnixTerminal.prototype.destroy (unixTerminal.js:219) and WindowsTerminal.prototype.destroy (windowsTerminal.js:141). We just never call it.
Goals
- After a PTY terminates (by any path — natural exit, kill, force-kill, native throw, disposal), the master fd is released to the OS on the same tick as teardown.
- The fix covers every code path in the repo that spawns
node-pty: the daemon, the legacy local provider, and the SSH relay. - Zero regressions to the
deadflag pattern (pty-subprocess.ts:119-122) that preventsNapi::Errorfrom killing the daemon. - No renderer-visible change. Public IPC / RPC contracts are untouched.
- Measurable: a repeatable test harness spawns N PTYs, terminates them via each exit path, and asserts the fd count returns to baseline.
Non-goals
- Fixing the orphaned-daemon issue. Handled in auto-kill-old-daemons.md.
- Killing daemon-side sessions when a worktree is deleted. Separate follow-up.
- Upgrading or replacing
node-pty. We are calling node-pty's existingdestroy()— no library change needed. - Adding a generic "fd leak monitor" watchdog. Observability belongs in a separate effort (see Related work).
Design decisions
Call node-pty's destroy() on every teardown path
The fix is exactly that. Each provider already wraps the raw IPty in its own small handle/struct. Add a single dispose() entry point on those wrappers whose body does:
try {
;(proc as unknown as { destroy?: () => void }).destroy?.()
} catch {
/* swallow — already torn down, or native-side error we can't recover from */
}
That is the primary mechanism. destroy() is what drives _close() → _socket.destroy() → _writeStream.dispose() inside node-pty, which is what releases the master fd. Everything else the wrapper needs (idempotency, callback nulling, SIGKILL fallback for unresponsive children) is a local concern layered around that single call.
Why not reinvent destroy()
An earlier draft of this design proposed a hand-rolled body: null callbacks + raw SIGKILL + proc.kill() in try/catch. That was wrong. None of those steps touch _socket, which is the thing holding the fd. The only reliable trigger is the close path node-pty already ships — UnixTerminal.prototype.destroy on POSIX, WindowsTerminal.prototype.destroy on ConPTY. Duplicating that logic in our wrappers would (a) not work on POSIX anyway because _socket is private and (b) drift from upstream on any future node-pty update. Forward to destroy(), layer idempotency and error-swallowing on top, stop.
Shared handle contract
Each provider exposes a dispose entry point with the same semantics: idempotent, synchronous, throws never. The daemon already has a SubprocessHandle type that is the right place to hang it; the local provider and relay each have equivalent internal records (ptyProcesses / ManagedPty) and get a small helper function that wraps the same destroy() call plus their provider-specific bookkeeping (listener disposal, map cleanup).
// src/main/daemon/session.ts
export type SubprocessHandle = {
pid: number
write(data: string): void
resize(cols: number, rows: number): void
kill(): void
forceKill(): void
signal(sig: string): void
onData(cb: (data: string) => void): void
onExit(cb: (code: number) => void): void
/** Release the native PTY handle via node-pty's own destroy() path.
* Idempotent. Safe to call after exit. Called by Session on every teardown
* path (natural exit, kill, force-kill, native throw, session dispose). */
dispose(): void
}
Daemon: pty-subprocess.ts
Track a disposed flag. dispose():
- If already disposed, return.
- Mark
disposed = trueanddead = true. - Null the JS-side
onDataCb/onExitCbreferences so the wrapper stops fanning out anything after destroy. - On POSIX only, neutralize
proc.killon this instance. Replace it with a no-op before step 5. This closes the pid-recycle SIGHUP hazard described below. Windows is exempt becauseWindowsTerminal.destroyis a call tokill()— neutralizing it turns destroy() into a no-op and leaks the ConPTY agent. - Call
proc.destroy()inside try/catch. On Unix this runs_close()→_socket.destroy()→_writeStream.dispose(); on Windows it defers to the ConPTY agent's close via_deferNoArgs(_this.kill())(windowsTerminal.js:141-146).
dispose(): void {
if (disposed) return
disposed = true
dead = true
onDataCb = null
onExitCb = null
// Why: node-pty's UnixTerminal.destroy() registers `_socket.once('close', () => this.kill('SIGHUP'))`
// (unixTerminal.js:219-229). On the dispose-while-alive path (e.g. daemon shutdown
// of an active session), SIGTERM has already been sent. The socket close fires
// asynchronously; by then the child may have exited and its pid been recycled
// to an unrelated process. Without this neutralization, SIGHUP can be delivered
// to a Chrome tab, editor, or other user process — silent cross-app corruption.
// `_socket.destroy()` still releases the fd; only the dangerous SIGHUP is removed.
//
// Platform guard: WindowsTerminal.destroy() implements the ConPTY close by
// CALLING `this.kill()` via `_deferNoArgs` (windowsTerminal.js:141-146). If we
// neutralize `kill` on Windows, destroy() becomes a no-op and the ConPTY
// agent leaks. The SIGHUP hazard is POSIX-only, so the neutralization is too.
if (process.platform !== 'win32') {
;(proc as unknown as { kill?: (sig?: string) => void }).kill = () => {}
}
try {
;(proc as unknown as { destroy?: () => void }).destroy?.()
} catch {
/* swallow — already torn down, or native-side error we can't recover from */
}
}
kill neutralization lives on this proc instance only — the wrapper's exposed kill() / forceKill() entry points call process.kill(proc.pid, sig) directly (pty-subprocess.ts:149-175), which is unaffected. Callers that want to send SIGTERM/SIGKILL still can; what they can't do after dispose is have node-pty's internal close listener deliver an unexpected SIGHUP against a stale pid.
No extra SIGKILL is needed from dispose(): forceKill remains available as a separate entry point for the "we need the child gone now" case. Callers that want both (Session on timeout) call forceKill() then dispose().
Daemon: Session / TerminalHost wiring
Session.dispose() (session.ts:189-228) and Session.handleSubprocessExit() (session.ts:250-271) each call this.subprocess.dispose():
handleSubprocessExit: call it immediately after updating_state = 'exited'and before fan-out toattachedClients. try/catch around the call — a throwing dispose must not prevent exit-code delivery.dispose(): call it at the end, afteremulator.dispose()and after the attached-client fanout.forceDispose()(session.ts:319-347): same asdispose().
subprocess.dispose() is idempotent because handleSubprocessExit and the outer dispose() can both fire for the same session (manual kill → child exits → handleSubprocessExit runs → later Session.dispose runs).
TerminalHost.dispose() (terminal-host.ts:215-240) already kills every session. Replace the session.kill() call in the shutdown loop with a new session.forceKillAndDisposeSubprocess() method, so daemon shutdown reaps stubborn children (SIGKILL) AND releases the PTY master fd synchronously — without depending on the 5s KILL_TIMEOUT_MS eventually calling forceDispose:
for (const [, session] of this.sessions) {
session.detachAllClients()
session.forceKillAndDisposeSubprocess() // synchronous; bypasses the 5s timer
}
// Preserve existing post-loop cleanup: sessions.clear(), killedTombstones.clear(),
// and any cleanup callbacks fire unchanged after the loop. Only the per-session
// kill() → forceKillAndDisposeSubprocess() substitution is new.
Why forceKillAndDisposeSubprocess instead of kill() + disposeSubprocess(). Today's shutdown path is session.kill() (SIGTERM, start 5s timer) → if the child ignores SIGTERM, forceDispose() eventually fires and sends SIGKILL. That 5s fallback is load-bearing: shells with traps, hung compilers, and long-running processes with signal handlers routinely ignore SIGTERM. The earlier draft dropped killTimer during disposeSubprocess, which would close the fd but leave the child orphaned at the OS level — worse than today's behavior. forceKillAndDisposeSubprocess explicitly sends SIGKILL (via subprocess.forceKill()) before releasing the fd, so stubborn children are still reaped synchronously:
// Public: orderly-shutdown path. Force-kills the child (SIGKILL is not
// ignorable), then releases the PTY master fd synchronously. Used only by
// TerminalHost.dispose() — renderer reconnects cold after daemon exit.
forceKillAndDisposeSubprocess(): void {
// Why: forceKill before the subprocess dispose. disposeSubprocess below
// neutralizes node-pty's kill() on POSIX, which is what the internal SIGHUP
// close listener would call. forceKill uses process.kill(pid, 'SIGKILL')
// directly (pty-subprocess.ts:159-168) — unaffected by the neutralization,
// because it does not go through proc.kill. SIGKILL is not ignorable; any
// child that would have survived the 5s timer is reaped immediately.
try {
this.subprocess.forceKill()
} catch {
/* swallow — child may already be gone */
}
this.#teardownSubprocess()
}
disposeSubprocess() is a tight helper — it flips the session to the terminal state, cancels any pending timers, and forwards to the subprocess dispose. It deliberately does not fan out onExit to attached clients, because TerminalHost.dispose() is the orderly-shutdown path and the renderer will reconnect cold.
Shared #teardown helper. Both Session.dispose() and Session.disposeSubprocess() mutate the same state (_disposed, _state, killTimer, shellReadyTimer) and forward to this.subprocess.dispose(). Route them through a single private helper #teardownSubprocess() so the state transition is defined exactly once — two dispose paths that drift on which flag gets flipped would be a merge hazard (a future change to one that forgets the other silently drops exit events):
// Session — shared private helper, called by both public paths.
#teardownSubprocess(): void {
if (this._disposed) return
this._disposed = true
// Note: `_state = 'exited'` is NOT set here — the outer caller (Session.dispose,
// Session.forceDispose) is responsible for the state transition AFTER capturing
// any invariants that depend on the pre-flip value. See the `wasTerminating`
// capture in Session.dispose for the load-bearing example.
if (this.killTimer) { clearTimeout(this.killTimer); this.killTimer = null }
if (this.shellReadyTimer) { clearTimeout(this.shellReadyTimer); this.shellReadyTimer = null }
try {
this.subprocess.dispose()
} catch (err) {
// Why: dispose() is documented never to throw, but if it does we must not
// prevent callers from completing their own cleanup (fanout, map removal).
console.warn('[Session] subprocess.dispose() threw:', err)
}
}
// Public: orderly-shutdown path. Does NOT broadcast onExit to clients; the
// renderer reconnects cold after TerminalHost.dispose(). Sets _state = 'exited'
// after teardown because this path has no pre-flip invariants to preserve.
disposeSubprocess(): void {
this.#teardownSubprocess()
this._state = 'exited'
}
// Public: existing Session.dispose() — MUST capture `wasTerminating` BEFORE
// calling #teardownSubprocess, because the check depends on `_state !== 'exited'`
// being the PRE-flip value. Forgetting this check would orphan processes that
// ignored SIGTERM (the `_isTerminating && _state !== 'exited'` guard at
// session.ts:200 is the only path that calls `subprocess.forceKill()` in the
// dispose-while-terminating case).
dispose(): void {
// Why: captured BEFORE the `_state = 'exited'` flip at line 230. This check
// guards the "dispose while kill() was already in flight" case — if true,
// the child hasn't reaped yet and we need to forceKill it here (the 5s
// killTimer is also about to be cleared by #teardownSubprocess). Matches
// the existing invariant at session.ts:200. Do not move this line below
// #teardownSubprocess or the `_state = 'exited'` assignment.
const wasTerminating = this._isTerminating && this._state !== 'exited'
const clientsToNotify = wasTerminating ? this.attachedClients.slice() : []
if (wasTerminating) {
// Existing forceDispose semantics — child hasn't reaped yet, force it now.
try { this.subprocess.forceKill() } catch { /* already dead */ }
this._exitCode = -1
this._isTerminating = false
}
this.#teardownSubprocess()
this._state = 'exited'
this.attachedClients = []
this.preReadyStdinQueue = []
this.postReadyFlushGate.clear()
this.emulator.dispose()
for (const client of clientsToNotify) {
client.onExit(-1)
}
}
Single source of truth for the state transition. If we ever need to add a new piece of tear-down state, one edit keeps both paths in sync. The asymmetry (_state = 'exited' lives in the public methods, not the helper) is deliberate: dispose() has a pre-flip invariant (wasTerminating) to preserve, disposeSubprocess() doesn't. Forcing both through a single helper that flipped _state would reintroduce the ordering bug.
forceDispose() must also route through the helper. forceDispose fires from the 5s killTimer when a SIGTERM'd child refused to exit (session.ts:319-347). This is the exact kill-timeout path this fd-leak fix targets — if forceDispose doesn't call subprocess.dispose(), the ptmx fd leaks on every force-kill. The current body flips _disposed = true directly at session.ts:324, which would short-circuit a later Session.dispose() call from TerminalHost tombstone cleanup at #teardownSubprocess's if (this._disposed) return guard, silently skipping subprocess.dispose().
// Private: fires from the 5s killTimer when SIGTERM was ignored.
// `_state === 'exited'` already short-circuits above; below that guard the
// session is still running, so the fd is still open. MUST release it.
private forceDispose(): void {
if (this._state === 'exited') {
return
}
// Why: unlike dispose(), forceDispose has no pre-flip invariants to capture.
// The caller (killTimer) has no attached-client fanout obligation beyond
// what the helper + the existing client loop below cover. Route the
// subprocess tear-down through the shared helper so `proc.destroy()` runs
// exactly once and `_disposed` is flipped in the same transition.
try { this.subprocess.forceKill() } catch { /* already dead */ }
this._exitCode = -1
this._isTerminating = false
this.#teardownSubprocess() // ← sets _disposed = true, calls subprocess.dispose()
this._state = 'exited'
const clients = this.attachedClients
this.attachedClients = []
this.preReadyStdinQueue = []
this.postReadyFlushGate.clear()
this.emulator.dispose()
for (const client of clients) {
client.onExit(-1)
}
}
Order matters. forceKill() must run BEFORE #teardownSubprocess() — the helper's subprocess.dispose() neutralizes proc.kill on POSIX, but subprocess.forceKill() uses process.kill(proc.pid, 'SIGKILL') directly and is unaffected by that neutralization. If we reorder, nothing changes functionally, but the explicit order documents the invariant.
After session.forceKillAndDisposeSubprocess(), the 5s killTimer started by any prior kill() is cleared, so forceDispose cannot fire against an already-disposed subprocess. Even if the timer somehow survived the clear, _state is now 'exited' and forceDispose's early-return guard (session.ts:320) would short-circuit it.
LocalPtyProvider
local-pty-provider.ts stores each spawned IPty in a module-level ptyProcesses map and has two teardown sites: shutdown(id) (single PTY) and killAll() / safeKillAndClean() (bulk). Both currently call proc.kill() and delete the map entry.
Change: add a destroyPtyProcess(proc) helper. Same shape as disposeManagedPty in the relay: on POSIX, neutralize proc.kill before calling destroy() to close the same SIGHUP-to-recycled-pid hazard; on Windows, skip the neutralization because WindowsTerminal.destroy calls kill() internally. Invoke the helper at the end of safeKillAndClean (after listener disposal, after proc.kill()), at the end of shutdown (same spot), and in the natural-exit onExit path after clearPtyState(id).
function destroyPtyProcess(proc: IPty): void {
// Why: same SIGHUP-to-recycled-pid hazard as the daemon and relay paths.
// UnixTerminal.destroy() registers `_socket.once('close', () => this.kill('SIGHUP'))`.
// LocalPtyProvider's own kill() has already fired; if the close listener
// delivers a trailing SIGHUP after pid recycling, it lands on a random user
// process. Local machines recycle pids fast — regression risk is highest here.
// Windows exempt: WindowsTerminal.destroy IS a kill() call via _deferNoArgs.
if (process.platform !== 'win32') {
;(proc as unknown as { kill?: (sig?: string) => void }).kill = () => {}
}
try {
;(proc as unknown as { destroy?: () => void }).destroy?.()
} catch {
/* swallow — already torn down */
}
}
The natural-exit onExit invocation is load-bearing: without it, a shell that exits cleanly (the common case) never releases its ptmx fd until the next GC. onExit is also the only path that runs without an external kill having been issued, so the neutralization is defensive against future reorderings — today proc.kill is a no-op after the child is reaped, but the kernel may still emit the close event.
Relay: relay/pty-handler.ts
PtyHandler stores each IPty on a ManagedPty record. Teardown happens in five places:
shutdown(params)— SIGTERM + 5s SIGKILL fallback, or SIGKILL immediately whenimmediate === true.- The
onExitwiring insidewireAndStore— deletes the map entry. spawn()'s stale-context path (pty-handler.ts:144-154) — client reconnected before the PTY response was delivered, so the PTY is orphaned and gets SIGTERM + 5s SIGKILL fallback.dispose()— loops every managed pty and shuts it down.sendSignal()— public entry point that forwards arbitrary signals; must keep working after dispose is queued.
Change (ordering matters):
onExitwiring: after the map-delete, calldisposeManagedPty(managed). This is the natural-exit path — the child has already exited and been reaped. Neutralizingpty.killhere is defensive: any trailing invocation from the socket close listener is no-op'd regardless of pid-recycle timing. The fd is released synchronously.shutdown(params)withimmediate === true: aftermanaged.pty.kill('SIGKILL'), calldisposeManagedPty(managed). SIGKILL has already reaped the child.shutdown(params)graceful path (SIGTERM): do not calldestroy()immediately after the SIGTERM send. The 5skillTimerand the naturalonExitalready cover fd release via step 1. Callingdestroy()right after SIGTERM collapses the graceful-shutdown window and risks interrupting shellEXITtraps or cleanup hooks that run between SIGTERM and shell exit. If thekillTimerfires (SIGKILL fallback), the SIGKILL reaps the child andonExit(step 1) callsdisposeManagedPtyon its own — do not add a redundant call inside the killTimer closure. Thedisposedguard makes a redundant call harmless but wiring it in both places is a merge hazard.spawn()stale-context killTimer: this path (pty-handler.ts:149-153) already sends SIGTERM then SIGKILL at 5s if the PTY wasn't acked. The relevant teardown path isonExit(step 1), which now callsdisposeManagedPty. No code change needed in the stale-context branch itself — the onExit wiring inwireAndStoreruns for this PTY the same as for any other. Confirmed covered.dispose()loop: calldisposeManagedPtyfor each active managed pty AFTERmanaged.pty.kill('SIGTERM'). This is the relay-shutdown path — we accept that any in-flight shell cleanup is cut short because the relay process itself is exiting.
The common shape, extracted into a helper:
function disposeManagedPty(managed: ManagedPty): void {
if (managed.disposed) return
managed.disposed = true
// Why: clear any pending 5s SIGKILL fallback timer. If graceful-shutdown
// armed a killTimer and the child then exited cleanly (firing onExit →
// disposeManagedPty), the timer would otherwise fire later and attempt
// pty.kill('SIGKILL') on an already-disposed instance. The ptys.has(id)
// guard inside the timer short-circuits today, but symmetry is clearer.
if (managed.killTimer) {
clearTimeout(managed.killTimer)
managed.killTimer = undefined
}
// Why: same SIGHUP-to-recycled-pid hazard as the daemon fix — neutralize
// node-pty's kill on the instance before calling destroy(). The relay
// typically runs on Linux remote hosts where pid recycling is fast.
// Windows exempt: WindowsTerminal.destroy calls kill() internally, so
// neutralizing it turns destroy() into a no-op.
if (process.platform !== 'win32') {
;(managed.pty as unknown as { kill?: (sig?: string) => void }).kill = () => {}
}
try {
;(managed.pty as unknown as { destroy?: () => void }).destroy?.()
} catch {
/* swallow */
}
}
disposed flag on ManagedPty. The flag is new — add it to the ManagedPty type. Purpose: prevent disposeManagedPty from running twice (both onExit and shutdown can fire for the same PTY), and — more importantly — let sendSignal, writeData, and other pre-existing entry points short-circuit if the caller tries to use a disposed PTY:
private async sendSignal(params: Record<string, unknown>): Promise<void> {
// ... existing signal validation ...
const managed = this.ptys.get(id)
if (!managed || managed.disposed) {
throw new Error(`PTY "${id}" not found`)
}
managed.pty.kill(signal) // ← on POSIX this is neutralized-to-no-op only
// AFTER disposeManagedPty runs. The early return
// above means we never reach this line post-dispose.
}
Without the disposed guard, a sendSignal('SIGTERM') call landing after onExit → disposeManagedPty has neutralized managed.pty.kill would silently succeed (return success, do nothing). The disposed check converts the silent failure into the existing "not found" error, which is what callers already handle.
The graceful-SIGTERM path of shutdown remains unchanged (SIGTERM + 5s killTimer that sends SIGKILL). Fd release on that path happens either via the onExit hook (step 1) when the shell exits cleanly, or via the killTimer → SIGKILL → onExit chain if SIGTERM was ignored. Under no circumstance does shutdown's graceful branch synchronously call destroy().
Why not just replace node-pty
Tempting, and probably correct long-term. Out of scope here: node-pty's API surface is deeply woven into the daemon, LocalPtyProvider, and the relay. Swapping it is a multi-week effort with its own regression surface. This bug is a one-line miss against an API the library already ships — fix that first.
Why not a daemon-level fd watchdog
A periodic "count my open fds, restart if over threshold" watchdog would mask the bug, not fix it. It also introduces the exact timer-based lifecycle the orphan-daemon design explicitly rejected for the same reasons (cold-restore instead of warm-reattach, false positives against live work). Observability yes, auto-restart no.
Interaction states
Because this fix sits below the IPC layer, the renderer sees no state changes. All user-visible interaction states are identical to today:
| Trigger | User-visible state | Behind the scenes (new) |
|---|---|---|
| Click "Close terminal" | Tab closes immediately | terminal:kill IPC → Session.kill() → handleSubprocessExit → subprocess.dispose() → proc.destroy() → fd released |
Shell exits naturally (exit / Ctrl-D) |
Tab shows exit code | handleSubprocessExit → subprocess.dispose() → proc.destroy() → fd released |
| Daemon is killed mid-session | Renderer reconnects cold on next app launch | On orderly shutdown: TerminalHost.dispose() loop → per-session forceKillAndDisposeSubprocess() — SIGKILL first (stubborn children reaped), then proc.destroy() releases the fd synchronously. Does not depend on the 5s KILL_TIMEOUT_MS. |
Child hits Napi::Error race |
Silent (today) | Silent (same) — next teardown trigger reaches subprocess.dispose(), which forwards to proc.destroy() regardless of dead |
Session.kill hits KILL_TIMEOUT_MS |
User sees "force-closed" log entry (unchanged) | forceDispose → forceKill + subprocess.dispose() → fd released |
| SSH worktree session closed | Tab closes immediately | Relay PtyHandler.shutdown → SIGTERM + managed.pty.destroy() on exit → fd released on remote host |
Data flow
Renderer Main (IPC) Daemon (TerminalHost) node-pty
│ │ │ │
│ terminal:kill │ │ │
├────────────────────────▶│ RPC: kill(sessionId) │ │
│ ├───────────────────────────▶│ Session.kill() │
│ │ ├─ subprocess.kill() ─────▶│ (SIGTERM)
│ │ │ │
│ │ │◀── onExit(code) ─────────│
│ │ │ handleSubprocessExit │
│ │ │ ├─ state='exited' │
│ │ │ ├─ subprocess.dispose()─┼─▶ destroy() →
│ │ │ │ │ _close() +
│ │ │ │ │ _socket.destroy() +
│ │ │ │ │ _writeStream.dispose()
│ │ │ │ │ → ptmx fd released ✔
│ │ │ └─ fanout onExit │
│ │◀─── event: 'exit' ─────────│ │
│◀── terminal:exit ───────│ │ │
Failure paths
Natural exit. Child dies, proc emits onExit → handleSubprocessExit → subprocess.dispose() → proc.destroy() → _socket.destroy() → fd released, same tick.
Explicit kill (Session.kill). SIGTERM → child exits → same path as above.
Kill timeout. Child ignores SIGTERM → KILL_TIMEOUT_MS elapses → forceDispose → subprocess.forceKill() (SIGKILL) + subprocess.dispose() (proc.destroy()). Fd released within 5s worst case.
Native throw. proc.write/resize/kill throws Napi::Error → dead = true. Session is wedged but still reachable. The next teardown trigger (user close, daemon shutdown, worktree close) reaches subprocess.dispose(), which forwards to proc.destroy(). The destroy() body itself is try/catch-wrapped — if the native side is already half-torn-down and throws, we swallow; the socket side still gets destroyed by any prior event.
Daemon shutdown. TerminalHost.dispose() iterates sessions and calls session.forceKillAndDisposeSubprocess() synchronously. The ordering is: SIGKILL (subprocess.forceKill() → process.kill(pid, 'SIGKILL')) → subprocess.dispose() → proc.destroy(). SIGKILL is not ignorable, so any child that would have survived the normal 5s KILL_TIMEOUT_MS SIGTERM window is reaped before the fd release. On POSIX, every ptmx fd is released before the daemon process exits. On Windows, WindowsTerminal.destroy defers via _deferNoArgs — dispose is enqueued but may not run synchronously; this is acceptable because ConPTY is not subject to the POSIX ptmx cap (see Risks).
Local provider (non-daemon). LocalPtyProvider.shutdown(id) / killAll() / natural onExit all route through the new destroyPtyProcess(proc) helper, same proc.destroy() call. Fd released symmetrically with the daemon path.
Relay (SSH). PtyHandler.shutdown and PtyHandler.dispose both call managed.pty.destroy() after the existing kill. On relay shutdown the loop is synchronous, so ptmx fds in the remote host's relay process are released before the relay exits.
Architectural fit
┌─ Daemon (forked) ───────────────┐ ┌─ Main process ─────────────┐ ┌─ Relay (remote host) ────┐
│ TerminalHost │ │ LocalPtyProvider │ │ PtyHandler │
│ └─ Session │ │ └─ ptyProcesses map │ │ └─ ManagedPty map │
│ └─ SubprocessHandle │ │ (proc.destroy ←NEW)│ │ (pty.destroy ←NEW)│
│ └─ dispose() ←NEW │ │ │ │ │
│ └─ proc.destroy()│ └────────────────────────────┘ └──────────────────────────┘
└─────────────────────────────────┘ │ │
│ └───────────┬─────────────────────┘
└───────────────┐ │
▼ ▼
┌──────────────────────────────────────────────┐
│ node-pty destroy() │
│ → _close() → _socket.destroy() → fd freed │
└──────────────────────────────────────────────┘
Three providers, one shared fix: forward the local dispose call to node-pty.destroy(). No new layer, no new dependency direction. Each provider already owns its handle lifecycle — we are filling a gap in three existing chains, not creating them.
Implementation plan
Files to change
| File | Change |
|---|---|
src/main/daemon/session.ts |
Add dispose(): void to SubprocessHandle type. Add forceKillAndDisposeSubprocess() + disposeSubprocess() public methods. Add private #teardownSubprocess() helper that does NOT flip _state (see dispose section). Call subprocess.dispose() from handleSubprocessExit, dispose, forceDispose. Preserve the wasTerminating = this._isTerminating && this._state !== 'exited' invariant by capturing it BEFORE any state flip in the refactored dispose. |
src/main/daemon/pty-subprocess.ts |
Implement dispose() on the returned handle. Idempotent. Nulls callbacks, sets dead = true, gates the proc.kill = noop neutralization on process.platform !== 'win32', forwards to (proc as unknown as { destroy?: () => void }).destroy?.() in try/catch. |
src/main/daemon/terminal-host.ts |
In dispose(), call session.forceKillAndDisposeSubprocess() instead of session.kill(). Preserves the SIGKILL-fallback behavior without depending on the 5s timer. |
src/main/providers/local-pty-provider.ts |
Add destroyPtyProcess(proc) helper: gates proc.kill = noop neutralization on POSIX, then calls proc.destroy() in try/catch. Invoke from safeKillAndClean, shutdown, and the onExit path (after clearPtyState). |
src/relay/pty-handler.ts |
Add disposed: boolean field to ManagedPty. In wireAndStore's onExit (after map delete) and in dispose() loop, call disposeManagedPty(managed). In shutdown immediate-path (after SIGKILL), call disposeManagedPty. In the 5s killTimer SIGKILL fallback (graceful path and spawn stale-context path), also call disposeManagedPty. Do not call it in the graceful SIGTERM branch. Guard sendSignal, writeData, resize, getCwd, getInitialCwd, clearBuffer, hasChildProcesses, getForegroundProcess, attach with managed.disposed checks (treat disposed as "not found"). |
src/main/daemon/session.test.ts |
Add dispose: vi.fn() to createMockSubprocess. |
src/main/daemon/terminal-host.test.ts |
Add dispose: vi.fn() to createMockSubprocess. |
src/main/daemon/terminal-host-startup.test.ts |
Add dispose: vi.fn() to mockSubprocess. |
src/main/daemon/production-launcher.test.ts |
Add dispose: vi.fn() to createMockSubprocess. |
src/main/daemon/daemon-pty-provider.test.ts |
Add dispose: vi.fn() to createMockSubprocess. |
src/main/daemon/reattach-snapshot.test.ts |
Add dispose: vi.fn() to createMockSubprocess. |
src/main/daemon/__tests__/pty-fd-leak.test.ts (new) |
Integration test for the daemon path: spawn N sessions, tear them down via each path, assert fd count returns to baseline. |
src/main/providers/__tests__/local-pty-fd-leak.test.ts (new) |
Same test, targeting LocalPtyProvider directly. |
src/relay/__tests__/pty-fd-leak.test.ts (new) |
Same test, targeting PtyHandler directly. |
Testing plan
The hard part of this fix is not the code — it's proving the leak is closed in all three providers. Two layers:
1. Unit-level handle behaviour
Per provider, for each exit path (kill, forceKill, native throw simulated by calling dispose after an artificial dead state, natural exit), assert:
proc.destroy()is called exactly once per PTY (spy on the mockIPty).- Calling the provider's dispose a second time is a no-op (idempotency).
- After dispose, subsequent
write/resize/killare no-ops and do not throw. - No
Napi::Errorsurfaces from any follow-on method call.
2. Integration-level fd count
One test file per provider (src/main/daemon/__tests__/pty-fd-leak.test.ts, src/main/providers/__tests__/local-pty-fd-leak.test.ts, src/relay/__tests__/pty-fd-leak.test.ts), same shape:
baseline = countPtmxFds(process.pid)
for i in 1..50:
pty = provider.spawn(...)
provider.shutdown(pty.id) // or forceKill / dispose / let it exit naturally
await exit event
after = countPtmxFds(process.pid)
assert after === baseline
Loop once per exit path per provider.
Platform-specific countPtmxFds implementation:
- macOS:
lsof -p <pid> -Ffn→ parse as (fd, name) pairs → count pairs whose fd is numeric (notcwd/txt/rtd) AND whose name is exactly/dev/ptmx. The-Ffnmachine-readable mode emits one field per line prefixed withf(fd) orn(name); a naivestartsWith('n')filter that doesn't cross-check the precedingfrow false-matches on non-fd rows whose NAME happens to be/dev/ptmx. Never usegrep ptmxunqualified — it false-matches on env vars and any path containing the substring. - Linux: iterate
/proc/<pid>/fd/*,readlinkeach entry, match against the regex/^(\/dev\/ptmx$|\/dev\/pts\/ptmx$|anon_inode:\[?ptmx\]?$)/. The master pty fd does not show as/dev/ptmxon modern Linux kernels — depending on the pty backend and kernel version,readlinkreturnsanon_inode:[ptmx](devpts backend on many distros) or/dev/pts/ptmx. A naivegrep /dev/ptmxyields zero matches and the test passes vacuously — zero regression coverage instead of real coverage. CI runs onubuntu-latest; this matters. - Windows: skip entirely (ConPTY doesn't expose ptmx; rely on the unit-level
destroy()spy).
Mandatory beforeAll counter smoke. Before the first assertion, spawn one real PTY, measure countPtmxFds, kill it, measure again, and assert beforeSpawn < afterSpawn and afterKill <= beforeSpawn. If either check fails, the counter itself is broken on this platform — skip the whole suite with a clear message (skip('ptmx counter not functional on this platform')). Otherwise, the fd-count assertions could go green on a broken counter and we'd think the fix landed when it didn't.
beforeAll(async () => {
const baseline = countPtmxFds(process.pid)
const probe = pty.spawn('/bin/sh', ['-c', 'sleep 30'], { /* ... */ })
await new Promise((r) => setTimeout(r, 100))
const withProbe = countPtmxFds(process.pid)
probe.kill('SIGKILL')
await new Promise((r) => setTimeout(r, 200))
const afterKill = countPtmxFds(process.pid)
if (!(withProbe > baseline) || !(afterKill <= baseline)) {
// Why: if the counter can't even observe spawn/kill deltas on a known-good
// PTY, it can't observe the leak we're trying to prove closed. Vacuously
// passing assertions would give false confidence. Skip instead.
return describe.skip('ptmx counter smoke failed; skipping fd-leak suite')
}
})
These tests spawn 50 real subprocesses each and must be gated behind an env-var skip (not a vitest tag — vitest 4 doesn't ship built-in tag filtering in this repo's config). Use describe.skipIf(!process.env.RUN_SLOW_TESTS). Every spawn inside the loop must be wrapped in try/finally that force-kills the child regardless of assertion outcome — otherwise a mid-loop failure leaks the remaining subprocesses into the test runner, corrupting the next suite's baseline.
3. Manual verification
Reproduce the reported 269/13 case locally:
- Start a fresh daemon.
- Baseline:
lsof -p $(pgrep -f daemon-entry) | grep -c ptmx→ small (one per live session). - Churn: open and close 50 terminals via the UI.
- Verify the count returns to baseline within seconds. Before the fix, it stays at ~50.
- Repeat against a non-daemon worktree (LocalPtyProvider path) and against an SSH worktree (relay path).
Rollout
No flag needed. The change is pure bug-fix with strictly smaller resource footprint. Ship with the orphan-daemon fix in the same release — they are complementary and together close the full leak story.
Regression surface
| Regression | Mitigation |
|---|---|
dispose() throws, breaking exit-code fanout to clients |
Wrap every dispose call site in try/catch. destroy() call itself is also in try/catch inside the wrapper. |
proc.destroy() is not present on an older/forked node-pty build |
Optional chain: (proc as unknown as { destroy?: () => void }).destroy?.(). Current pinned version has it on both UnixTerminal and WindowsTerminal; tests assert it's called. |
| Double-dispose double-frees the native handle | Idempotency guard (if (disposed) return) at the top of dispose. node-pty's own destroy() is also safe to call twice — _close() is idempotent (it replaces write/end with no-ops and flips _writable/_readable to false the first time; the second call overwrites no-ops with no-ops). _socket.destroy() and _writeStream.dispose() are both idempotent per Node stream semantics. |
dead flag pattern regresses, Napi::Error kills daemon |
All existing dead-guarded methods stay exactly as-is. dispose() sets dead = true and then calls destroy(); it adds to the pattern, does not replace it. |
| Newly-nulled onData callback drops final data burst | Session already fans out synchronously in handleSubprocessData before exit. Nulling after exit is safe. |
Neutralizing proc.kill on Windows turns destroy() into a no-op (ConPTY leak) |
WindowsTerminal.destroy calls kill() via _deferNoArgs — we gate neutralization on process.platform !== 'win32'. Verified in pty-subprocess.ts and local-pty-provider.ts and relay/pty-handler.ts helpers. Test: mock process.platform = 'win32'; assert proc.kill is NOT replaced with noop; assert destroy() is still called. |
Shared #teardownSubprocess helper flips _state before callers capture wasTerminating, orphaning stubborn children |
Helper explicitly does NOT flip _state. Each public method (dispose, disposeSubprocess, forceKillAndDisposeSubprocess) owns its own _state = 'exited' line and captures any pre-flip invariants before calling the helper. Test: session with _isTerminating=true, _state='running' calls dispose(); assert subprocess.forceKill() is called before subprocess.dispose(). |
sendSignal / writeData / resize silently no-op after relay onExit neutralized managed.pty.kill |
Every public entry point on PtyHandler that touches a ManagedPty checks managed.disposed first and treats disposed as "not found" (existing error path). Converts silent failure to an explicit error callers already handle. |
TerminalHost.dispose() drops 5s SIGKILL fallback for stubborn children |
New forceKillAndDisposeSubprocess method explicitly sends SIGKILL via subprocess.forceKill() before releasing the fd. SIGKILL is not ignorable. Test: session whose child ignores SIGTERM; call forceKillAndDisposeSubprocess; assert child is reaped AND fd is released, both synchronously. |
| LocalPtyProvider delivers trailing SIGHUP to recycled pid | destroyPtyProcess(proc) mirrors the daemon and relay helpers: POSIX-guarded proc.kill = noop neutralization before destroy(). Covers natural-exit, safeKillAndClean, and shutdown paths. |
Risks
| Risk | Severity | Why acceptable |
|---|---|---|
destroy() doesn't actually release the fd |
Low | We are literally calling node-pty's canonical close path (_close() → _socket.destroy() → _writeStream.dispose()). Only way this is ineffective is a node-pty bug or a version regression — and the integration fd-count tests catch both before merge. |
| Idempotent dispose hides a real double-free bug | Low | Idempotency only skips work; it does not suppress errors. |
TerminalHost.dispose() synchronous loop blocks daemon shutdown |
Low | destroy() on each PTY is a handful of syscalls; 50 sessions ≈ single-digit ms. |
destroy() on Windows defers via _deferNoArgs |
Low | ConPTY never hit the ptmx cap (it's a POSIX resource). Windows dispose is correctness-for-symmetry, not a load-bearing leak fix. |
| The real leak is elsewhere (e.g., an fd leak outside the PTY code path) | Low | Integration tests directly measure fd reclamation across a full spawn/kill lifecycle. If the fix works there, it addresses the observed symptom. |
Related work (out of scope)
- Orphaned legacy daemons: auto-kill-old-daemons.md — complementary fix targeting daemon-level lifecycle.
- Worktree-delete does not kill sessions: separate follow-up. Today, deleting a worktree from the sidebar leaves its PTY sessions live in the daemon, which is a different leak vector.
- Daemon fd observability: expose an IPC
daemon:statshandler so Orca can surface fd count in a debug view. Would let us detect regressions in production rather than waiting for user reports. - Replace
node-pty: out of scope; see "Why not just replace node-pty" above.