fix: /dev/ptmx leak on macOS (#882)

* fix: /dev/ptmx leak on macOS

* fix: flaky test

* fix: ignore spawn_helper in process title getter

Author: deepak1556

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<Napi::String>();
 
   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

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<void> {
+          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<void>(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', []);

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;