Skip to content

fix: upgrade node-pty to fix macOS PTY exhaustion#1

Open
speakman wants to merge 2 commits intomfazekas:fix/pty-proxy-stdin-corruptionfrom
speakman:fix/node-pty-macos-ptmx-leak
Open

fix: upgrade node-pty to fix macOS PTY exhaustion#1
speakman wants to merge 2 commits intomfazekas:fix/pty-proxy-stdin-corruptionfrom
speakman:fix/node-pty-macos-ptmx-leak

Conversation

@speakman
Copy link
Copy Markdown

Summary

Fixes the macOS PTY exhaustion bug introduced by the node-pty dependency in slopus#553.

  • Upgrade node-pty from 1.1.0 to 1.2.0-beta.11 which includes the /dev/ptmx leak fix (microsoft/node-pty#882)
  • Fix abort handler to kill the entire process group (-pid, SIGTERM) instead of just the launcher PID (SIGHUP)

Root Cause

node-pty v1.1.0 has an off-by-one bug in pty_posix_spawn() (macOS codepath in src/unix/pty.cc):

// Allocate placeholder fds to keep master fd >= 3
int low_fds[3];
size_t count = 0;

for (; count < 3; count++) {
    low_fds[count] = posix_openpt(O_RDWR);  // allocates a PTY pair!
    if (low_fds[count] >= STDERR_FILENO)
        break;  // count stays at 0 in normal case
}

*master = posix_openpt(O_RDWR);  // the real master

// Cleanup — BUGGY:
for (; count > 0; count--) {
    close(low_fds[count]);  // never executes when count == 0
}

In the normal case (fds 0,1,2 occupied), low_fds[0] gets fd >= 3, the break fires with count = 0, and the cleanup loop count > 0 is immediately false. low_fds[0] is never closed, leaking 1 PTY master fd per pty.spawn() call. With macOS's 511 PTY limit (kern.tty.ptmx_max), this exhausts all PTYs after ~511 mode switches.

The upstream fix (in 1.2.0-beta.9+) changes the cleanup to:

for (size_t i = 0; i <= count; i++) {
    close(low_fds[i]);
}

Test plan

  • All 8 claudeLocal.test.ts tests pass
  • Verify PTY count stays stable across multiple local↔remote mode switches on macOS
  • Verify abort properly terminates both the node launcher and the Claude binary

node-pty v1.1.0 has an off-by-one bug in pty_posix_spawn() on macOS
where low_fds[0] (allocated via posix_openpt()) is never closed due
to a buggy cleanup loop (`count > 0` vs the correct `i <= count`).
This leaks 1 PTY master fd per pty.spawn() call, eventually exhausting
all 511 PTY pairs on macOS (kern.tty.ptmx_max).

Upstream fix: microsoft/node-pty#882 (merged 2026-01-28).

Also fixes the abort handler to kill the entire process group (-pid)
with SIGTERM instead of sending SIGHUP to just the launcher PID,
ensuring the Claude binary grandchild is also terminated.
Spawns real node-pty processes and counts PTY fds via lsof to verify
no leak occurs. Covers: normal exit, kill, process group SIGTERM, and
rapid spawn-kill cycles (50 iterations).

With the old v1.1.0 bug these tests would show ~N leaked fds per N
spawns. With v1.2.0-beta.11 they show 0.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is testing the concept and not the actual implementation, I'm not sure if it should be part of the test suite

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.

2 participants