Defer conptyNative.connect() to prevent event loop blocking

src/conpty_console_list_agent.ts

@@ -10,6 +10,14 @@ import { loadNativeModule } from './utils';
 
 const getConsoleProcessList = loadNativeModule('conpty_console_list').module.getConsoleProcessList;
 const shellPid = parseInt(process.argv[2], 10);
-const consoleProcessList = getConsoleProcessList(shellPid);
+let consoleProcessList: number[] = [];
+if (shellPid > 0) {
+  try {
+    consoleProcessList = getConsoleProcessList(shellPid);
+  } catch {
+    // AttachConsole can fail if the process already exited or is invalid.
+    consoleProcessList = [];
+  }
+}
 process.send!({ consoleProcessList });
 process.exit(0);

src/windowsPtyAgent.test.ts

@@ -4,7 +4,7 @@
  */
 
 import * as assert from 'assert';
-import { argsToCommandLine } from './windowsPtyAgent';
+import { argsToCommandLine, WindowsPtyAgent } from './windowsPtyAgent';
 
 function check(file: string, args: string | string[], expected: string): void {
   assert.equal(argsToCommandLine(file, args), expected);
@@ -91,4 +91,117 @@ if (process.platform === 'win32') {
       });
     });
   });
+
+  describe('WindowsPtyAgent', () => {
+    describe('connection timing (issue #763)', () => {
+      it('should defer conptyNative.connect() until worker is ready', function (done) {
+        this.timeout(10000);
+
+        const term = new WindowsPtyAgent(
+          'cmd.exe',
+          '/c echo test',
+          Object.keys(process.env).map(k => `${k}=${process.env[k]}`),
+          process.cwd(),
+          80,
+          30,
+          false,
+          false,
+          false
+        );
+
+        // The innerPid should be 0 initially since connect() is deferred
+        // until the worker signals ready. This verifies the fix for #763.
+        const initialPid = term.innerPid;
+
+        // Wait for the connection to complete via ready_datapipe event
+        term.outSocket.on('ready_datapipe', () => {
+          // After worker is ready and connect() is called, innerPid should be set
+          // Use a small delay to ensure _completePtyConnection has run
+          setTimeout(() => {
+            assert.notStrictEqual(term.innerPid, 0, 'innerPid should be set after worker is ready');
+            assert.strictEqual(initialPid, 0, 'innerPid should have been 0 before worker was ready');
+            term.kill();
+            done();
+          }, 100);
+        });
+      });
+
+      it('should successfully spawn a process after deferred connection', function (done) {
+        this.timeout(10000);
+
+        const term = new WindowsPtyAgent(
+          'cmd.exe',
+          '/c echo hello',
+          Object.keys(process.env).map(k => `${k}=${process.env[k]}`),
+          process.cwd(),
+          80,
+          30,
+          false,
+          false,
+          false
+        );
+
+        let output = '';
+        term.outSocket.on('data', (data: string) => {
+          output += data;
+        });
+
+        // Wait for process to complete and verify output
+        setTimeout(() => {
+          assert.ok(output.includes('hello'), `Expected output to contain "hello", got: ${output}`);
+          term.kill();
+          done();
+        }, 2000);
+      });
+
+      it('should allow async work between construction and connection (non-blocking)', function (done) {
+        this.timeout(10000);
+
+        // Track the sequence of events to verify non-blocking behavior
+        const events: string[] = [];
+
+        const term = new WindowsPtyAgent(
+          'cmd.exe',
+          '/c echo test',
+          Object.keys(process.env).map(k => `${k}=${process.env[k]}`),
+          process.cwd(),
+          80,
+          30,
+          false,
+          false,
+          false
+        );
+
+        events.push('constructor_returned');
+        assert.strictEqual(term.innerPid, 0, 'innerPid should be 0 immediately after construction');
+
+        // Schedule async work - this MUST run before ready_datapipe if constructor is non-blocking
+        setImmediate(() => {
+          events.push('setImmediate_ran');
+          // innerPid might still be 0 or might be set by now, depending on timing
+          // The key is that setImmediate ran, proving the event loop wasn't blocked
+        });
+
+        term.outSocket.on('ready_datapipe', () => {
+          events.push('ready_datapipe');
+
+          setTimeout(() => {
+            events.push('final_check');
+
+            // Verify the sequence: constructor returned, then async work could run
+            assert.ok(events.includes('constructor_returned'), 'constructor should have returned');
+            assert.ok(events.includes('setImmediate_ran'), 'setImmediate should have run (event loop not blocked)');
+            assert.ok(events.indexOf('constructor_returned') < events.indexOf('setImmediate_ran'),
+              'constructor should return before setImmediate runs');
+
+            // Most importantly: innerPid should now be set
+            assert.notStrictEqual(term.innerPid, 0, 'innerPid should be set after connection');
+
+            term.kill();
+            done();
+          }, 100);
+        });
+      });
+    });
+  });
 }

