From 8ad083216b9b64b2dad20994e192ea049170c8b1 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Thu, 29 Jan 2026 03:18:41 +0900 Subject: [PATCH 1/3] fix: /dev/ptmx leak on macOS --- src/unix/pty.cc | 95 +++++++++++++++++++++++++++------------- src/unixTerminal.test.ts | 31 +++++++++++++ 2 files changed, 96 insertions(+), 30 deletions(-) diff --git a/src/unix/pty.cc b/src/unix/pty.cc index 4917560b..88b1a4c9 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -279,7 +279,7 @@ pty_posix_spawn(char** argv, char** env, const struct winsize *winp, int* master, pid_t* pid, - int* err); + std::string* err); #endif struct DelBuf { @@ -388,7 +388,7 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) { std::string helper_path = info[9].As(); pid_t pid; - int master; + int master = -1; #if defined(__APPLE__) int argc = argv_.Length(); int argl = argc + 4; @@ -403,10 +403,13 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) { argv[i + 3] = strdup(arg.c_str()); } - int err = -1; + std::string err; pty_posix_spawn(argv, env, term, &winp, &master, &pid, &err); - if (err != 0) { - throw Napi::Error::New(napiEnv, std::string("pty_posix_spawn failed with error: ") + std::to_string(err) + " (" + strerror(err) + ")"); + if (!err.empty()) { + if (master != -1) { + close(master); + } + throw Napi::Error::New(napiEnv, err); } if (pty_nonblock(master) == -1) { throw Napi::Error::New(napiEnv, "Could not set master fd to nonblocking."); @@ -725,15 +728,26 @@ pty_getproc(int fd, char *tty) { #endif #if defined(__APPLE__) +static std::string format_error(const char* func, int err_code) { + char buf[256]; + snprintf(buf, sizeof(buf), "%s: %s", func, strerror(err_code)); + return buf; +} + static void pty_posix_spawn(char** argv, char** env, const struct termios *termp, const struct winsize *winp, int* master, pid_t* pid, - int* err) { + std::string* err) { int low_fds[3]; size_t count = 0; + int res = 0; + int slave = -1; + char slave_pty_name[128]; + int spawn_err; + sigset_t signal_set; for (; count < 3; count++) { low_fds[count] = posix_openpt(O_RDWR); @@ -745,82 +759,103 @@ pty_posix_spawn(char** argv, char** env, POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETSID; + + posix_spawn_file_actions_t acts; + posix_spawn_file_actions_init(&acts); + + posix_spawnattr_t attrs; + posix_spawnattr_init(&attrs); + *master = posix_openpt(O_RDWR); if (*master == -1) { - return; + *err = format_error("posix_openpt failed", errno); + goto done; } - int res = grantpt(*master) || unlockpt(*master); + res = grantpt(*master); if (res == -1) { - return; + *err = format_error("grantpt failed", errno); + goto done; + } + + res = unlockpt(*master); + if (res == -1) { + *err = format_error("unlockpt failed", errno); + goto done; } // Use TIOCPTYGNAME instead of ptsname() to avoid threading problems. - int slave; - char slave_pty_name[128]; res = ioctl(*master, TIOCPTYGNAME, slave_pty_name); if (res == -1) { - return; + *err = format_error("ioctl(TIOCPTYGNAME) failed", errno); + goto done; } slave = open(slave_pty_name, O_RDWR | O_NOCTTY); if (slave == -1) { - return; + *err = format_error("open slave pty failed", errno); + goto done; } if (termp) { res = tcsetattr(slave, TCSANOW, termp); if (res == -1) { - return; + *err = format_error("tcsetattr failed", errno); + goto done; }; } if (winp) { res = ioctl(slave, TIOCSWINSZ, winp); if (res == -1) { - return; + *err = format_error("ioctl(TIOCSWINSZ) failed", errno); + goto done; } } - posix_spawn_file_actions_t acts; - posix_spawn_file_actions_init(&acts); posix_spawn_file_actions_adddup2(&acts, slave, STDIN_FILENO); posix_spawn_file_actions_adddup2(&acts, slave, STDOUT_FILENO); posix_spawn_file_actions_adddup2(&acts, slave, STDERR_FILENO); posix_spawn_file_actions_addclose(&acts, slave); posix_spawn_file_actions_addclose(&acts, *master); - posix_spawnattr_t attrs; - posix_spawnattr_init(&attrs); - *err = posix_spawnattr_setflags(&attrs, flags); - if (*err != 0) { + spawn_err = posix_spawnattr_setflags(&attrs, flags); + if (spawn_err != 0) { + *err = format_error("posix_spawnattr_setflags failed", spawn_err); goto done; } - sigset_t signal_set; /* Reset all signal the child to their default behavior */ sigfillset(&signal_set); - *err = posix_spawnattr_setsigdefault(&attrs, &signal_set); - if (*err != 0) { + spawn_err = posix_spawnattr_setsigdefault(&attrs, &signal_set); + if (spawn_err != 0) { + *err = format_error("posix_spawnattr_setsigdefault failed", spawn_err); goto done; } /* Reset the signal mask for all signals */ sigemptyset(&signal_set); - *err = posix_spawnattr_setsigmask(&attrs, &signal_set); - if (*err != 0) { + spawn_err = posix_spawnattr_setsigmask(&attrs, &signal_set); + if (spawn_err != 0) { + *err = format_error("posix_spawnattr_setsigmask failed", spawn_err); goto done; } do - *err = posix_spawn(pid, argv[0], &acts, &attrs, argv, env); - while (*err == EINTR); + spawn_err = posix_spawn(pid, argv[0], &acts, &attrs, argv, env); + while (spawn_err == EINTR); + if (spawn_err != 0) { + *err = format_error("posix_spawn failed", spawn_err); + } done: posix_spawn_file_actions_destroy(&acts); posix_spawnattr_destroy(&attrs); + if (slave != -1) { + close(slave); + } - for (; count > 0; count--) { - close(low_fds[count]); + for (size_t i = 0; i <= count; i++) { + close(low_fds[i]); } } #endif diff --git a/src/unixTerminal.test.ts b/src/unixTerminal.test.ts index 63f1fc3c..f13e66d4 100644 --- a/src/unixTerminal.test.ts +++ b/src/unixTerminal.test.ts @@ -366,6 +366,37 @@ if (process.platform !== 'win32') { done(); }); }); + it('should not leak /dev/ptmx file descriptors after pty exit', async function(): Promise { + this.timeout(30000); + + const getPtmxFDCount = (): number => { + try { + const output = cp.execSync(`lsof -p ${process.pid} 2>/dev/null`, { encoding: 'utf8' }); + return output.split('\n').filter(line => line.includes('ptmx')).length; + } catch { + return 0; + } + }; + + const initialCount = getPtmxFDCount(); + for (let i = 0; i < 20; i++) { + const term = new UnixTerminal('/bin/bash', ['-c', 'echo hello']); + await new Promise(resolve => { + term.onExit(() => { + term.destroy(); + resolve(); + }); + }); + } + + await new Promise(r => setTimeout(r, 500)); + + const finalCount = getPtmxFDCount(); + assert.ok( + finalCount <= initialCount, + `Leaked ${finalCount - initialCount} /dev/ptmx FDs after spawning 20 PTYs (initial: ${initialCount}, final: ${finalCount})` + ); + }); } it('should handle exec() errors', (done) => { const term = new UnixTerminal('/bin/bogus.exe', []); From 565182e1a3548e49be8b1666e67b0aaa1b7875d4 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Thu, 29 Jan 2026 03:55:43 +0900 Subject: [PATCH 2/3] fix: flaky test --- src/unixTerminal.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/unixTerminal.test.ts b/src/unixTerminal.test.ts index f13e66d4..7425efa5 100644 --- a/src/unixTerminal.test.ts +++ b/src/unixTerminal.test.ts @@ -291,8 +291,10 @@ if (process.platform !== 'win32') { if (process.platform === 'darwin') { it('should return the name of the process', (done) => { const term = new UnixTerminal('/bin/echo'); - assert.strictEqual(term.process, '/bin/echo'); - term.on('exit', () => done()); + term.on('exit', () => { + assert.strictEqual(term.process, '/bin/echo'); + done(); + }); term.destroy(); }); it('should return the name of the sub process', (done) => { From 968b05f4f89c2e37feaa3f25a0ab0ea53eb70be7 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Thu, 29 Jan 2026 04:35:13 +0900 Subject: [PATCH 3/3] fix: ignore spawn_helper in process title getter --- src/unixTerminal.test.ts | 6 ++---- src/unixTerminal.ts | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/unixTerminal.test.ts b/src/unixTerminal.test.ts index 7425efa5..f13e66d4 100644 --- a/src/unixTerminal.test.ts +++ b/src/unixTerminal.test.ts @@ -291,10 +291,8 @@ if (process.platform !== 'win32') { if (process.platform === 'darwin') { it('should return the name of the process', (done) => { const term = new UnixTerminal('/bin/echo'); - term.on('exit', () => { - assert.strictEqual(term.process, '/bin/echo'); - done(); - }); + assert.strictEqual(term.process, '/bin/echo'); + term.on('exit', () => done()); term.destroy(); }); it('should return the name of the sub process', (done) => { diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index 55dfb3ea..2776d501 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -258,7 +258,7 @@ export class UnixTerminal extends Terminal { public get process(): string { if (process.platform === 'darwin') { const title = pty.process(this._fd); - return (title !== 'kernel_task') ? title : this._file; + return (title !== 'kernel_task' && title !== 'spawn_helper') ? title : this._file; } return pty.process(this._fd, this._pty) || this._file;