Skip to content

Fix Linux terminal fd leak: bump node-pty to close inherited fds#1127

Merged
srid merged 1 commit into
masterfrom
frozen-term
Jun 2, 2026
Merged

Fix Linux terminal fd leak: bump node-pty to close inherited fds#1127
srid merged 1 commit into
masterfrom
frozen-term

Conversation

@srid

@srid srid commented Jun 2, 2026

Copy link
Copy Markdown
Member

Long-lived Kolu sessions on Linux leak a /dev/ptmx master fd into every shell spawned after the first, and those leaked masters ride along into each shell's children — the suspected cause of the frozen split-terminal after a raw-mode TUI exits (#1076), plus a steady fd leak that pressures EMFILE in busy sessions. The root cause was a stale node-pty fork: our pin (fix/darwin-pty-fd-leak) branched 2025-12-21 and sat 127 commits behind upstream, predating the Linux fix for exactly this.

This repoints node-pty to a refreshed fork branch and drops a fork patch that has since landed upstream.

What changed

Before (fix/darwin-pty-fd-leak) After (feat/foreground-pid-dist)
Base upstream @ 2025-12-21 upstream main (2026-05-18)
Linux fd-close fix (52417d516) ❌ missing ✅ included
foregroundPid accessor ✅ (fork patch) ✅ (same patch, now also PR #918)
macOS /dev/ptmx fix (#882) fork patch dropped — now upstream
built lib/ for git install

Upstream 52417d516 closes inherited fds in the forkpty() child before exec (close_range(2)/proc/self/fd fallback). macOS was never affected (POSIX_SPAWN_CLOEXEC_DEFAULT), which is why this bug is Linux-only.

Verification

Measured on this branch with the rebuilt addon:

  • Leak gone: a 2nd shell spawned after the first now reports 0 leaked /dev/ptmx fds (it was 1 before — confirmed in a real Kolu split sub-terminal, whose shell held fd 29 -> /dev/ptmx, the main terminal's master).
  • foregroundPid intact: resolves to the foreground pgid (== pid), so pty-host's spawn-time sanity check still passes — agent/Dock foreground detection unaffected.
  • just check clean; native pty.node compiles under nix (node-gyp rebuild).

Fork housekeeping

The upstreaming of foregroundPid is microsoft/node-pty#918 (rebased here, mergeable, awaiting maintainer review). Once it merges, the fork shrinks to just the committed-lib/ packaging shim — and could be retired entirely if we build node-pty from source in the nix derivation.

Refs #1076.

Generated by Claude Code (model claude-opus-4-8).

Repoint node-pty from `fix/darwin-pty-fd-leak` (branched 2025-12-21, 127
commits behind upstream) to `feat/foreground-pid-dist` — upstream main +
our `foregroundPid` accessor (microsoft/node-pty#918) + committed lib.

This picks up upstream `52417d516` ("close inherited file descriptors in
child process on Linux"), which the old base predated. Without it, every
forkpty'd shell inherited the server's open fds — including prior
terminals' /dev/ptmx master fds. Verified in real kolu: a split
sub-terminal's shell held `fd 29 -> /dev/ptmx` (the main terminal's
leaked master). After the bump a 2nd shell shows 0 leaked ptmx, and the
`foregroundPid` accessor still resolves (kolu's spawn sanity-check).

macOS is unaffected (POSIX_SPAWN_CLOEXEC_DEFAULT); the now-upstream macOS
/dev/ptmx fix (#882) is dropped from the fork.

Refs #1076
@srid

srid commented Jun 2, 2026

Copy link
Copy Markdown
Member Author

Evidence

The fix is a resource-leak fix, so the proof is a measurement, not a pixel diff.

fd-leak: before → after (reproducible)

Spawn 6 PTYs sequentially (as the kolu server does), then count each shell's leaked /dev/ptmx master fds via /proc/<pid>/fd:

shell BEFORE — fix/darwin-pty-fd-leak (cc137d90) AFTER — feat/foreground-pid-dist (999e2bd)
1 0 0
2 1 0
3 2 0
4 3 0
5 4 0
6 5 0
TOTAL 15 0

Each shell N inherits the master fds of all N−1 terminals opened before it — and every process that shell spawns (agents, TUIs, build tools) inherits them too. The leak is O(n²) in a long session. With the bump: zero.

Cross-checked against the running app earlier in this investigation: a split sub-terminal's shell held fd 29 -> /dev/ptmx (PTMX_COUNT=1) — the main terminal's leaked master — reproducing upstream #710 ("every new terminal opens one more fd to /dev/ptmx").

foregroundPid preserved

The fork's load-bearing accessor still resolves on the rebased branch:

foregroundPid: 4190634 | pid: 4190634 | typeof: number

so pty-host's spawn-time sanity check passes and agent/Dock foreground detection is unaffected.

CI

justciall 22 nodes succeeded across both lanes (x86_64-linux on an ephemeral box, aarch64-darwin on the static host):

  • ci::e2e ✅ both platforms (Linux: 396/396 scenarios, 3221/3221 steps)
  • ci::nix ✅ both — node-pty native compiles in the nix sandbox
  • ci::pnpm-hash-fresh ✅ both — the refreshed pnpmDeps hash is reproducible
  • ci::unit, ci::smoke, ci::home-manager, ci::biome, ci::fmt, ci::surface-example-build ✅ both

Measurement harness: spawn N PTYs, readlink /proc/<shell-pid>/fd/*, count targets matching /dev/ptmx. Run on Linux (the affected platform; macOS uses POSIX_SPAWN_CLOEXEC_DEFAULT and never leaked).

Generated by Claude Code (model claude-opus-4-8).

@srid srid merged commit a429991 into master Jun 2, 2026
24 checks passed
@srid srid deleted the frozen-term branch June 2, 2026 15:50
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