Repository navigation
Commit a429991
authored
Fix Linux terminal fd leak: bump node-pty to close inherited fds (#1127)
**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)](#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`](microsoft/node-pty@52417d516))
| ❌ missing | ✅ included |
| `foregroundPid` accessor | ✅ (fork patch) | ✅ (same patch, now also
[PR #918](microsoft/node-pty#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](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`)._1 parent c28ffe9 commit a429991
3 files changed
Lines changed: 7 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
58 | 58 | | |
59 | 59 | | |
60 | 60 | | |
61 | | - | |
| 61 | + | |
62 | 62 | | |
63 | 63 | | |
64 | 64 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
26 | | - | |
| 26 | + | |
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| |||
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
0 commit comments