fix(pty-daemon): patch node-pty /dev/ptmx leak that exhausts system ptys

Closed
SA
4mo ago
+219-16

What & why

Fixes #5699 — the root cause this time, found by reproducing the reporter's exact state on a live machine.

The bug (upstream node-pty 1.1.0, macOS): pty_posix_spawn leaks one /dev/ptmx master fd on every spawn. Its low-fd workaround probes posix_openpt into low_fds[], but the cleanup loop —

for (; count > 0; count--) {
    close(low_fds[count]);
}

— never closes low_fds[0], and in practice the first probe always lands at fd ≥ 3 (break with count == 0), so nothing is ever closed.

Why this becomes #5699: each leaked master counts against the system-wide pty cap (kern.tty.ptmx_max = 511 on macOS). Field evidence from the machine where this was diagnosed: a 28-day-old pty-daemon held 509 pty fds while tracking only 20 live sessions; the system sat at 527/511 ptys. At the cap, PTY allocation fails machine-wide — presets toast Failed to run preset, session spawns die with spawn failed (... errno=ok): posix_spawnp failed. (the re-probe in Pty.ts succeeds because plain spawns need no pty — that errno=ok is the leak's signature), and every other terminal app on the machine breaks too. Reboot "fixes" it by releasing all ptys, exactly as the reporter described.

The fix: upstream already fixed this in microsoft/node-pty#882 (fix: /dev/ptmx leak on macOS), but it's unreleased — 1.1.0 is still the latest stable and the fix only exists in 1.2.0 betas. This PR backports the two fd-hygiene changes onto our pinned 1.1.0 as a bun patch (patches/node-pty@1.1.0.patch): close all low_fds probes, and close the parent's slave copy after posix_spawn. Native rebuild picks the patch up automatically (build/Release is checked before prebuilds/ in node-pty's loader).

How I tested it

Reproduced and verified end-to-end on macOS (arm64):

  • Repro (before): a standalone daemon (both the packaged 1.150 app bundle and a fresh repo build) leaks exactly +1 ptmx per session opened, 15/15 across clean-exit, background-child, and held-tty scenarios — the leak happens at spawn time, independent of exit handling. lsof shows the extra fd is the low_fds[0] probe.
  • After the patch: same repro leaks 0 fds; a session while alive holds exactly one pty fd (the real master), and it closes on exit.
  • Regression tests: test/fd-release.test.ts counts the process's open pty fds across spawn/exit cycles (mirroring upstream #882's own test), wired into test:integration.
  • Full pty-daemon integration suite passes under Node: 52/52. bun run lint and tsc --noEmit (pty-daemon) pass.

Related: #5711 adds host-service wedge auto-recovery for the same issue's secondary failure mode (Failed to fetch toasts); this PR fixes the resource leak that starves the machine in the first place.

Known gap: macOS CLI dist still ships the unpatched prebuild

A self-review pass surfaced one important limitation that needs a maintainer decision: packages/cli/scripts/build-dist.ts deliberately deletes node-pty's build/ on darwin ("removing node-pty build/ so bindings falls back to prebuilds/", presumably for cross-arch packaging), so the macOS CLI dist — including the CLI binary bundled into the desktop app — loads the npm-shipped prebuilds/darwin-*/pty.node, which this source patch cannot reach. The desktop app's own runtime is fixed (electron-rebuild compiles the patched source into build/Release, which the loader prefers), but pty-daemon/host-service processes spawned by the macOS CLI keep leaking. Fixing that means either rebuilding the patched source per-arch during CLI packaging and replacing the prebuilds, or forcing npm_config_build_from_source on darwin — both touch the release pipeline, so I left it out of this PR rather than risk the builds. Happy to do it as a follow-up if you tell me which direction you prefer.

Notes for reviewers

  • The backport is intentionally minimal (the two leak fixes only) rather than the full #882 refactor of error paths — the deterministic per-spawn leak is what matters in the field. When node-pty 1.2.0 goes stable, bumping the dependency supersedes this patch.
  • The patch approach follows the same precedent used by comparable Electron terminal apps pinning node-pty 1.1.0.

Checklist

  • PR title follows conventional commits (type(scope): subject)
  • bun run lint and bun run typecheck pass (CI fails on lint warnings too)
  • "Allow edits from maintainers" is checked on fork PRs
Open in Stage

Summary by CodeRabbit

  • Bug Fixes
    • Resolved macOS PTY file-descriptor leaks by ensuring exit watchers and PTY spawn/cleanup fully close all required descriptors.
    • Prevented lingering /dev/ptmx master descriptors across repeated spawn/exit cycles.
  • Tests
    • Added a macOS regression suite that verifies no descriptor accumulation after repeated PTY sessions.
    • Improved integration test reliability by switching to a shared waitFor polling helper and expanding the integration test run to include the new regression.

Summary by cubic

Backports macOS /dev/ptmx and kqueue fd leak fixes to node-pty@1.1.0, stopping per-spawn leaks that can exhaust system ptys or the process fd limit and break terminal spawns. Resolves #5699 and keeps long‑lived daemons stable.

  • Bug Fixes

    • Close all low_fds probe fds and the parent’s slave fd after posix_spawn (backported from microsoft/node-pty#882).
    • Close the kqueue opened in SetupExitCallback on macOS to stop per‑spawn fd leaks (backported from microsoft/node-pty#931).
    • Add fd-release.test.ts to verify pty and macOS kqueue fd counts; use a shared waitFor helper and harden checks (fail on empty lsof, count Linux /dev/pts/).

  • Dependencies

    • Add patches/node-pty@1.1.0.patch and enable via patchedDependencies in package.json and bun.lock.