fix(core): free an exited web terminal's PTY resources at exit time

packages/core/src/services/web-terminal-registry.test.ts

@@ -406,6 +406,83 @@ describe('WebTerminalRegistry', () => {
     expect(kill).not.toHaveBeenCalled();
   });
 
+  it('frees an exited session at exit time, not at the idle reclaim', async () => {
+    osPlatform.mockReturnValue('win32');
+    const registry = new WebTerminalRegistry();
+    await registry.create({
+      terminalId: 'terminal:exit-time-release',
+      workspaceCwd: '/workspace',
+    });
+
+    onExit({ exitCode: 0 });
+    // One turn of the event loop, on real timers: the 15-minute idle reclaim
+    // cannot have run, and nothing below calls release(). A live exit closes
+    // the route's socket with 4000, which the client treats as non-retryable,
+    // so no tab close follows either — that window is what #11353 is about.
+    await new Promise<void>((resolve) => setImmediate(resolve));
+
+    // node-pty strands its conout worker on a natural exit, so that worker is
+    // the resource an exited web terminal held for up to IDLE_RECLAIM_MS — and
+    // exited sessions do not count against the admission cap, so accumulation
+    // inside the window was unbounded. See #11303 / #11353.
+    expect(conoutDispose).toHaveBeenCalledOnce();
+    expect(disposeData).toHaveBeenCalledOnce();
+    expect(disposeExit).toHaveBeenCalledOnce();
+    // Nothing may signal an exited shell's possibly-recycled pid.
+    expect(kill).not.toHaveBeenCalled();
+    expect(spawnSync).not.toHaveBeenCalled();
+    // The session itself survives the release, for scrollback replay.
+    expect(registry.readSnapshot('terminal:exit-time-release')).toBeDefined();
+  });
+
+  it('still replays buffered scrollback after the exit-time release', async () => {
+    osPlatform.mockReturnValue('win32');
+    const registry = new WebTerminalRegistry();
+    await registry.create({
+      terminalId: 'terminal:replay-after-exit-release',
+      workspaceCwd: '/workspace',
+    });
+
+    onData('boot\r\n');
+    onExit({ exitCode: 3 });
+    await new Promise<void>((resolve) => setImmediate(resolve));
+
+    // The exit-time release frees PTY handles only. The session and its buffer
+    // stay in the map, so a second tab attaching to this terminal id still gets
+    // the scrollback plus the exit state — which is what terminal.ts's
+    // releaseAfterReplay path depends on.
+    expect(conoutDispose).toHaveBeenCalledOnce();
+    expect(registry.readSnapshot('terminal:replay-after-exit-release')).toEqual(
+      {
+        output: 'boot\r\n',
+        exited: true,
+        exitCode: 3,
+        workspaceCwd: '/workspace',
+      },
+    );
+  });
+
+  it('does not free an exited session twice when release follows', async () => {
+    osPlatform.mockReturnValue('win32');
+    const registry = new WebTerminalRegistry();
+    await registry.create({
+      terminalId: 'terminal:exit-release-once',
+      workspaceCwd: '/workspace',
+    });
+
+    onExit({ exitCode: 0 });
+    await new Promise<void>((resolve) => setImmediate(resolve));
+    // The tab close, a workspace drain, dispose() or the reclaim all still run
+    // release() on a session whose PTY was already freed at exit time.
+    expect(registry.release('terminal:exit-release-once')).toBe(true);
+
+    expect(conoutDispose).toHaveBeenCalledOnce();
+    expect(disposeData).toHaveBeenCalledOnce();
+    expect(disposeExit).toHaveBeenCalledOnce();
+    expect(kill).not.toHaveBeenCalled();
+    expect(spawnSync).not.toHaveBeenCalled();
+  });
+
   it('forwards live output and bounds unacknowledged PTY input', async () => {
     const registry = new WebTerminalRegistry();
     await registry.create({

packages/core/src/services/web-terminal-registry.ts

@@ -72,6 +72,13 @@ interface PtySession {
   reclaimTimer?: ReturnType<typeof setTimeout>;
   dataDisposable?: { dispose(): void };
   exitDisposable?: { dispose(): void };
+  /**
+   * Set once the PTY-side resources above have been freed. The exit-time
+   * release frees them while the session stays in the map for scrollback
+   * replay, so a later `release()` — tab close, workspace drain, `dispose()`,
+   * idle reclaim — must not free them a second time. See #11353.
+   */
+  ptyResourcesReleased: boolean;
 }
 
 interface SpawnedWebTerminalPty extends WebTerminalPty {
@@ -270,6 +277,23 @@ export class WebTerminalRegistry {
       session.exited = true;
       session.exitCode = e.exitCode;
       for (const listener of [...session.exitListeners]) listener(e);
+      // Nothing needs the PTY once the shell is gone: write() and resize()
+      // already short-circuit on `exited`, and readSnapshot() replays the
+      // JS-side `buffer`, not the console. Waiting for release() instead left
+      // every exited web terminal holding node-pty's conout worker — and,
+      // upstream, its conhost.exe — for up to IDLE_RECLAIM_MS, because the
+      // route keeps the session alive for scrollback and the client treats the
+      // 4000 close as non-retryable, so only a tab close releases it. Exited
+      // sessions also do not count against the admission cap, so accumulation
+      // inside that window was unbounded. See #11303 / #11353.
+      //
+      // Deferred one turn rather than run inline: onExit can arrive slightly
+      // before late PTY data is processed, the same race shellExecutionService
+      // drains before finalizing. setImmediate runs after the poll-phase
+      // callbacks already queued this tick, so trailing output still reaches
+      // `buffer` before the data listener is detached. handleData is fully
+      // synchronous, so one turn is enough — there is no chain to flush.
+      setImmediate(() => this.releasePtyResources(session));
     };
     let dataDisposable: { dispose(): void } | undefined;
     let exitDisposable: { dispose(): void } | undefined;
@@ -347,6 +371,7 @@ export class WebTerminalRegistry {
       exitListeners: new Set(),
       dataDisposable,
       exitDisposable,
+      ptyResourcesReleased: false,
     };
     sessionRef.current = session;
     this.sessions.set(terminalId, session);
@@ -458,29 +483,20 @@ export class WebTerminalRegistry {
         listener({ exitCode: 143, signal: 15 });
       }
     }
-    session.dataDisposable?.dispose();
-    session.exitDisposable?.dispose();
     session.outputListeners.clear();
     session.exitListeners.clear();
     if (!session.exited) {
+      // killPtyTree has to run before releasePtyResources: its pty.kill()
+      // defers the whole teardown while `_isReady` is false, so a terminal
+      // released before its shell's first output byte (tab closed during slow
+      // pwsh startup, or a workspace drain) still has a kill() queued in
+      // node-pty's `_deferreds`. The wrapper's kill() notes the close only when
+      // it really ran; releaseHost then disposes the worker a deferred kill
+      // would strand, and skips the native close so the queued kill() stays the
+      // single closer — never a second close.
       killPtyTree(session.pty);
-      // killPtyTree's pty.kill() defers its whole teardown while `_isReady` is
-      // false, so a terminal released before its shell's first output byte (tab
-      // closed during slow pwsh startup, or a workspace drain) still has a
-      // kill() queued in node-pty's `_deferreds`. The wrapper's kill() notes
-      // the close only when it really ran; releaseHost then disposes the worker
-      // a deferred kill would strand, and skips the native close so the queued
-      // kill() stays the single closer — never a second close.
-      session.pty.releaseHost?.();
-    } else {
-      // The shell already exited, so nothing may signal its (possibly recycled)
-      // pid — but node-pty does not release its conout worker thread on a
-      // natural exit, so without this every terminal the user exits leaks one
-      // for the life of the CLI. Same defect as the shell-tool path in
-      // shellExecutionService. The conhost.exe half is not freed here (the
-      // native baton is already gone); see releaseConPtyHost. See #11303.
-      session.pty.releaseHost?.();
     }
+    this.releasePtyResources(session);
     return true;
   }
 
@@ -502,6 +518,33 @@ export class WebTerminalRegistry {
     this.cancelledCreations.delete(terminalId);
   }
 
+  /**
+   * Free a session's PTY-side resources exactly once: detach the data/exit
+   * listeners, then release the ConPTY host / conout worker that node-pty
+   * strands on a natural exit. Without the second half every terminal the user
+   * exits leaks a worker for the life of the CLI — the same defect the
+   * shell-tool path has. The conhost.exe half is not freed on that path (the
+   * native baton is already gone); see releaseConPtyHost. See #11303.
+   *
+   * Deliberately leaves the session's map entry and its `buffer` alone, and
+   * never signals the pid: on the exited path the shell is gone and its pid may
+   * be recycled, which is why #11313 added `releaseHost` instead of reusing
+   * `kill()`. Keeping the entry is what lets `readSnapshot()` still replay the
+   * scrollback after an exit-time release.
+   *
+   * Called from `handleExit` (deferred one turn, so an exited web terminal
+   * stops holding the worker for the whole idle-reclaim window — #11353) and
+   * from `release()` on both of its arms, where the flag keeps a release that
+   * follows an exit-time release from disposing anything twice.
+   */
+  private releasePtyResources(session: PtySession): void {
+    if (session.ptyResourcesReleased) return;
+    session.ptyResourcesReleased = true;
+    session.dataDisposable?.dispose();
+    session.exitDisposable?.dispose();
+    session.pty.releaseHost?.();
+  }
+
   private clearReclaim(session: PtySession): void {
     if (session.reclaimTimer) {
       clearTimeout(session.reclaimTimer);