src/windowsPtyAgent.ts

@@ -42,6 +42,8 @@ export class WindowsPtyAgent {
   public get innerPid(): number { return this._innerPid; }
   public get pty(): number { return this._pty; }
 
+  private _pendingPtyInfo: { pty: number, commandLine: string, cwd: string, env: string[] } | undefined;
+
   constructor(
     file: string,
     args: ArgvOrCommandLine,
@@ -78,9 +80,29 @@ export class WindowsPtyAgent {
     this._outSocket = new Socket();
     this._outSocket.setEncoding('utf8');
     // The conout socket must be ready out on another thread to avoid deadlocks
+    // We must wait for the worker to connect before calling conptyNative.connect()
+    // to avoid blocking the Node.js event loop in ConnectNamedPipe.
+    // See https://github.com/microsoft/node-pty/issues/763
     this._conoutSocketWorker = new ConoutConnection(term.conout, this._useConptyDll);
+
+    // Store pending connection info - we'll complete the connection when worker is ready
+    this._pendingPtyInfo = { pty: this._pty, commandLine, cwd, env };
+
+    // Timeout to ensure connection completes even if worker fails to signal ready
+    const connectionTimeout = setTimeout(() => {
+      if (this._pendingPtyInfo) {
+        // Worker never signaled ready - complete connection anyway to avoid zombie state
+        this._completePtyConnection();
+      }
+    }, 5000);
+
     this._conoutSocketWorker.onReady(() => {
+      clearTimeout(connectionTimeout);
       this._conoutSocketWorker.connectSocket(this._outSocket);
+      // Now that the worker has connected to the output pipe, we can safely call
+      // conptyNative.connect() which calls ConnectNamedPipe - it won't block because
+      // the client (worker) is already connected
+      this._completePtyConnection();
     });
     this._outSocket.on('connect', () => {
       this._outSocket.emit('ready_datapipe');
@@ -93,8 +115,16 @@ export class WindowsPtyAgent {
       writable: true
     });
     this._inSocket.setEncoding('utf8');
+  }
 
-    const connect = conptyNative.connect(this._pty, commandLine, cwd, env, this._useConptyDll, c => this._$onProcessExit(c));
+  private _completePtyConnection(): void {
+    if (!this._pendingPtyInfo) {
+      return;
+    }
+    const { pty, commandLine, cwd, env } = this._pendingPtyInfo;
+    this._pendingPtyInfo = undefined;
+
+    const connect = conptyNative.connect(pty, commandLine, cwd, env, this._useConptyDll, c => this._$onProcessExit(c));
     this._innerPid = connect.pid;
   }
 
@@ -110,6 +140,9 @@ export class WindowsPtyAgent {
   }
 
   public kill(): void {
+    // Prevent deferred connection from completing after kill
+    this._pendingPtyInfo = undefined;
+
     // Tell the agent to kill the pty, this releases handles to the process
     if (!this._useConptyDll) {
       this._inSocket.readable = false;
@@ -136,6 +169,9 @@ export class WindowsPtyAgent {
   }
 
   private _getConsoleProcessList(): Promise<number[]> {
+    if (this._innerPid <= 0) {
+      return Promise.resolve([]);
+    }
     return new Promise<number[]>(resolve => {
       const agent = fork(path.join(__dirname, 'conpty_console_list_agent'), [ this._innerPid.toString() ]);
       agent.on('message', message => {

src/windowsTerminal.test.ts

@@ -104,25 +104,69 @@ if (process.platform === 'win32') {
         it('should kill the process tree', function (done: Mocha.Done): void {
           this.timeout(20000);
           const term = new WindowsTerminal('cmd.exe', [], { useConptyDll });
-          // Start sub-processes
-          term.write('powershell.exe\r');
-          term.write('node.exe\r');
-          console.log('start poll for tree size');
-          pollForProcessTreeSize(term.pid, 3, 500, 5000).then(list => {
-            assert.strictEqual(list[0].name.toLowerCase(), 'cmd.exe');
-            assert.strictEqual(list[1].name.toLowerCase(), 'powershell.exe');
-            assert.strictEqual(list[2].name.toLowerCase(), 'node.exe');
-            term.kill();
-            const desiredState: IProcessState = {};
-            desiredState[list[0].pid] = false;
-            desiredState[list[1].pid] = false;
-            desiredState[list[2].pid] = false;
-            term.on('exit', () => {
-              pollForProcessState(desiredState, 1000, 5000).then(() => {
-                done();
-              }).catch(done);
-            });
-          }).catch(done);
+          const socket = (term as any)._socket;
+          let started = false;
+          const startPolling = (): void => {
+            if (started) {
+              return;
+            }
+            if (term.pid === 0) {
+              setTimeout(startPolling, 50);
+              return;
+            }
+            started = true;
+            // Start sub-processes
+            term.write('powershell.exe\r');
+            term.write('node.exe\r');
+            console.log('start poll for tree size');
+            pollForProcessTreeSize(term.pid, 3, 500, 5000).then(list => {
+              assert.strictEqual(list[0].name.toLowerCase(), 'cmd.exe');
+              assert.strictEqual(list[1].name.toLowerCase(), 'powershell.exe');
+              assert.strictEqual(list[2].name.toLowerCase(), 'node.exe');
+              term.kill();
+              const desiredState: IProcessState = {};
+              desiredState[list[0].pid] = false;
+              desiredState[list[1].pid] = false;
+              desiredState[list[2].pid] = false;
+              term.on('exit', () => {
+                pollForProcessState(desiredState, 1000, 5000).then(() => {
+                  done();
+                }).catch(done);
+              });
+            }).catch(done);
+          };
+
+          if (term.pid > 0) {
+            startPolling();
+          } else {
+            socket.once('ready_datapipe', () => setTimeout(startPolling, 50));
+          }
+        });
+      });
+
+      describe('pid', () => {
+        it('should be 0 before ready and set after ready_datapipe (issue #763)', function (done) {
+          this.timeout(10000);
+          const term = new WindowsTerminal('cmd.exe', '/c echo test', { useConptyDll });
+
+          // pid may be 0 immediately after construction due to deferred connection
+          const initialPid = term.pid;
+
+          // Access internal socket to listen for ready_datapipe
+          const socket = (term as any)._socket;
+          socket.on('ready_datapipe', () => {
+            // After ready_datapipe, pid should be set to a valid non-zero value
+            setTimeout(() => {
+              assert.notStrictEqual(term.pid, 0, 'pid should be set after ready_datapipe');
+              assert.strictEqual(typeof term.pid, 'number', 'pid should be a number');
+              // If initial was 0, it should now be different (proves the fix works)
+              if (initialPid === 0) {
+                assert.notStrictEqual(term.pid, initialPid, 'pid should be updated from initial value');
+              }
+              term.on('exit', () => done());
+              term.kill();
+            }, 100);
+          });
         });
       });
 

src/windowsTerminal.ts

@@ -59,6 +59,8 @@ export class WindowsTerminal extends Terminal {
     // The forked windows terminal is not available until `ready` event is
     // emitted.
     this._socket.on('ready_datapipe', () => {
+      // Update pid now that the agent has connected
+      this._pid = this._agent.innerPid;
 
       // Run deferreds and set ready state once the first data event is received.
       this._socket.once('data', () => {