From 32ddfcd89c0f7de9a86f7fe135000a971d6b3682 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Thu, 27 Aug 2026 13:06:12 +0800 Subject: [PATCH 1/2] fix(subprocess): read the process table once per terminal poll MacProcessInspector answered the descendant tree and every member's liveness with its own `/bin/ps` fork, so one readiness poll cost N+1 full table reads for N tracked descendants. With execFileSync on that path and a 50 ms poll interval, any command spawning two or more children saturated the host event loop until it exited. ProcessInspector.snapshot() now returns one ProcessSnapshot that answers tree, session, and alive from a single observation, and signalProcess takes the caller's observation so its PID-reuse fence does not re-read the table per member. --- ...26-08-27-process-table-snapshots.i18n.yaml | 6 + .../2026-08-27-process-table-snapshots.md | 67 ++++++++ .../2026-08-27-process-table-snapshots.zh.md | 67 ++++++++ .../subprocess-local/src/process-inspector.ts | 151 +++++++++++++----- .../subprocess-local/src/terminal.ts | 46 +++--- .../subprocess-local/src/windows-inspector.ts | 30 ++-- .../subprocess-local/tests/local.spec.ts | 12 +- .../tests/process-exit.spec.ts | 5 +- .../tests/process-inspector.spec.ts | 38 +++-- .../subprocess-local/tests/terminal.spec.ts | 89 +++++++++-- .../tests/windows-inspector.spec.ts | 28 ++-- .../terminal-bash/tests/session.spec.ts | 10 +- 12 files changed, 417 insertions(+), 132 deletions(-) create mode 100644 .agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.i18n.yaml create mode 100644 .agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.md create mode 100644 .agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.zh.md diff --git a/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.i18n.yaml b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.i18n.yaml new file mode 100644 index 0000000000..01a60f2417 --- /dev/null +++ b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.md +2026-08-27-process-table-snapshots.md: 2f031cc2952ffe1c007e04acf4dabbeac0630630 +2026-08-27-process-table-snapshots.zh.md: fbad15c306d250aeb64247a301ed20777f39c4a3 diff --git a/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.md b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.md new file mode 100644 index 0000000000..2f031cc295 --- /dev/null +++ b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.md @@ -0,0 +1,67 @@ +# Agent Note: Process-table snapshots replace per-question inspector reads + +Status: implemented + +English | [中文](2026-08-27-process-table-snapshots.zh.md) + +## Problem + +A terminal readiness poll asks the platform three questions: the shell's descendant tree, its POSIX session membership, and whether each tracked descendant is still running. When each question reads the process table independently, the poll's cost scales with the number of descendants the running command spawned. + +On macOS each read is a `/bin/ps -axo` fork that parses the entire table — 14.33 ms for 795 processes on the measured host. `LocalTerminalHandle.inspectForeground()` reads the tree once and then asks liveness once per tracked descendant, so one poll costs N+1 table reads for N descendants. `dsh-terminal-bash` polls every 50 ms for up to 30 s, and `ProcessInspectorInternals.exec` is `execFileSync`, so each poll blocks the event loop for its full duration. + +Measured by driving the production `MacProcessInspector` against a real process tree: + +| tracked descendants | one poll | share of the 50 ms interval | +|---|---|---| +| 0 | 18.0 ms | 36% | +| 1 | 33.8 ms | 68% | +| 2 | 49.1 ms | 98% | +| 5 | 87.1 ms | 174% | +| 10 | 178.4 ms | 357% | + +Any command spawning two or more children — a pipeline, `make`, `pnpm`, `git` — saturates the host event loop until it exits. + +Teardown has the same structure. `signalProcess` fences each signal against PID reuse by asking liveness itself, so signalling N members costs N table reads. + +## Decision + +`ProcessInspector.snapshot()` returns a `ProcessSnapshot`, one observation of the process table that answers `tree(rootPid)`, `session(sessionId)`, and `alive(identity)`. It replaces the three per-question methods; the inspector's remaining surface is `foregroundPgid`, `isStdinWaiting`, `signalGroup`, and `signalProcess`. + +Each caller captures one snapshot and answers every question of a single pass from it. `LocalTerminalHandle.descendants()` takes a snapshot, reads the tree and session from it, and filters survivors through the same `alive`, so a readiness poll costs one table read regardless of descendant count. `waitForMembers` captures a fresh snapshot per polling iteration, because its whole purpose is observing change. + +`signalProcess(identity, signal, observed)` takes the caller's observation rather than reading the table itself. The PID-reuse fence stays, and `signalMembers` now captures once for a whole signalling round instead of once per member. Passing the observation explicitly is what keeps Linux teardown from regressing: `alive` there is answered from a `/proc` walk the snapshot already paid for, not from a fresh walk per member. + +Platform differences live in how a snapshot is built, not in what it promises: + +- **macOS** builds it from one `ps` table. That table exposes neither a session id nor a state column, so `session` is empty and `alive` reports presence with a matching start identity. +- **Linux** walks `/proc` once, carrying each entry's parent, start identity, session, and state. `alive` treats the `Z`, `X`, and `x` states as quiescent, as a per-pid `stat` read did. +- **Windows** captures the Toolhelp32 enumeration for `tree`, has no POSIX sessions, and answers `alive` from the live process handle, because wait state is not a table column there. + +`PosixProcessSnapshot` holds both POSIX shapes: a row's `session` and `state` are `undefined` where the platform's table omits them, which is what makes the macOS answers fall out of the shared implementation instead of a second class. + +## Testing + +`packages/subprocess/subprocess-local/tests/terminal.spec.ts` drives a real `MacProcessInspector` over an injected `exec` and asserts one foreground inspection performs exactly one `-axo` table read at 0, 2, and 10 descendants. That count, not wall time, is the durable invariant: it holds on any host and fails the moment a caller re-reads the table per member. + +## Alternatives considered + +**A batched `aliveMembers(members)` call, leaving the other methods alone.** This collapses the per-member reads and is a much smaller edit, but the tree read stays separate, so a macOS poll still forks `ps` twice plus the `tpgid` read — about 32 ms at 10 descendants, still 64% of the 50 ms interval. The event loop remains mostly blocked, so the measured problem survives the fix. + +**Caching the macOS table inside `MacProcessInspector` behind a short TTL.** This needs no interface change, but it makes staleness invisible: a caller cannot tell whether a liveness answer came from this instant or from the end of the previous poll, and a signal decided on a stale row is exactly what the PID-reuse fence exists to prevent. Hidden caching also conflicts with the repository's preference for explicit defaulting and explicit boundaries. + +**Keeping `isAlive` on the inspector next to `snapshot()`.** This avoids touching the signalling call sites, at the cost of two ways to ask one question, where only one of them is cheap in a loop. The asymmetry would have to be re-explained at every call site. + +**Making `exec` asynchronous instead of reducing the read count.** An async `execFile` stops the poll from blocking the loop but still forks N+1 processes per poll; on a busy machine that trades a stall for sustained fork pressure. It remains a worthwhile follow-up on top of the reduced count, not a substitute for it. + +## Consequences + +A readiness poll's process-table cost is now constant in descendant count. On macOS one poll performs one full table read plus the small `tpgid` read, which is the 0-descendant cost in the table above for every descendant count. + +Liveness for a single identity on Linux costs a full `/proc` walk rather than one `stat` read. Every caller that asks about several identities amortizes that walk across them, which is why `signalProcess` takes an observation rather than capturing its own; a future caller that genuinely needs one isolated liveness answer pays more than it did. + +A snapshot is a point-in-time view, and the type's documentation says so. Holding one across an `await` and then signalling from it would widen the PID-reuse window that the fence narrows; `waitForMembers` re-captures per iteration for exactly this reason. + +Every `ProcessInspector` implementation and test fake carries the new shape, including the Windows inspector and the `dsh-terminal-bash` session fake. Test fakes that previously replaced `processTree`, `processSession`, or `isAlive` to stage a scan now replace the corresponding per-question read hook, which keeps their staging behavior and call-counting identical. + +The synchronous `execFileSync` boundary and the fixed 50 ms poll interval are unchanged; both remain open follow-ups for the same readiness path. diff --git a/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.zh.md b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.zh.md new file mode 100644 index 0000000000..fbad15c306 --- /dev/null +++ b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.zh.md @@ -0,0 +1,67 @@ +# Agent Note: Process-table snapshots replace per-question inspector reads + +Status: implemented + +[English](2026-08-27-process-table-snapshots.md) | 中文 + +## Problem + +一次终端就绪轮询要向平台问三个问题：shell 的子进程树、它的 POSIX 会话成员、以及每个被跟踪的子进程是否仍在运行。当每个问题各自去读一次进程表时，这次轮询的代价就随着当前命令派生出的子进程数量增长。 + +在 macOS 上每一次读取都是一次 `/bin/ps -axo` fork，并解析整张表——在实测主机上 795 个进程需要 14.33 ms。`LocalTerminalHandle.inspectForeground()` 读一次树，然后按每个被跟踪的子进程各问一次存活，所以 N 个子进程的一次轮询要读 N+1 次表。`dsh-terminal-bash` 每 50 ms 轮询一次、最长 30 s，而 `ProcessInspectorInternals.exec` 是 `execFileSync`，因此每次轮询在其整个时长内阻塞事件循环。 + +用生产环境的 `MacProcessInspector` 驱动真实进程树实测： + +| 被跟踪的子进程数 | 一次轮询 | 占 50 ms 间隔的比例 | +|---|---|---| +| 0 | 18.0 ms | 36% | +| 1 | 33.8 ms | 68% | +| 2 | 49.1 ms | 98% | +| 5 | 87.1 ms | 174% | +| 10 | 178.4 ms | 357% | + +任何派生两个及以上子进程的命令——一条管道、`make`、`pnpm`、`git`——都会把宿主事件循环打满，直到它退出。 + +拆卸路径的结构相同。`signalProcess` 自己去问存活来给每个信号加 PID 复用围栏，因此向 N 个成员发信号要读 N 次表。 + +## Decision + +`ProcessInspector.snapshot()` 返回一个 `ProcessSnapshot`，即对进程表的一次观察，由它回答 `tree(rootPid)`、`session(sessionId)` 和 `alive(identity)`。它取代了那三个按问题划分的方法；检查器剩下的接口是 `foregroundPgid`、`isStdinWaiting`、`signalGroup` 和 `signalProcess`。 + +每个调用方捕获一次快照，并从中回答本次流程的全部问题。`LocalTerminalHandle.descendants()` 取一次快照，从中读取树与会话，并用同一个 `alive` 过滤幸存者，因此一次就绪轮询无论有多少子进程都只读一次表。`waitForMembers` 每一轮轮询各捕获一次新快照，因为它的用途正是观察变化。 + +`signalProcess(identity, signal, observed)` 接收调用方的观察，而不是自己去读表。PID 复用围栏保留，而 `signalMembers` 现在为整轮信号只捕获一次，而不是每个成员各一次。把观察显式传入正是 Linux 拆卸不退化的原因：那里的 `alive` 由快照已经付过代价的一次 `/proc` 遍历回答，而不是每个成员各遍历一次。 + +平台差异体现在快照如何构建，而不在它承诺什么： + +- **macOS** 由一张 `ps` 表构建。该表既不暴露会话 id 也不暴露状态列，所以 `session` 为空，`alive` 报告的是「存在且起始标识匹配」。 +- **Linux** 遍历一次 `/proc`，携带每个条目的父进程、起始标识、会话与状态。`alive` 把 `Z`、`X`、`x` 状态视为静止，与按 pid 读 `stat` 的判定一致。 +- **Windows** 捕获 Toolhelp32 枚举供 `tree` 使用，没有 POSIX 会话，并且从活的进程句柄回答 `alive`，因为等待状态在那里不是表的一列。 + +`PosixProcessSnapshot` 同时承载两种 POSIX 形态：当平台的表省略某字段时，该行的 `session` 与 `state` 为 `undefined`，这使得 macOS 的答案从共享实现中自然得出，而不必新增一个类。 + +## Testing + +`packages/subprocess/subprocess-local/tests/terminal.spec.ts` 通过注入的 `exec` 驱动真实的 `MacProcessInspector`，断言一次前台检查在 0、2、10 个子进程下都恰好执行一次 `-axo` 表读取。这个次数——而非墙钟时间——才是持久不变量：它在任何主机上都成立，并且在任何调用方按成员重复读表的那一刻失败。 + +## Alternatives considered + +**只加一个批量的 `aliveMembers(members)`，其余方法不动。** 这能合并按成员的读取，改动也小得多，但树的读取仍然独立，因此 macOS 上一次轮询仍要 fork 两次 `ps` 外加 `tpgid` 读取——10 个子进程时约 32 ms，仍占 50 ms 间隔的 64%。事件循环依旧大部分时间被阻塞，实测到的问题在修复之后依然存在。 + +**在 `MacProcessInspector` 内部用短 TTL 缓存 macOS 的表。** 这不需要改接口，但它让陈旧性不可见：调用方无法分辨一个存活答案来自此刻还是来自上一次轮询结束时，而基于陈旧行发出的信号正是 PID 复用围栏要防止的事情。隐式缓存也与仓库偏好显式默认与显式边界的立场冲突。 + +**在 `snapshot()` 旁保留 `isAlive`。** 这样不必改动发信号的调用点，代价是同一个问题有两种问法，而其中只有一种在循环里是廉价的。这种不对称将不得不在每个调用点重新解释一遍。 + +**把 `exec` 改成异步，而不是减少读取次数。** 异步的 `execFile` 能让轮询不再阻塞事件循环，但每次轮询仍然 fork N+1 个进程；在繁忙的机器上这是把一次停顿换成了持续的 fork 压力。它在减少读取次数之上仍是值得做的后续项，而不是它的替代。 + +## Consequences + +一次就绪轮询的进程表代价现在与子进程数量无关。在 macOS 上，一次轮询执行一次完整表读取加一次小的 `tpgid` 读取，也就是上表中 0 子进程那一行的代价，对任意子进程数量都成立。 + +Linux 上查询单个标识的存活，代价从读一个 `stat` 文件变成一次完整的 `/proc` 遍历。每个要查询多个标识的调用方都会把这次遍历摊薄，这正是 `signalProcess` 接收观察而非自行捕获的原因；将来若有调用方确实只需要一次孤立的存活查询，它付出的代价会比过去高。 + +快照是一个时间点视图，该类型的文档也这样声明。跨 `await` 持有一份快照再据此发信号，会扩大围栏本来要收窄的 PID 复用窗口；`waitForMembers` 每轮重新捕获正是为此。 + +每个 `ProcessInspector` 实现与测试替身都采用新形态，包括 Windows 检查器和 `dsh-terminal-bash` 的会话替身。此前通过替换 `processTree`、`processSession` 或 `isAlive` 来编排扫描的测试替身，现在替换对应的按问题读取钩子，其编排行为与调用计数保持不变。 + +同步的 `execFileSync` 边界与固定的 50 ms 轮询间隔未做改动；两者都仍是同一条就绪路径上待办的后续项。 diff --git a/packages/subprocess/subprocess-local/src/process-inspector.ts b/packages/subprocess/subprocess-local/src/process-inspector.ts index 7a74213baf..1c89e8b5f1 100644 --- a/packages/subprocess/subprocess-local/src/process-inspector.ts +++ b/packages/subprocess/subprocess-local/src/process-inspector.ts @@ -16,6 +16,41 @@ interface FileStatus { isCharacterDevice(): boolean } +/** + * One observation of the platform process table, shared by every question a + * single readiness poll or teardown pass asks. + * + * The table is read once, at capture — a `/bin/ps` fork on macOS, a `/proc` + * walk on Linux, a Toolhelp32 enumeration on Windows. Answering {@link tree}, + * {@link session}, or {@link alive} never re-reads it, which is what keeps a + * poll's cost independent of how many descendants the running command spawned. + * Windows liveness additionally consults the live process handle, because wait + * state is not a table column there. + * + * A snapshot is a point-in-time view. Take a fresh one per poll or teardown + * pass; a stale one must never decide that a process is still worth signalling. + */ +export interface ProcessSnapshot { + /** + * Return the root and its transitive descendants as observed, children first. + * @param rootPid - tree root to descend from. + * @returns Observed root and descendants, children before parents. + */ + tree(rootPid: number): ProcessIdentity[] + /** + * Return observed members of one POSIX process session. + * @param sessionId - POSIX session identifier. + * @returns Observed session members, empty where the platform's table omits session ids. + */ + session(sessionId: number): ProcessIdentity[] + /** + * Return whether the exact identity was a non-quiescent process. + * @param identity - PID plus start identity to match. + * @returns Whether that exact identity — not merely that PID — was running. + */ + alive(identity: ProcessIdentity): boolean +} + /** Injectable OS process operations used by one local PTY session. */ export interface ProcessInspector { foregroundPgid(shellPid: number): number | undefined @@ -27,14 +62,19 @@ export interface ProcessInspector { * @returns Whether a group member is blocked reading the shell's terminal input. */ isStdinWaiting(pgid: number, shellPid: number): boolean - /** Return the root and its current transitive descendants, children first. */ - processTree(rootPid: number): ProcessIdentity[] - /** Return current members of one POSIX process session when the platform exposes them. */ - processSession(sessionId: number): ProcessIdentity[] - /** Return whether the exact identity remains a non-quiescent process. */ - isAlive(identity: ProcessIdentity): boolean + /** + * Read the process table once and answer tree, session, and liveness from it. + * @returns A point-in-time process-table observation. + */ + snapshot(): ProcessSnapshot signalGroup(pgid: number, signal: SubprocessTerminalSignal): void - signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL'): void + /** + * Signal one exact process identity, fenced against PID reuse. + * @param identity - PID plus start identity to signal. + * @param signal - termination signal to deliver. + * @param observed - observation the identity fence reads; pass one taken for this teardown pass. + */ + signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL', observed: ProcessSnapshot): void } /** Testable boundary around filesystem, process-table, and signal syscalls. */ @@ -292,16 +332,14 @@ abstract class PosixProcessInspector implements ProcessInspector { abstract foregroundPgid(shellPid: number): number | undefined abstract isStdinWaiting(pgid: number, shellPid: number): boolean - abstract processTree(rootPid: number): ProcessIdentity[] - abstract processSession(sessionId: number): ProcessIdentity[] - abstract isAlive(identity: ProcessIdentity): boolean + abstract snapshot(): ProcessSnapshot signalGroup(pgid: number, signal: SubprocessTerminalSignal): void { this.internals.kill(-pgid, signal) } - signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL'): void { - if (this.isAlive(identity)) this.internals.kill(identity.pid, signal) + signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL', observed: ProcessSnapshot): void { + if (observed.alive(identity)) this.internals.kill(identity.pid, signal) } } @@ -309,6 +347,42 @@ interface ProcessTreeEntry extends ProcessIdentity { parentPid: number } +/** One process-table row, carrying the fields a platform's table exposes. */ +interface ProcessRow extends ProcessTreeEntry { + /** POSIX session identifier, or undefined where the table omits it. */ + session: number | undefined + /** Single-letter process state, or undefined where the table omits it. */ + state: string | undefined +} + +// Zombie and dead states answer "present in the table" but never "still +// running"; a table without a state column can only report presence. +function quiescent(state: string | undefined): boolean { + return state !== undefined && /^[ZXx]$/.test(state) +} + +class PosixProcessSnapshot implements ProcessSnapshot { + private readonly byPid: Map + + constructor(private readonly rows: ProcessRow[]) { + this.byPid = new Map(rows.map(row => [row.pid, row])) + } + + tree(rootPid: number): ProcessIdentity[] { + return processTree(this.rows, rootPid) + } + + session(sessionId: number): ProcessIdentity[] { + return this.rows.flatMap(row => + row.session === sessionId ? [{ pid: row.pid, started: row.started }] : []) + } + + alive(identity: ProcessIdentity): boolean { + const row = this.byPid.get(identity.pid) + return row?.started === identity.started && !quiescent(row.state) + } +} + function processTree(entries: ProcessTreeEntry[], rootPid: number): ProcessIdentity[] { const byPid = new Map(entries.map(entry => [entry.pid, entry])) const root = byPid.get(rootPid) @@ -364,35 +438,34 @@ class LinuxProcessInspector extends PosixProcessInspector { return false } - processTree(rootPid: number): ProcessIdentity[] { - const entries = numericEntries(this.internals, '/proc').flatMap((pid) => { + snapshot(): ProcessSnapshot { + return new PosixProcessSnapshot(numericEntries(this.internals, '/proc').flatMap((pid) => { const stat = readLinuxStat(this.internals, pid) - return stat === undefined ? [] : [{ pid, parentPid: stat.parentPid, started: stat.started }] - }) - return processTree(entries, rootPid) - } - - processSession(sessionId: number): ProcessIdentity[] { - return numericEntries(this.internals, '/proc').flatMap((pid) => { - const stat = readLinuxStat(this.internals, pid) - return stat?.session === sessionId ? [{ pid, started: stat.started }] : [] - }) - } - - isAlive(identity: ProcessIdentity): boolean { - const stat = readLinuxStat(this.internals, identity.pid) - return stat?.started === identity.started && !/^[ZXx]$/.test(stat.state) + return stat === undefined ? [] : [{ + pid, + parentPid: stat.parentPid, + started: stat.started, + session: stat.session, + state: stat.state, + }] + })) } } -interface PsEntry extends ProcessTreeEntry {} - -function macProcessTable(internals: ProcessInspectorInternals): PsEntry[] { +// `ps` exposes neither the session id nor a state column in this format, so a +// macOS row can answer presence and parentage but never session membership. +function macProcessTable(internals: ProcessInspectorInternals): ProcessRow[] { return internals.exec('/bin/ps', ['-axo', 'pid=,ppid=,lstart=']).split('\n').flatMap((line) => { const match = /^\s*(\d+)\s+(\d+)\s+(.+?)\s*$/.exec(line) if (match?.[1] === undefined || match[2] === undefined || match[3] === undefined) return [] - return [{ pid: Number(match[1]), parentPid: Number(match[2]), started: match[3] }] + return [{ + pid: Number(match[1]), + parentPid: Number(match[2]), + started: match[3], + session: undefined, + state: undefined, + }] }) } @@ -410,16 +483,8 @@ class MacProcessInspector extends PosixProcessInspector { return false } - processTree(rootPid: number): ProcessIdentity[] { - return processTree(macProcessTable(this.internals), rootPid) - } - - processSession(_sessionId: number): ProcessIdentity[] { - return [] - } - - isAlive(identity: ProcessIdentity): boolean { - return macProcessTable(this.internals).some(entry => entry.pid === identity.pid && entry.started === identity.started) + snapshot(): ProcessSnapshot { + return new PosixProcessSnapshot(macProcessTable(this.internals)) } } diff --git a/packages/subprocess/subprocess-local/src/terminal.ts b/packages/subprocess/subprocess-local/src/terminal.ts index 80782e24e7..eb6bf618c1 100644 --- a/packages/subprocess/subprocess-local/src/terminal.ts +++ b/packages/subprocess/subprocess-local/src/terminal.ts @@ -10,7 +10,7 @@ import type { SubprocessTerminalHandle, SubprocessTerminalSignal, } from '@deepseek-ai/dsh-subprocess' -import type { ProcessIdentity, ProcessInspector } from './process-inspector.ts' +import type { ProcessIdentity, ProcessInspector, ProcessSnapshot } from './process-inspector.ts' function delay(ms: number): Promise { return new Promise(resolve => setTimeout(resolve, ms)) @@ -59,7 +59,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { private readonly platform: NodeJS.Platform = process.platform, ) { this.pid = terminal.pid - this.rootIdentity = inspector.processTree(this.pid).find(member => member.pid === this.pid) + this.rootIdentity = inspector.snapshot().tree(this.pid).find(member => member.pid === this.pid) this.done = this.outcome.promise this.dataDisposable = terminal.onData((data) => { this.output.write(Buffer.from(data, 'utf8')) }) this.exitDisposable = terminal.onExit(({ exitCode, signal: exitSignal }) => { @@ -83,7 +83,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { // Local inspection is synchronous; the seam returns a promise for remote transports. // oxlint-disable-next-line typescript/require-await -- Preserve promise rejection semantics at the async provider contract. async inspectForeground(): Promise { - this.descendants() + this.descendants(this.inspector.snapshot()) const processGroupId = this.inspector.foregroundPgid(this.pid) if (processGroupId === undefined) return undefined return { @@ -139,7 +139,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { if (this.exited) return if (this.rootIdentity !== undefined) { try { - this.inspector.signalProcess(this.rootIdentity, 'SIGKILL') + this.inspector.signalProcess(this.rootIdentity, 'SIGKILL', this.inspector.snapshot()) } catch (_rootExitedDuringHostExit) { // Exact identity signalling contains both exit races and PID reuse. } @@ -152,42 +152,43 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { } } - private survivors(members: ProcessIdentity[]): ProcessIdentity[] { - return members.filter(member => this.inspector.isAlive(member)) + private survivors(members: ProcessIdentity[], observed: ProcessSnapshot): ProcessIdentity[] { + return members.filter(member => observed.alive(member)) } - private descendants(): ProcessIdentity[] { + private descendants(observed: ProcessSnapshot): ProcessIdentity[] { // Adopt newly scanned members only while the numeric root pid provably // still carries the spawned shell's start identity: after the shell dies, // a recycled pid's tree and session must not donate an unrelated // process's children to this session's signalling. Already-adopted // members keep their own start identities, which every signal rechecks. - const tree = this.inspector.processTree(this.pid) + const tree = observed.tree(this.pid) const root = tree.find(member => member.pid === this.pid) const rootVerified = this.rootIdentity !== undefined && root !== undefined && root.started === this.rootIdentity.started this.trackedDescendants = this.survivors(this.unionMembers( this.trackedDescendants, - ...rootVerified ? [tree, this.inspector.processSession(this.pid)] : [], - ).filter(member => member.pid !== this.pid)) + ...rootVerified ? [tree, observed.session(this.pid)] : [], + ).filter(member => member.pid !== this.pid), observed) return this.trackedDescendants } private async waitForMembers(members: ProcessIdentity[]): Promise { const until = Date.now() + this.graceMs - let survivors = this.survivors(members) + let survivors = this.survivors(members, this.inspector.snapshot()) while (survivors.length > 0 && Date.now() < until) { await delay(Math.min(25, Math.max(1, until - Date.now()))) - survivors = this.survivors(members) + survivors = this.survivors(members, this.inspector.snapshot()) } return survivors } private signalMembers(members: ProcessIdentity[], signal: 'SIGTERM' | 'SIGKILL'): void { + const observed = this.inspector.snapshot() for (const member of members) { try { - this.inspector.signalProcess(member, signal) + this.inspector.signalProcess(member, signal, observed) } catch (_alreadyExitedDuringSignal) { // The exact process identity is rechecked; a same-tick exit is success. } @@ -197,7 +198,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { private forceStopDescendants(): void { let members = this.trackedDescendants try { - members = this.descendants() + members = this.descendants(this.inspector.snapshot()) } catch (_processTableUnavailableDuringHostExit) { // Preserve already-captured identities when a final process-table scan fails. } @@ -219,13 +220,14 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { } private async stopDescendants(): Promise { - const captured = this.descendants() + const captured = this.descendants(this.inspector.snapshot()) this.signalMembers(captured, 'SIGTERM') const capturedSurvivors = await this.waitForMembers(captured) - const members = this.unionMembers(capturedSurvivors, this.descendants()) + const members = this.unionMembers(capturedSurvivors, this.descendants(this.inspector.snapshot())) this.signalMembers(members, 'SIGKILL') const survivors = await this.waitForMembers(members) - return this.survivors(this.unionMembers(survivors, this.descendants())) + const observed = this.inspector.snapshot() + return this.survivors(this.unionMembers(survivors, this.descendants(observed)), observed) } private async stopShell(): Promise { @@ -262,9 +264,9 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { // (the same console-list agent), so the tiers verify the shell's absence // through the inspector instead of waiting on `done` alone. const shellGone = (): boolean => - this.exited || (this.rootIdentity !== undefined && !this.inspector.isAlive(this.rootIdentity)) + this.exited || (this.rootIdentity !== undefined && !this.inspector.snapshot().alive(this.rootIdentity)) if (!shellGone() && this.rootIdentity !== undefined) { - this.inspector.signalProcess(this.rootIdentity, 'SIGTERM') + this.inspector.signalProcess(this.rootIdentity, 'SIGTERM', this.inspector.snapshot()) await this.waitForWindowsShellExit() } if (!shellGone() && this.rootIdentity === undefined) { @@ -276,7 +278,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { await Promise.race([this.done.then(() => undefined), delay(this.graceMs)]) } if (!shellGone() && this.rootIdentity !== undefined) { - this.inspector.signalProcess(this.rootIdentity, 'SIGKILL') + this.inspector.signalProcess(this.rootIdentity, 'SIGKILL', this.inspector.snapshot()) await this.waitForWindowsShellExit() } if (!shellGone()) throw new Error(`terminal cleanup failed; surviving pid: ${this.pid}`) @@ -285,7 +287,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { private async waitForWindowsShellExit(): Promise { const until = Date.now() + this.graceMs while (!this.exited && Date.now() < until) { - if (this.rootIdentity !== undefined && !this.inspector.isAlive(this.rootIdentity)) return + if (this.rootIdentity !== undefined && !this.inspector.snapshot().alive(this.rootIdentity)) return await delay(Math.min(25, Math.max(1, until - Date.now()))) } } @@ -315,7 +317,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { if (this.exited) return /* v8 ignore next -- stopShellWindows() verified the shell is gone or threw; the identity re-check is a defensive fence for a future caller. */ - if (this.rootIdentity !== undefined && this.inspector.isAlive(this.rootIdentity)) return + if (this.rootIdentity !== undefined && this.inspector.snapshot().alive(this.rootIdentity)) return this.exited = true this.output.end() this.outcome.resolve({ exitCode: null, signal: null }) diff --git a/packages/subprocess/subprocess-local/src/windows-inspector.ts b/packages/subprocess/subprocess-local/src/windows-inspector.ts index 6889a3e65d..7b2ae5a23d 100644 --- a/packages/subprocess/subprocess-local/src/windows-inspector.ts +++ b/packages/subprocess/subprocess-local/src/windows-inspector.ts @@ -12,7 +12,7 @@ import { spawnSync } from 'node:child_process' import koffi from 'koffi' import type { SubprocessTerminalSignal } from '@deepseek-ai/dsh-subprocess' -import type { ProcessIdentity, ProcessInspector } from './process-inspector.ts' +import type { ProcessIdentity, ProcessInspector, ProcessSnapshot } from './process-inspector.ts' /** One Toolhelp32 process-table row. */ export interface ProcessEntry { @@ -97,25 +97,27 @@ export class WindowsProcessInspector implements ProcessInspector { return false } - processTree(rootPid: number): ProcessIdentity[] { - return windowsProcessTree(this.internals.snapshot(), rootPid, pid => this.internals.processState(pid)?.started) - } - - processSession(_sessionId: number): ProcessIdentity[] { - return [] - } - - isAlive(identity: ProcessIdentity): boolean { - const state = this.internals.processState(identity.pid) - return state?.active === true && state.started === identity.started + snapshot(): ProcessSnapshot { + const entries = this.internals.snapshot() + return { + tree: rootPid => windowsProcessTree(entries, rootPid, pid => this.internals.processState(pid)?.started), + // Windows has no POSIX sessions; the shell pid stands in as a pseudo group. + session: () => [], + alive: (identity) => { + // Wait state is a per-handle question, not a Toolhelp32 column, so + // liveness reads the live process object rather than `entries`. + const state = this.internals.processState(identity.pid) + return state?.active === true && state.started === identity.started + }, + } } signalGroup(pgid: number, signal: SubprocessTerminalSignal): void { this.internals.taskkill(pgid, signal === 'SIGKILL') } - signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL'): void { - if (this.isAlive(identity)) this.internals.taskkill(identity.pid, signal === 'SIGKILL') + signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL', observed: ProcessSnapshot): void { + if (observed.alive(identity)) this.internals.taskkill(identity.pid, signal === 'SIGKILL') } } /* jscpd:ignore-end */ diff --git a/packages/subprocess/subprocess-local/tests/local.spec.ts b/packages/subprocess/subprocess-local/tests/local.spec.ts index 41e6b48bc8..589102ccf8 100644 --- a/packages/subprocess/subprocess-local/tests/local.spec.ts +++ b/packages/subprocess/subprocess-local/tests/local.spec.ts @@ -302,9 +302,7 @@ describe('LocalSubprocessRuntime', () => { const inspector = { foregroundPgid: () => undefined, isStdinWaiting: () => false, - processTree: () => [], - processSession: () => [], - isAlive: () => false, + snapshot: () => ({ tree: () => [], session: () => [], alive: () => false }), signalGroup: () => {}, signalProcess: () => {}, } @@ -369,9 +367,11 @@ describe('LocalSubprocessRuntime', () => { ;(ctx.subprocess as InstanceType).terminalInspector = { foregroundPgid: () => 123, isStdinWaiting: () => false, - processTree: () => [{ pid: 123, started: 'shell' }, { pid: 124, started: 'child' }], - processSession: () => [], - isAlive: identity => alive.has(identity.pid), + snapshot: () => ({ + tree: () => [{ pid: 123, started: 'shell' }, { pid: 124, started: 'child' }], + session: () => [], + alive: identity => alive.has(identity.pid), + }), signalGroup: () => {}, signalProcess: () => {}, } diff --git a/packages/subprocess/subprocess-local/tests/process-exit.spec.ts b/packages/subprocess/subprocess-local/tests/process-exit.spec.ts index cfea99f12a..c00c666e33 100644 --- a/packages/subprocess/subprocess-local/tests/process-exit.spec.ts +++ b/packages/subprocess/subprocess-local/tests/process-exit.spec.ts @@ -42,7 +42,7 @@ async function readTree(path: string): Promise { async function captureIdentities(inspector: ProcessInspector, state: TreeState): Promise { return vi.waitFor(() => { const expected = new Set([state.root, state.descendant]) - const identities = inspector.processTree(state.root).filter(identity => expected.has(identity.pid)) + const identities = inspector.snapshot().tree(state.root).filter(identity => expected.has(identity.pid)) if (identities.length !== expected.size) throw new Error('managed tree is not fully observable yet') return identities }, { interval: 10, timeout: scenarioTimeoutMs }) @@ -68,9 +68,10 @@ function cleanupTree(state: TreeState | undefined, identities: ProcessIdentity[] return } const inspector = createProcessInspector() + const observed = inspector.snapshot() for (const identity of identities) { try { - inspector.signalProcess(identity, 'SIGKILL') + inspector.signalProcess(identity, 'SIGKILL', observed) } catch (_alreadyGone) { // Exact start identity prevents PID-reuse cleanup from reaching another process. } diff --git a/packages/subprocess/subprocess-local/tests/process-inspector.spec.ts b/packages/subprocess/subprocess-local/tests/process-inspector.spec.ts index ab34922576..ac5c95c13f 100644 --- a/packages/subprocess/subprocess-local/tests/process-inspector.spec.ts +++ b/packages/subprocess/subprocess-local/tests/process-inspector.spec.ts @@ -121,28 +121,31 @@ describe('Linux process inspector', () => { expect(inspector.foregroundPgid(10)).toBe(40) expect(inspector.foregroundPgid(11)).toBeUndefined() expect(inspector.foregroundPgid(99)).toBeUndefined() - expect(inspector.processTree(10)).toEqual([ + const observed = inspector.snapshot() + expect(observed.tree(10)).toEqual([ { pid: 13, started: '503' }, { pid: 12, started: '502' }, { pid: 10, started: '500' }, ]) - expect(inspector.processTree(99)).toEqual([]) - expect(inspector.processSession(30)).toEqual([ + expect(observed.tree(99)).toEqual([]) + expect(observed.session(30)).toEqual([ { pid: 10, started: '500' }, { pid: 11, started: '501' }, { pid: 12, started: '502' }, { pid: 13, started: '503' }, ]) - expect(inspector.processSession(99)).toEqual([]) - expect(inspector.isAlive({ pid: 10, started: '500' })).toBe(true) - expect(inspector.isAlive({ pid: 10, started: 'old' })).toBe(false) + expect(observed.session(99)).toEqual([]) + expect(observed.alive({ pid: 10, started: '500' })).toBe(true) + expect(observed.alive({ pid: 10, started: 'old' })).toBe(false) inspector.signalGroup(40, 'SIGINT') - inspector.signalProcess({ pid: 10, started: '500' }, 'SIGTERM') - inspector.signalProcess({ pid: 10, started: 'old' }, 'SIGKILL') + inspector.signalProcess({ pid: 10, started: '500' }, 'SIGTERM', observed) + inspector.signalProcess({ pid: 10, started: 'old' }, 'SIGKILL', observed) expect(fake.kills).toEqual([[-40, 'SIGINT'], [10, 'SIGTERM']]) fake.files.set('/proc/10/stat', stat(10, 20, 30, 40, '500', 1, 'Z')) - expect(inspector.isAlive({ pid: 10, started: '500' })).toBe(false) - inspector.signalProcess({ pid: 10, started: '500' }, 'SIGKILL') + // A zombie is present in the table but never signallable; a fresh capture sees the new state. + const afterExit = inspector.snapshot() + expect(afterExit.alive({ pid: 10, started: '500' })).toBe(false) + inspector.signalProcess({ pid: 10, started: '500' }, 'SIGKILL', afterExit) expect(fake.kills).toEqual([[-40, 'SIGINT'], [10, 'SIGTERM']]) }) @@ -285,21 +288,22 @@ describe('macOS process inspector', () => { const inspector = createProcessInspector('darwin', 'arm64', fake.internals) expect(inspector.foregroundPgid(10)).toBe(55) expect(inspector.isStdinWaiting(55, 10)).toBe(false) - expect(inspector.processTree(10)).toEqual([ + const observed = inspector.snapshot() + expect(observed.tree(10)).toEqual([ { pid: 12, started: 'Mon Jul 21 10:00:02 2026' }, { pid: 11, started: 'Mon Jul 21 10:00:01 2026' }, { pid: 10, started: 'Mon Jul 21 10:00:00 2026' }, ]) - expect(inspector.processTree(99)).toEqual([]) - expect(inspector.processSession(10)).toEqual([]) - expect(inspector.isAlive({ pid: 11, started: 'Mon Jul 21 10:00:01 2026' })).toBe(true) + expect(observed.tree(99)).toEqual([]) + expect(observed.session(10)).toEqual([]) + expect(observed.alive({ pid: 11, started: 'Mon Jul 21 10:00:01 2026' })).toBe(true) inspector.signalGroup(55, 'SIGTSTP') - inspector.signalProcess({ pid: 11, started: 'Mon Jul 21 10:00:01 2026' }, 'SIGKILL') - inspector.signalProcess({ pid: 12, started: 'missing' }, 'SIGTERM') + inspector.signalProcess({ pid: 11, started: 'Mon Jul 21 10:00:01 2026' }, 'SIGKILL', observed) + inspector.signalProcess({ pid: 12, started: 'missing' }, 'SIGTERM', observed) expect(fake.kills).toEqual([[-55, 'SIGTSTP'], [11, 'SIGKILL']]) fake.setPs(' 10 11 Mon Jul 21 10:00:00 2026\n 11 10 Mon Jul 21 10:00:01 2026\n') - expect(inspector.processTree(10)).toEqual([ + expect(inspector.snapshot().tree(10)).toEqual([ { pid: 11, started: 'Mon Jul 21 10:00:01 2026' }, { pid: 10, started: 'Mon Jul 21 10:00:00 2026' }, ]) diff --git a/packages/subprocess/subprocess-local/tests/terminal.spec.ts b/packages/subprocess/subprocess-local/tests/terminal.spec.ts index 330660eda3..75beef6a34 100644 --- a/packages/subprocess/subprocess-local/tests/terminal.spec.ts +++ b/packages/subprocess/subprocess-local/tests/terminal.spec.ts @@ -1,9 +1,12 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import type { IDisposable, IPty } from 'node-pty' import { LocalTerminalHandle } from '@deepseek-ai/dsh-subprocess-local/src/terminal.ts' +import { createProcessInspector } from '@deepseek-ai/dsh-subprocess-local/src/process-inspector.ts' import type { ProcessIdentity, ProcessInspector, + ProcessInspectorInternals, + ProcessSnapshot, } from '@deepseek-ai/dsh-subprocess-local/src/process-inspector.ts' import type { SubprocessTerminalSignal } from '@deepseek-ai/dsh-subprocess' @@ -69,18 +72,27 @@ class FakeInspector implements ProcessInspector { this.stdinChecks.push([pgid, shellPid]) return this.waiting } - processTree() { return this.root === undefined ? this.members : [this.root, ...this.members] } - processSession() { return this.sessionMembers } - isAlive(identity: ProcessIdentity) { return this.alive.has(identity.pid) } + /** Per-question table reads; tests replace one to stage a scan without rebuilding the fake. */ + readTree: () => ProcessIdentity[] = () => this.root === undefined ? this.members : [this.root, ...this.members] + readSession: () => ProcessIdentity[] = () => this.sessionMembers + readAlive: (identity: ProcessIdentity) => boolean = identity => this.alive.has(identity.pid) + + snapshot(): ProcessSnapshot { + return { + tree: () => this.readTree(), + session: () => this.readSession(), + alive: identity => this.readAlive(identity), + } + } signalGroup(pgid: number, signal: SubprocessTerminalSignal) { if (this.throwGroup) throw new Error('group failed') this.groups.push([pgid, signal]) } - signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL') { + signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL', observed: ProcessSnapshot) { // Mirrors the real inspectors' alive-gated signalling. if (!this.alive.has(identity.pid)) return if (this.throwProcess) throw new Error('process raced') - if (!this.isAlive(identity)) return + if (!observed.alive(identity)) return this.processes.push([identity.pid, signal]) if (this.removeOnSignal) this.alive.delete(identity.pid) } @@ -104,8 +116,8 @@ describe('LocalTerminalHandle', () => { inspector.alive.add(pty.pid) inspector.alive.add(first.pid) const signalProcess = inspector.signalProcess.bind(inspector) - inspector.signalProcess = (identity, signal) => { - signalProcess(identity, signal) + inspector.signalProcess = (identity, signal, observed) => { + signalProcess(identity, signal, observed) if (identity.pid === pty.pid) { inspector.members = [first, late] inspector.alive.add(late.pid) @@ -135,7 +147,7 @@ describe('LocalTerminalHandle', () => { inspector.alive.add(captured.pid) const handle = new LocalTerminalHandle(pty.asPty(), inspector, 10) await handle.inspectForeground() - inspector.processTree = () => { throw new Error('process table unavailable') } + inspector.readTree = () => { throw new Error('process table unavailable') } inspector.throwProcess = true expect(() => { handle.terminateForHostExit() }).not.toThrow() @@ -166,7 +178,7 @@ describe('LocalTerminalHandle', () => { inspector.alive.add(pty.pid) const handle = new LocalTerminalHandle(pty.asPty(), inspector, 10) inspector.root = { pid: pty.pid, started: 'recycled' } - inspector.isAlive = identity => identity.started === 'recycled' + inspector.readAlive = identity => identity.started === 'recycled' handle.terminateForHostExit() @@ -258,7 +270,7 @@ describe('LocalTerminalHandle', () => { const pty = new FakePty() const inspector = new FakeInspector() const disowned = { pid: 124, started: 'disowned' } - inspector.processSession = () => inspector.alive.has(disowned.pid) ? [disowned] : [] + inspector.readSession = () => inspector.alive.has(disowned.pid) ? [disowned] : [] inspector.alive.add(124) const handle = makeHandle(pty, inspector, 20) @@ -318,7 +330,7 @@ describe('LocalTerminalHandle', () => { const inspector = new FakeInspector() const root = { pid: 123, started: 'shell' } let reads = 0 - inspector.processTree = () => { + inspector.readTree = () => { reads += 1 if (reads === 1) return [root] if (reads === 2) { @@ -385,7 +397,7 @@ describe('LocalTerminalHandle', () => { const root = { pid: 123, started: 'shell' } let reads = 0 inspector.alive.add(captured.pid) - inspector.processTree = () => { reads += 1; return reads === 1 ? [root] : reads === 2 ? [root, captured] : [] } + inspector.readTree = () => { reads += 1; return reads === 1 ? [root] : reads === 2 ? [root, captured] : [] } inspector.signalProcess = (identity, signal) => { inspector.processes.push([identity.pid, signal]) if (signal === 'SIGKILL') inspector.alive.delete(identity.pid) @@ -514,3 +526,56 @@ describe('LocalTerminalHandle on Windows', () => { expect(inspector.processes).toEqual([]) }) }) + +describe('process-table read amplification', () => { + // The macOS inspector answers every question by forking `/bin/ps`, so a + // readiness poll that asks per descendant scales its blocking cost with the + // command's process tree. These pin the read count, not the wall time. + function darwinInternals(table: string): { internals: ProcessInspectorInternals; tableReads: string[] } { + const tableReads: string[] = [] + const unreachable = (): never => { throw new Error('darwin inspection uses exec and kill only') } + return { + tableReads, + internals: { + readFile: unreachable, + readDir: unreachable, + readLink: unreachable, + stat: unreachable, + open: unreachable, + read: unreachable, + close: unreachable, + exec(_file, args) { + if (args.includes('tpgid=')) return '456\n' + tableReads.push(args.join(' ')) + return table + }, + kill() {}, + }, + } + } + + /** A shell at pid 123 with `count` descendants chained beneath it. */ + function shellTable(count: number): string { + const rows = [' 123 1 Mon Jul 21 10:00:00 2026'] + for (let index = 0; index < count; index += 1) { + rows.push(` ${String(124 + index)} ${String(123 + index)} Mon Jul 21 10:00:${String(index + 1).padStart(2, '0')} 2026`) + } + return `${rows.join('\n')}\n` + } + + async function tableReadsForOnePoll(descendants: number): Promise { + const { internals, tableReads } = darwinInternals(shellTable(descendants)) + const inspector = createProcessInspector('darwin', 'arm64', internals) + const handle = new LocalTerminalHandle(new FakePty().asPty(), inspector, 10, 'darwin') + tableReads.length = 0 + const foreground = await handle.inspectForeground() + expect(foreground).toEqual({ processGroupId: 456, inputWaiting: false }) + return tableReads.length + } + + it('reads the macOS process table once per foreground inspection regardless of descendant count', async () => { + expect(await tableReadsForOnePoll(0)).toBe(1) + expect(await tableReadsForOnePoll(2)).toBe(1) + expect(await tableReadsForOnePoll(10)).toBe(1) + }) +}) diff --git a/packages/subprocess/subprocess-local/tests/windows-inspector.spec.ts b/packages/subprocess/subprocess-local/tests/windows-inspector.spec.ts index 5950fd9328..c2129923b4 100644 --- a/packages/subprocess/subprocess-local/tests/windows-inspector.spec.ts +++ b/packages/subprocess/subprocess-local/tests/windows-inspector.spec.ts @@ -66,7 +66,7 @@ describe('WindowsProcessInspector (injected internals)', () => { const inspector = new WindowsProcessInspector(fake.internals) expect(inspector.foregroundPgid(77)).toBe(77) expect(inspector.isStdinWaiting(77, 10)).toBe(false) - expect(inspector.processSession(77)).toEqual([]) + expect(inspector.snapshot().session(77)).toEqual([]) }) it('delegates tree walks and identity checks to the internals', () => { @@ -74,16 +74,16 @@ describe('WindowsProcessInspector (injected internals)', () => { fake.add({ pid: 10, parentPid: 0 }, 't10') fake.add({ pid: 11, parentPid: 10 }, 't11') const inspector = new WindowsProcessInspector(fake.internals) - expect(inspector.processTree(10)).toEqual([ + expect(inspector.snapshot().tree(10)).toEqual([ { pid: 11, started: 't11' }, { pid: 10, started: 't10' }, ]) - expect(inspector.isAlive({ pid: 11, started: 't11' })).toBe(true) - expect(inspector.isAlive({ pid: 11, started: 'stale' })).toBe(false) - expect(inspector.isAlive({ pid: 99, started: 't99' })).toBe(false) + expect(inspector.snapshot().alive({ pid: 11, started: 't11' })).toBe(true) + expect(inspector.snapshot().alive({ pid: 11, started: 'stale' })).toBe(false) + expect(inspector.snapshot().alive({ pid: 99, started: 't99' })).toBe(false) fake.add({ pid: 12, parentPid: 10 }, 't12', false) - expect(inspector.isAlive({ pid: 12, started: 't12' })).toBe(false) + expect(inspector.snapshot().alive({ pid: 12, started: 't12' })).toBe(false) }) it('maps SIGKILL to a forced taskkill and other signals to the grace form', () => { @@ -100,9 +100,9 @@ describe('WindowsProcessInspector (injected internals)', () => { fake.add({ pid: 10, parentPid: 0 }, 't10') fake.add({ pid: 11, parentPid: 10 }, 't11', false) const inspector = new WindowsProcessInspector(fake.internals) - inspector.signalProcess({ pid: 10, started: 't10' }, 'SIGKILL') - inspector.signalProcess({ pid: 11, started: 't11' }, 'SIGKILL') - inspector.signalProcess({ pid: 10, started: 'stale' }, 'SIGTERM') + inspector.signalProcess({ pid: 10, started: 't10' }, 'SIGKILL', inspector.snapshot()) + inspector.signalProcess({ pid: 11, started: 't11' }, 'SIGKILL', inspector.snapshot()) + inspector.signalProcess({ pid: 10, started: 'stale' }, 'SIGTERM', inspector.snapshot()) expect(fake.kills).toEqual([[10, true]]) }) @@ -130,19 +130,21 @@ const win32 = process.platform === 'win32' ? describe : describe.skip win32('WindowsProcessInspector over the real koffi bindings', () => { it('walks the live process table from the test runner itself', () => { const inspector = createWindowsProcessInspector() - const tree = inspector.processTree(process.pid) + const tree = inspector.snapshot().tree(process.pid) const self = tree.find(member => member.pid === process.pid) expect(self).toBeDefined() - expect(inspector.isAlive(self!)).toBe(true) + expect(inspector.snapshot().alive(self!)).toBe(true) expect(inspector.foregroundPgid(process.pid)).toBe(process.pid) }) it('reports unreadable identities for absent processes and no-ops tree signalling', () => { const inspector = createWindowsProcessInspector() - expect(inspector.isAlive({ pid: 0x7FFFFFFF, started: 'absent' })).toBe(false) + expect(inspector.snapshot().alive({ pid: 0x7FFFFFFF, started: 'absent' })).toBe(false) expect(() => { inspector.signalGroup(0x7FFFFFFF, 'SIGKILL') }).not.toThrow() expect(() => { inspector.signalGroup(0x7FFFFFFF, 'SIGTERM') }).not.toThrow() expect(() => { inspector.signalGroup(0, 'SIGKILL') }).not.toThrow() - expect(() => { inspector.signalProcess({ pid: 0x7FFFFFFF, started: 'absent' }, 'SIGKILL') }).not.toThrow() + expect(() => { + inspector.signalProcess({ pid: 0x7FFFFFFF, started: 'absent' }, 'SIGKILL', inspector.snapshot()) + }).not.toThrow() }) }) diff --git a/packages/terminal/terminal-bash/tests/session.spec.ts b/packages/terminal/terminal-bash/tests/session.spec.ts index 897690cbcb..02ce95db02 100644 --- a/packages/terminal/terminal-bash/tests/session.spec.ts +++ b/packages/terminal/terminal-bash/tests/session.spec.ts @@ -27,9 +27,13 @@ class FakeInspector implements ProcessInspector { foregroundPgid() { return this.pgid } isStdinWaiting() { return this.waiting } - processTree() { return this.members } - processSession() { return [] } - isAlive(identity: ProcessIdentity) { return this.alive.has(identity.pid) } + snapshot() { + return { + tree: () => this.members, + session: () => [], + alive: (identity: ProcessIdentity) => this.alive.has(identity.pid), + } + } signalGroup(pgid: number, signal: TerminalSignal) { if (this.throwGroup) throw new Error('group failed') this.groups.push([pgid, signal]) From 9757224349549fab13af1f7d44dfd70f6940a28e Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Thu, 27 Aug 2026 15:19:43 +0800 Subject: [PATCH 2/2] fix(subprocess): fence each signal against current process state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the shared observation defeated the very fence it fed: it carries the original PID-to-start-time pairing forward, so a recycled PID still matches it and takes a signal meant for the process that exited. Capturing it outside the per-member try also let one failed read abort a whole teardown round, breaking the synchronous host-exit contract, and an empty round paid a read for no members. signalProcess now reads ProcessInspector.isAlive immediately before delivering, from the narrowest per-identity source each platform offers; signalMembers and waitForMembers return before capturing when a round has no members. snapshot() keeps serving the readiness poll, whose per-poll table read stays at one. Windows enumerates Toolhelp32 lazily on the first tree question, so a snapshot asked only for liveness — the 25 ms teardown poll — performs no table walk at all. --- ...26-08-27-process-table-snapshots.i18n.yaml | 4 +- .../2026-08-27-process-table-snapshots.md | 16 ++++-- .../2026-08-27-process-table-snapshots.zh.md | 16 ++++-- .../subprocess-local/src/process-inspector.ts | 55 ++++++++++++++----- .../subprocess-local/src/terminal.ts | 18 +++--- .../subprocess-local/src/windows-inspector.ts | 27 +++++---- .../subprocess-local/tests/local.spec.ts | 2 + .../tests/process-exit.spec.ts | 3 +- .../tests/process-inspector.spec.ts | 31 ++++++++--- .../subprocess-local/tests/terminal.spec.ts | 47 ++++++++++++++-- .../tests/windows-inspector.spec.ts | 49 ++++++++++++----- .../terminal-bash/tests/session.spec.ts | 1 + 12 files changed, 196 insertions(+), 73 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.i18n.yaml b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.i18n.yaml index 01a60f2417..138ca27a49 100644 --- a/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.md -2026-08-27-process-table-snapshots.md: 2f031cc2952ffe1c007e04acf4dabbeac0630630 -2026-08-27-process-table-snapshots.zh.md: fbad15c306d250aeb64247a301ed20777f39c4a3 +2026-08-27-process-table-snapshots.md: 27c364607ca1e03a926c309f26007477a8785636 +2026-08-27-process-table-snapshots.zh.md: 9c5fe65bb83a0d04fe5639b3ffefcf377c3588ef diff --git a/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.md b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.md index 2f031cc295..27c364607c 100644 --- a/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.md +++ b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.md @@ -30,19 +30,23 @@ Teardown has the same structure. `signalProcess` fences each signal against PID Each caller captures one snapshot and answers every question of a single pass from it. `LocalTerminalHandle.descendants()` takes a snapshot, reads the tree and session from it, and filters survivors through the same `alive`, so a readiness poll costs one table read regardless of descendant count. `waitForMembers` captures a fresh snapshot per polling iteration, because its whole purpose is observing change. -`signalProcess(identity, signal, observed)` takes the caller's observation rather than reading the table itself. The PID-reuse fence stays, and `signalMembers` now captures once for a whole signalling round instead of once per member. Passing the observation explicitly is what keeps Linux teardown from regressing: `alive` there is answered from a `/proc` walk the snapshot already paid for, not from a fresh walk per member. +Signalling does not share that observation. `ProcessInspector.isAlive(identity)` answers current state from the narrowest per-identity source a platform offers — one `/proc//stat` read on Linux, one `ps` table on macOS, one process-handle check on Windows — and `signalProcess` takes that fence immediately before delivering the signal. An observation cannot stand in for it: the observation preserves the original PID-to-start-time pairing, so a recycled PID would still match it and take a signal meant for the process that exited. Reading the fence per target also keeps a failed read costing one target instead of the rest of a teardown round, which is what the [synchronous exit-cleanup contract](../bug-fix/2026-08-11-synchronous-subprocess-exit-cleanup.md) requires. + +`signalMembers` and `waitForMembers` return before capturing anything when a round has no members, so a command that spawned no descendants pays no table read for its teardown sweeps. Platform differences live in how a snapshot is built, not in what it promises: - **macOS** builds it from one `ps` table. That table exposes neither a session id nor a state column, so `session` is empty and `alive` reports presence with a matching start identity. - **Linux** walks `/proc` once, carrying each entry's parent, start identity, session, and state. `alive` treats the `Z`, `X`, and `x` states as quiescent, as a per-pid `stat` read did. -- **Windows** captures the Toolhelp32 enumeration for `tree`, has no POSIX sessions, and answers `alive` from the live process handle, because wait state is not a table column there. +- **Windows** enumerates Toolhelp32 lazily, on the first `tree` question. It has no POSIX sessions, and answers `alive` from the live process handle, because wait state is not a table column there — so a snapshot asked only for liveness never enumerates. The terminal's Windows teardown polls liveness every 25 ms and would otherwise walk and discard the whole table each time. `PosixProcessSnapshot` holds both POSIX shapes: a row's `session` and `state` are `undefined` where the platform's table omits them, which is what makes the macOS answers fall out of the shared implementation instead of a second class. ## Testing -`packages/subprocess/subprocess-local/tests/terminal.spec.ts` drives a real `MacProcessInspector` over an injected `exec` and asserts one foreground inspection performs exactly one `-axo` table read at 0, 2, and 10 descendants. That count, not wall time, is the durable invariant: it holds on any host and fails the moment a caller re-reads the table per member. +`packages/subprocess/subprocess-local/tests/terminal.spec.ts` drives a real `MacProcessInspector` over an injected `exec` and asserts one foreground inspection performs exactly one `-axo` table read at 0, 2, and 10 descendants. That count, not wall time, is the durable invariant: it holds on any host and fails the moment a caller re-reads the table per member. The same file pins that a signalling round with no members captures nothing, and that a capture failure during synchronous host exit still lets the PTY root be killed. + +`process-inspector.spec.ts` pins the fence directly: an identity observed alive and then absent from the table takes no signal. `windows-inspector.spec.ts` pins that a snapshot answering only liveness performs no Toolhelp32 enumeration. ## Alternatives considered @@ -50,7 +54,7 @@ Platform differences live in how a snapshot is built, not in what it promises: **Caching the macOS table inside `MacProcessInspector` behind a short TTL.** This needs no interface change, but it makes staleness invisible: a caller cannot tell whether a liveness answer came from this instant or from the end of the previous poll, and a signal decided on a stale row is exactly what the PID-reuse fence exists to prevent. Hidden caching also conflicts with the repository's preference for explicit defaulting and explicit boundaries. -**Keeping `isAlive` on the inspector next to `snapshot()`.** This avoids touching the signalling call sites, at the cost of two ways to ask one question, where only one of them is cheap in a loop. The asymmetry would have to be re-explained at every call site. +**Fencing signals with the round's shared observation.** This removes the last per-member read and was the shape first implemented here. Review rejected it: the fence exists to defeat PID reuse, and an observation defeats the fence instead, because it carries the original PID-to-start-time pairing forward. The window is narrow — the kills in one round are microseconds apart, against a PID space of 99999 on macOS and 4194304 on Linux — but the `README` states the guarantee without qualification, and buying microseconds of teardown time by weakening it is the wrong trade. Keeping both `snapshot().alive` and `isAlive` is therefore not two ways to ask one question: one asks what the table showed, the other asks what is true now, and only the second may decide a signal. **Making `exec` asynchronous instead of reducing the read count.** An async `execFile` stops the poll from blocking the loop but still forks N+1 processes per poll; on a busy machine that trades a stall for sustained fork pressure. It remains a worthwhile follow-up on top of the reduced count, not a substitute for it. @@ -58,9 +62,9 @@ Platform differences live in how a snapshot is built, not in what it promises: A readiness poll's process-table cost is now constant in descendant count. On macOS one poll performs one full table read plus the small `tpgid` read, which is the 0-descendant cost in the table above for every descendant count. -Liveness for a single identity on Linux costs a full `/proc` walk rather than one `stat` read. Every caller that asks about several identities amortizes that walk across them, which is why `signalProcess` takes an observation rather than capturing its own; a future caller that genuinely needs one isolated liveness answer pays more than it did. +Teardown keeps its previous per-signal cost: one narrow liveness read per target, which on macOS is one `ps` fork per member. That cost was never the measured problem — a terminal tears down once, while its readiness path polls up to 600 times — so the fix deliberately spends it to keep the fence reading current state. -A snapshot is a point-in-time view, and the type's documentation says so. Holding one across an `await` and then signalling from it would widen the PID-reuse window that the fence narrows; `waitForMembers` re-captures per iteration for exactly this reason. +A snapshot is a point-in-time view, and the type's documentation says so. `waitForMembers` re-captures per iteration because observing change is its purpose, and no signal is ever decided from a captured view. Every `ProcessInspector` implementation and test fake carries the new shape, including the Windows inspector and the `dsh-terminal-bash` session fake. Test fakes that previously replaced `processTree`, `processSession`, or `isAlive` to stage a scan now replace the corresponding per-question read hook, which keeps their staging behavior and call-counting identical. diff --git a/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.zh.md b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.zh.md index fbad15c306..9c5fe65bb8 100644 --- a/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.zh.md +++ b/.agents/notes/implemented/architecture/2026-08-27-process-table-snapshots.zh.md @@ -30,19 +30,23 @@ Status: implemented 每个调用方捕获一次快照，并从中回答本次流程的全部问题。`LocalTerminalHandle.descendants()` 取一次快照，从中读取树与会话，并用同一个 `alive` 过滤幸存者，因此一次就绪轮询无论有多少子进程都只读一次表。`waitForMembers` 每一轮轮询各捕获一次新快照，因为它的用途正是观察变化。 -`signalProcess(identity, signal, observed)` 接收调用方的观察，而不是自己去读表。PID 复用围栏保留，而 `signalMembers` 现在为整轮信号只捕获一次，而不是每个成员各一次。把观察显式传入正是 Linux 拆卸不退化的原因：那里的 `alive` 由快照已经付过代价的一次 `/proc` 遍历回答，而不是每个成员各遍历一次。 +发信号不共用这份观察。`ProcessInspector.isAlive(identity)` 用各平台最窄的按标识来源回答当前状态——Linux 读一个 `/proc//stat`、macOS 读一次 `ps` 表、Windows 查一次进程句柄——`signalProcess` 在投递信号前就地取这道围栏。观察无法代替它：观察把原始的「PID 与起始时间」配对保留了下来，因此被复用的 PID 仍会与之匹配，并领走本该发给已退出进程的信号。逐目标读取围栏还让一次失败的读取只损失一个目标，而不是整轮拆卸的其余部分，这正是[同步退出清理约定](../bug-fix/2026-08-11-synchronous-subprocess-exit-cleanup.zh.md)的要求。 + +`signalMembers` 与 `waitForMembers` 在一轮没有成员时直接返回、不做任何捕获，因此没有派生子进程的命令，其拆卸扫描不付表读取代价。 平台差异体现在快照如何构建，而不在它承诺什么： - **macOS** 由一张 `ps` 表构建。该表既不暴露会话 id 也不暴露状态列，所以 `session` 为空，`alive` 报告的是「存在且起始标识匹配」。 - **Linux** 遍历一次 `/proc`，携带每个条目的父进程、起始标识、会话与状态。`alive` 把 `Z`、`X`、`x` 状态视为静止，与按 pid 读 `stat` 的判定一致。 -- **Windows** 捕获 Toolhelp32 枚举供 `tree` 使用，没有 POSIX 会话，并且从活的进程句柄回答 `alive`，因为等待状态在那里不是表的一列。 +- **Windows** 把 Toolhelp32 枚举惰性化到第一次 `tree` 提问。它没有 POSIX 会话，并且从活的进程句柄回答 `alive`，因为等待状态在那里不是表的一列——所以只问存活的快照永不枚举。终端在 Windows 上的拆卸每 25 ms 轮询一次存活，否则每次都会遍历整张表再丢弃。 `PosixProcessSnapshot` 同时承载两种 POSIX 形态：当平台的表省略某字段时，该行的 `session` 与 `state` 为 `undefined`，这使得 macOS 的答案从共享实现中自然得出，而不必新增一个类。 ## Testing -`packages/subprocess/subprocess-local/tests/terminal.spec.ts` 通过注入的 `exec` 驱动真实的 `MacProcessInspector`，断言一次前台检查在 0、2、10 个子进程下都恰好执行一次 `-axo` 表读取。这个次数——而非墙钟时间——才是持久不变量：它在任何主机上都成立，并且在任何调用方按成员重复读表的那一刻失败。 +`packages/subprocess/subprocess-local/tests/terminal.spec.ts` 通过注入的 `exec` 驱动真实的 `MacProcessInspector`，断言一次前台检查在 0、2、10 个子进程下都恰好执行一次 `-axo` 表读取。这个次数——而非墙钟时间——才是持久不变量：它在任何主机上都成立，并且在任何调用方按成员重复读表的那一刻失败。同一文件还钉住：没有成员的一轮信号不做任何捕获；同步主机退出期间捕获失败时，PTY root 仍会被杀掉。 + +`process-inspector.spec.ts` 直接钉住围栏：一个先被观察为存活、随后从表中消失的标识不会收到信号。`windows-inspector.spec.ts` 钉住只回答存活的快照不执行 Toolhelp32 枚举。 ## Alternatives considered @@ -50,7 +54,7 @@ Status: implemented **在 `MacProcessInspector` 内部用短 TTL 缓存 macOS 的表。** 这不需要改接口，但它让陈旧性不可见：调用方无法分辨一个存活答案来自此刻还是来自上一次轮询结束时，而基于陈旧行发出的信号正是 PID 复用围栏要防止的事情。隐式缓存也与仓库偏好显式默认与显式边界的立场冲突。 -**在 `snapshot()` 旁保留 `isAlive`。** 这样不必改动发信号的调用点，代价是同一个问题有两种问法，而其中只有一种在循环里是廉价的。这种不对称将不得不在每个调用点重新解释一遍。 +**用整轮共享的观察来给信号加围栏。** 这能去掉最后一处按成员的读取，也是本次最初实现的形态。评审否决了它：围栏的存在就是为了击败 PID 复用，而观察反过来击败了围栏——因为它把原始的「PID 与起始时间」配对一路带了下来。窗口确实很窄（一轮里各次 kill 相隔微秒级，而 macOS 的 PID 空间是 99999、Linux 是 4194304），但 `README` 是无条件地声明这条保证的，用削弱它来换取微秒级的拆卸时间是错误的取舍。因此同时保留 `snapshot().alive` 与 `isAlive` 并不是同一个问题的两种问法：前者问表当时显示了什么，后者问此刻什么为真，而只有后者可以决定一次信号。 **把 `exec` 改成异步，而不是减少读取次数。** 异步的 `execFile` 能让轮询不再阻塞事件循环，但每次轮询仍然 fork N+1 个进程；在繁忙的机器上这是把一次停顿换成了持续的 fork 压力。它在减少读取次数之上仍是值得做的后续项，而不是它的替代。 @@ -58,9 +62,9 @@ Status: implemented 一次就绪轮询的进程表代价现在与子进程数量无关。在 macOS 上，一次轮询执行一次完整表读取加一次小的 `tpgid` 读取，也就是上表中 0 子进程那一行的代价，对任意子进程数量都成立。 -Linux 上查询单个标识的存活，代价从读一个 `stat` 文件变成一次完整的 `/proc` 遍历。每个要查询多个标识的调用方都会把这次遍历摊薄，这正是 `signalProcess` 接收观察而非自行捕获的原因；将来若有调用方确实只需要一次孤立的存活查询，它付出的代价会比过去高。 +拆卸保持原有的按次代价：每个目标一次窄的存活读取，在 macOS 上即每个成员一次 `ps` fork。这项代价从来不是实测到的问题——一个终端只拆卸一次，而它的就绪路径最多轮询 600 次——所以本次修复刻意付出它，以保证围栏读的是当前状态。 -快照是一个时间点视图，该类型的文档也这样声明。跨 `await` 持有一份快照再据此发信号，会扩大围栏本来要收窄的 PID 复用窗口；`waitForMembers` 每轮重新捕获正是为此。 +快照是一个时间点视图，该类型的文档也这样声明。`waitForMembers` 每轮重新捕获是因为观察变化正是它的用途；任何信号都不会从一份已捕获的视图上做决定。 每个 `ProcessInspector` 实现与测试替身都采用新形态，包括 Windows 检查器和 `dsh-terminal-bash` 的会话替身。此前通过替换 `processTree`、`processSession` 或 `isAlive` 来编排扫描的测试替身，现在替换对应的按问题读取钩子，其编排行为与调用计数保持不变。 diff --git a/packages/subprocess/subprocess-local/src/process-inspector.ts b/packages/subprocess/subprocess-local/src/process-inspector.ts index 1c89e8b5f1..08cec3b4ab 100644 --- a/packages/subprocess/subprocess-local/src/process-inspector.ts +++ b/packages/subprocess/subprocess-local/src/process-inspector.ts @@ -20,15 +20,16 @@ interface FileStatus { * One observation of the platform process table, shared by every question a * single readiness poll or teardown pass asks. * - * The table is read once, at capture — a `/bin/ps` fork on macOS, a `/proc` - * walk on Linux, a Toolhelp32 enumeration on Windows. Answering {@link tree}, - * {@link session}, or {@link alive} never re-reads it, which is what keeps a - * poll's cost independent of how many descendants the running command spawned. - * Windows liveness additionally consults the live process handle, because wait - * state is not a table column there. + * The table is read at most once, on the first question that needs it — a + * `/bin/ps` fork on macOS, a `/proc` walk on Linux, a Toolhelp32 enumeration on + * Windows. Later questions never re-read it, which is what keeps a poll's cost + * independent of how many descendants the running command spawned. Windows + * liveness needs no table at all: wait state is a per-handle question there, so + * a snapshot asked only for liveness never enumerates. * - * A snapshot is a point-in-time view. Take a fresh one per poll or teardown - * pass; a stale one must never decide that a process is still worth signalling. + * A snapshot answers what the process table showed, which is what batch + * filtering wants and what signalling must not use: {@link ProcessInspector.isAlive} + * is the fence a signal takes, because it reads current state instead. */ export interface ProcessSnapshot { /** @@ -64,17 +65,34 @@ export interface ProcessInspector { isStdinWaiting(pgid: number, shellPid: number): boolean /** * Read the process table once and answer tree, session, and liveness from it. - * @returns A point-in-time process-table observation. + * @returns A process-table observation whose reads are shared. */ snapshot(): ProcessSnapshot + /** + * Return whether the exact identity is a non-quiescent process right now. + * + * Reads the narrowest per-identity source the platform offers rather than a + * whole table, so a signalling round can re-check every target without + * paying for a scan. Callers filtering many members at once want + * {@link ProcessSnapshot.alive} instead. + * + * @param identity - PID plus start identity to match. + * @returns Whether that exact identity — not merely that PID — is running. + */ + isAlive(identity: ProcessIdentity): boolean signalGroup(pgid: number, signal: SubprocessTerminalSignal): void /** * Signal one exact process identity, fenced against PID reuse. + * + * The fence reads current state immediately before the signal. An observation + * taken earlier in the same round cannot stand in for it: the observation + * preserves the original PID-to-start-time pairing, so a recycled PID would + * still match and take a signal meant for the process that exited. + * * @param identity - PID plus start identity to signal. * @param signal - termination signal to deliver. - * @param observed - observation the identity fence reads; pass one taken for this teardown pass. */ - signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL', observed: ProcessSnapshot): void + signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL'): void } /** Testable boundary around filesystem, process-table, and signal syscalls. */ @@ -333,13 +351,14 @@ abstract class PosixProcessInspector implements ProcessInspector { abstract foregroundPgid(shellPid: number): number | undefined abstract isStdinWaiting(pgid: number, shellPid: number): boolean abstract snapshot(): ProcessSnapshot + abstract isAlive(identity: ProcessIdentity): boolean signalGroup(pgid: number, signal: SubprocessTerminalSignal): void { this.internals.kill(-pgid, signal) } - signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL', observed: ProcessSnapshot): void { - if (observed.alive(identity)) this.internals.kill(identity.pid, signal) + signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL'): void { + if (this.isAlive(identity)) this.internals.kill(identity.pid, signal) } } @@ -438,6 +457,11 @@ class LinuxProcessInspector extends PosixProcessInspector { return false } + isAlive(identity: ProcessIdentity): boolean { + const stat = readLinuxStat(this.internals, identity.pid) + return stat?.started === identity.started && !quiescent(stat.state) + } + snapshot(): ProcessSnapshot { return new PosixProcessSnapshot(numericEntries(this.internals, '/proc').flatMap((pid) => { const stat = readLinuxStat(this.internals, pid) @@ -483,6 +507,11 @@ class MacProcessInspector extends PosixProcessInspector { return false } + isAlive(identity: ProcessIdentity): boolean { + return macProcessTable(this.internals) + .some(entry => entry.pid === identity.pid && entry.started === identity.started) + } + snapshot(): ProcessSnapshot { return new PosixProcessSnapshot(macProcessTable(this.internals)) } diff --git a/packages/subprocess/subprocess-local/src/terminal.ts b/packages/subprocess/subprocess-local/src/terminal.ts index eb6bf618c1..624de54984 100644 --- a/packages/subprocess/subprocess-local/src/terminal.ts +++ b/packages/subprocess/subprocess-local/src/terminal.ts @@ -139,7 +139,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { if (this.exited) return if (this.rootIdentity !== undefined) { try { - this.inspector.signalProcess(this.rootIdentity, 'SIGKILL', this.inspector.snapshot()) + this.inspector.signalProcess(this.rootIdentity, 'SIGKILL') } catch (_rootExitedDuringHostExit) { // Exact identity signalling contains both exit races and PID reuse. } @@ -175,6 +175,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { } private async waitForMembers(members: ProcessIdentity[]): Promise { + if (members.length === 0) return [] const until = Date.now() + this.graceMs let survivors = this.survivors(members, this.inspector.snapshot()) while (survivors.length > 0 && Date.now() < until) { @@ -185,10 +186,11 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { } private signalMembers(members: ProcessIdentity[], signal: 'SIGTERM' | 'SIGKILL'): void { - const observed = this.inspector.snapshot() for (const member of members) { try { - this.inspector.signalProcess(member, signal, observed) + // Each signal reads its own identity fence, inside this try: a failed + // read must cost one target, never the rest of a teardown round. + this.inspector.signalProcess(member, signal) } catch (_alreadyExitedDuringSignal) { // The exact process identity is rechecked; a same-tick exit is success. } @@ -264,9 +266,9 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { // (the same console-list agent), so the tiers verify the shell's absence // through the inspector instead of waiting on `done` alone. const shellGone = (): boolean => - this.exited || (this.rootIdentity !== undefined && !this.inspector.snapshot().alive(this.rootIdentity)) + this.exited || (this.rootIdentity !== undefined && !this.inspector.isAlive(this.rootIdentity)) if (!shellGone() && this.rootIdentity !== undefined) { - this.inspector.signalProcess(this.rootIdentity, 'SIGTERM', this.inspector.snapshot()) + this.inspector.signalProcess(this.rootIdentity, 'SIGTERM') await this.waitForWindowsShellExit() } if (!shellGone() && this.rootIdentity === undefined) { @@ -278,7 +280,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { await Promise.race([this.done.then(() => undefined), delay(this.graceMs)]) } if (!shellGone() && this.rootIdentity !== undefined) { - this.inspector.signalProcess(this.rootIdentity, 'SIGKILL', this.inspector.snapshot()) + this.inspector.signalProcess(this.rootIdentity, 'SIGKILL') await this.waitForWindowsShellExit() } if (!shellGone()) throw new Error(`terminal cleanup failed; surviving pid: ${this.pid}`) @@ -287,7 +289,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { private async waitForWindowsShellExit(): Promise { const until = Date.now() + this.graceMs while (!this.exited && Date.now() < until) { - if (this.rootIdentity !== undefined && !this.inspector.snapshot().alive(this.rootIdentity)) return + if (this.rootIdentity !== undefined && !this.inspector.isAlive(this.rootIdentity)) return await delay(Math.min(25, Math.max(1, until - Date.now()))) } } @@ -317,7 +319,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle { if (this.exited) return /* v8 ignore next -- stopShellWindows() verified the shell is gone or threw; the identity re-check is a defensive fence for a future caller. */ - if (this.rootIdentity !== undefined && this.inspector.snapshot().alive(this.rootIdentity)) return + if (this.rootIdentity !== undefined && this.inspector.isAlive(this.rootIdentity)) return this.exited = true this.output.end() this.outcome.resolve({ exitCode: null, signal: null }) diff --git a/packages/subprocess/subprocess-local/src/windows-inspector.ts b/packages/subprocess/subprocess-local/src/windows-inspector.ts index 7b2ae5a23d..6820505c3d 100644 --- a/packages/subprocess/subprocess-local/src/windows-inspector.ts +++ b/packages/subprocess/subprocess-local/src/windows-inspector.ts @@ -97,18 +97,25 @@ export class WindowsProcessInspector implements ProcessInspector { return false } + isAlive(identity: ProcessIdentity): boolean { + const state = this.internals.processState(identity.pid) + return state?.active === true && state.started === identity.started + } + snapshot(): ProcessSnapshot { - const entries = this.internals.snapshot() + // Enumerated on the first question that reads the table. Liveness never + // does — wait state is a per-handle question here — so the Windows + // teardown poll, which asks only for liveness, pays no Toolhelp32 walk. + let entries: ProcessEntry[] | undefined return { - tree: rootPid => windowsProcessTree(entries, rootPid, pid => this.internals.processState(pid)?.started), + tree: rootPid => windowsProcessTree( + entries ??= this.internals.snapshot(), + rootPid, + pid => this.internals.processState(pid)?.started, + ), // Windows has no POSIX sessions; the shell pid stands in as a pseudo group. session: () => [], - alive: (identity) => { - // Wait state is a per-handle question, not a Toolhelp32 column, so - // liveness reads the live process object rather than `entries`. - const state = this.internals.processState(identity.pid) - return state?.active === true && state.started === identity.started - }, + alive: identity => this.isAlive(identity), } } @@ -116,8 +123,8 @@ export class WindowsProcessInspector implements ProcessInspector { this.internals.taskkill(pgid, signal === 'SIGKILL') } - signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL', observed: ProcessSnapshot): void { - if (observed.alive(identity)) this.internals.taskkill(identity.pid, signal === 'SIGKILL') + signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL'): void { + if (this.isAlive(identity)) this.internals.taskkill(identity.pid, signal === 'SIGKILL') } } /* jscpd:ignore-end */ diff --git a/packages/subprocess/subprocess-local/tests/local.spec.ts b/packages/subprocess/subprocess-local/tests/local.spec.ts index 589102ccf8..9e4925bdb2 100644 --- a/packages/subprocess/subprocess-local/tests/local.spec.ts +++ b/packages/subprocess/subprocess-local/tests/local.spec.ts @@ -303,6 +303,7 @@ describe('LocalSubprocessRuntime', () => { foregroundPgid: () => undefined, isStdinWaiting: () => false, snapshot: () => ({ tree: () => [], session: () => [], alive: () => false }), + isAlive: () => false, signalGroup: () => {}, signalProcess: () => {}, } @@ -372,6 +373,7 @@ describe('LocalSubprocessRuntime', () => { session: () => [], alive: identity => alive.has(identity.pid), }), + isAlive: identity => alive.has(identity.pid), signalGroup: () => {}, signalProcess: () => {}, } diff --git a/packages/subprocess/subprocess-local/tests/process-exit.spec.ts b/packages/subprocess/subprocess-local/tests/process-exit.spec.ts index c00c666e33..1fc952a869 100644 --- a/packages/subprocess/subprocess-local/tests/process-exit.spec.ts +++ b/packages/subprocess/subprocess-local/tests/process-exit.spec.ts @@ -68,10 +68,9 @@ function cleanupTree(state: TreeState | undefined, identities: ProcessIdentity[] return } const inspector = createProcessInspector() - const observed = inspector.snapshot() for (const identity of identities) { try { - inspector.signalProcess(identity, 'SIGKILL', observed) + inspector.signalProcess(identity, 'SIGKILL') } catch (_alreadyGone) { // Exact start identity prevents PID-reuse cleanup from reaching another process. } diff --git a/packages/subprocess/subprocess-local/tests/process-inspector.spec.ts b/packages/subprocess/subprocess-local/tests/process-inspector.spec.ts index ac5c95c13f..797076ebfc 100644 --- a/packages/subprocess/subprocess-local/tests/process-inspector.spec.ts +++ b/packages/subprocess/subprocess-local/tests/process-inspector.spec.ts @@ -138,14 +138,15 @@ describe('Linux process inspector', () => { expect(observed.alive({ pid: 10, started: '500' })).toBe(true) expect(observed.alive({ pid: 10, started: 'old' })).toBe(false) inspector.signalGroup(40, 'SIGINT') - inspector.signalProcess({ pid: 10, started: '500' }, 'SIGTERM', observed) - inspector.signalProcess({ pid: 10, started: 'old' }, 'SIGKILL', observed) + inspector.signalProcess({ pid: 10, started: '500' }, 'SIGTERM') + inspector.signalProcess({ pid: 10, started: 'old' }, 'SIGKILL') expect(fake.kills).toEqual([[-40, 'SIGINT'], [10, 'SIGTERM']]) fake.files.set('/proc/10/stat', stat(10, 20, 30, 40, '500', 1, 'Z')) - // A zombie is present in the table but never signallable; a fresh capture sees the new state. - const afterExit = inspector.snapshot() - expect(afterExit.alive({ pid: 10, started: '500' })).toBe(false) - inspector.signalProcess({ pid: 10, started: '500' }, 'SIGKILL', afterExit) + // A zombie is present in the table but never signallable; both the batch + // view and the signal fence report it quiescent once the state changes. + expect(inspector.snapshot().alive({ pid: 10, started: '500' })).toBe(false) + expect(inspector.isAlive({ pid: 10, started: '500' })).toBe(false) + inspector.signalProcess({ pid: 10, started: '500' }, 'SIGKILL') expect(fake.kills).toEqual([[-40, 'SIGINT'], [10, 'SIGTERM']]) }) @@ -298,8 +299,8 @@ describe('macOS process inspector', () => { expect(observed.session(10)).toEqual([]) expect(observed.alive({ pid: 11, started: 'Mon Jul 21 10:00:01 2026' })).toBe(true) inspector.signalGroup(55, 'SIGTSTP') - inspector.signalProcess({ pid: 11, started: 'Mon Jul 21 10:00:01 2026' }, 'SIGKILL', observed) - inspector.signalProcess({ pid: 12, started: 'missing' }, 'SIGTERM', observed) + inspector.signalProcess({ pid: 11, started: 'Mon Jul 21 10:00:01 2026' }, 'SIGKILL') + inspector.signalProcess({ pid: 12, started: 'missing' }, 'SIGTERM') expect(fake.kills).toEqual([[-55, 'SIGTSTP'], [11, 'SIGKILL']]) fake.setPs(' 10 11 Mon Jul 21 10:00:00 2026\n 11 10 Mon Jul 21 10:00:01 2026\n') @@ -309,6 +310,20 @@ describe('macOS process inspector', () => { ]) }) + it('re-reads the process table before signalling instead of trusting an earlier observation', () => { + const fake = fakeInternals() + fake.setPs(' 11 10 Mon Jul 21 10:00:01 2026\n') + const inspector = createProcessInspector('darwin', 'arm64', fake.internals) + inspector.snapshot() + // The member exits after that observation; a recycled pid would otherwise + // inherit the observed identity and take the signal meant for the original. + fake.setPs('') + + inspector.signalProcess({ pid: 11, started: 'Mon Jul 21 10:00:01 2026' }, 'SIGKILL') + + expect(fake.kills).toEqual([]) + }) + it('returns undefined for missing or invalid foreground groups and dispatches platform inspectors', () => { const fake = fakeInternals() fake.setTpgid('-1') diff --git a/packages/subprocess/subprocess-local/tests/terminal.spec.ts b/packages/subprocess/subprocess-local/tests/terminal.spec.ts index 75beef6a34..b8f23a89c4 100644 --- a/packages/subprocess/subprocess-local/tests/terminal.spec.ts +++ b/packages/subprocess/subprocess-local/tests/terminal.spec.ts @@ -76,23 +76,29 @@ class FakeInspector implements ProcessInspector { readTree: () => ProcessIdentity[] = () => this.root === undefined ? this.members : [this.root, ...this.members] readSession: () => ProcessIdentity[] = () => this.sessionMembers readAlive: (identity: ProcessIdentity) => boolean = identity => this.alive.has(identity.pid) + /** Liveness as of right now; tests diverge it from readAlive to stage an exit between scan and signal. */ + readCurrentAlive: (identity: ProcessIdentity) => boolean = identity => this.readAlive(identity) + /** Counts process-table captures so read-amplification cases can pin them. */ + captures = 0 snapshot(): ProcessSnapshot { + this.captures += 1 return { tree: () => this.readTree(), session: () => this.readSession(), alive: identity => this.readAlive(identity), } } + + isAlive(identity: ProcessIdentity) { return this.readCurrentAlive(identity) } signalGroup(pgid: number, signal: SubprocessTerminalSignal) { if (this.throwGroup) throw new Error('group failed') this.groups.push([pgid, signal]) } - signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL', observed: ProcessSnapshot) { + signalProcess(identity: ProcessIdentity, signal: 'SIGTERM' | 'SIGKILL') { // Mirrors the real inspectors' alive-gated signalling. - if (!this.alive.has(identity.pid)) return if (this.throwProcess) throw new Error('process raced') - if (!observed.alive(identity)) return + if (!this.isAlive(identity)) return this.processes.push([identity.pid, signal]) if (this.removeOnSignal) this.alive.delete(identity.pid) } @@ -116,8 +122,8 @@ describe('LocalTerminalHandle', () => { inspector.alive.add(pty.pid) inspector.alive.add(first.pid) const signalProcess = inspector.signalProcess.bind(inspector) - inspector.signalProcess = (identity, signal, observed) => { - signalProcess(identity, signal, observed) + inspector.signalProcess = (identity, signal) => { + signalProcess(identity, signal) if (identity.pid === pty.pid) { inspector.members = [first, late] inspector.alive.add(late.pid) @@ -527,6 +533,37 @@ describe('LocalTerminalHandle on Windows', () => { }) }) +describe('signalling freshness and containment', () => { + it('keeps synchronous host exit going when the process table cannot be captured', () => { + const pty = new FakePty() + const inspector = new FakeInspector() + inspector.alive.add(pty.pid) + const handle = new LocalTerminalHandle(pty.asPty(), inspector, 10) + inspector.snapshot = () => { throw new Error('process table unavailable') } + + expect(() => { handle.terminateForHostExit() }).not.toThrow() + + // forceStopShell still runs: a failed scan must not cost the PTY root. + expect(inspector.processes).toEqual([[pty.pid, 'SIGKILL']]) + }) + + it('captures no process table for a signalling round with no members', () => { + const pty = new FakePty() + const inspector = new FakeInspector() + inspector.alive.add(pty.pid) + const handle = new LocalTerminalHandle(pty.asPty(), inspector, 10) + // Only the shell exists, so every descendant scan yields an empty round. + inspector.readTree = () => [{ pid: pty.pid, started: 'shell' }] + inspector.captures = 0 + + handle.terminateForHostExit() + + // Two descendant scans and nothing else: no capture for either empty + // signalling round, and none for the identity-fenced shell kill. + expect(inspector.captures).toBe(2) + }) +}) + describe('process-table read amplification', () => { // The macOS inspector answers every question by forking `/bin/ps`, so a // readiness poll that asks per descendant scales its blocking cost with the diff --git a/packages/subprocess/subprocess-local/tests/windows-inspector.spec.ts b/packages/subprocess/subprocess-local/tests/windows-inspector.spec.ts index c2129923b4..de8df8d535 100644 --- a/packages/subprocess/subprocess-local/tests/windows-inspector.spec.ts +++ b/packages/subprocess/subprocess-local/tests/windows-inspector.spec.ts @@ -16,10 +16,12 @@ function fakeInternals() { const entries: ProcessEntry[] = [] const states = new Map() const kills: Array<[number, boolean]> = [] + const counts = { enumerations: 0, stateReads: 0 } return { + counts, internals: { - snapshot: () => [...entries], - processState: pid => states.get(pid), + snapshot: () => { counts.enumerations += 1; return [...entries] }, + processState: (pid) => { counts.stateReads += 1; return states.get(pid) }, taskkill: (pid: number, force: boolean) => { kills.push([pid, force]) }, } satisfies WindowsProcessInspectorInternals, add(entry: ProcessEntry, started?: string, active = true): void { @@ -30,6 +32,29 @@ function fakeInternals() { } } +describe('WindowsProcessInspector table enumeration', () => { + it('enumerates the process table only for questions that need it', () => { + const fake = fakeInternals() + fake.add({ pid: 10, parentPid: 0 }, 't10') + fake.add({ pid: 11, parentPid: 10 }, 't11') + const inspector = new WindowsProcessInspector(fake.internals) + + // Liveness is a per-handle question on Windows, so a snapshot asked only + // for liveness must not pay a Toolhelp32 walk. The terminal's Windows + // teardown polls exactly this way, every 25 ms. + const observed = inspector.snapshot() + expect(observed.alive({ pid: 11, started: 't11' })).toBe(true) + expect(fake.counts.enumerations).toBe(0) + + expect(observed.tree(10)).toHaveLength(2) + expect(fake.counts.enumerations).toBe(1) + + // A second tree question reuses the same observation. + observed.tree(10) + expect(fake.counts.enumerations).toBe(1) + }) +}) + describe('windowsProcessTree', () => { it('walks a table children-first with readable identities only', () => { const started = (pid: number): string | undefined => pid === 12 ? undefined : `t${pid}` @@ -78,12 +103,12 @@ describe('WindowsProcessInspector (injected internals)', () => { { pid: 11, started: 't11' }, { pid: 10, started: 't10' }, ]) - expect(inspector.snapshot().alive({ pid: 11, started: 't11' })).toBe(true) - expect(inspector.snapshot().alive({ pid: 11, started: 'stale' })).toBe(false) - expect(inspector.snapshot().alive({ pid: 99, started: 't99' })).toBe(false) + expect(inspector.isAlive({ pid: 11, started: 't11' })).toBe(true) + expect(inspector.isAlive({ pid: 11, started: 'stale' })).toBe(false) + expect(inspector.isAlive({ pid: 99, started: 't99' })).toBe(false) fake.add({ pid: 12, parentPid: 10 }, 't12', false) - expect(inspector.snapshot().alive({ pid: 12, started: 't12' })).toBe(false) + expect(inspector.isAlive({ pid: 12, started: 't12' })).toBe(false) }) it('maps SIGKILL to a forced taskkill and other signals to the grace form', () => { @@ -100,9 +125,9 @@ describe('WindowsProcessInspector (injected internals)', () => { fake.add({ pid: 10, parentPid: 0 }, 't10') fake.add({ pid: 11, parentPid: 10 }, 't11', false) const inspector = new WindowsProcessInspector(fake.internals) - inspector.signalProcess({ pid: 10, started: 't10' }, 'SIGKILL', inspector.snapshot()) - inspector.signalProcess({ pid: 11, started: 't11' }, 'SIGKILL', inspector.snapshot()) - inspector.signalProcess({ pid: 10, started: 'stale' }, 'SIGTERM', inspector.snapshot()) + inspector.signalProcess({ pid: 10, started: 't10' }, 'SIGKILL') + inspector.signalProcess({ pid: 11, started: 't11' }, 'SIGKILL') + inspector.signalProcess({ pid: 10, started: 'stale' }, 'SIGTERM') expect(fake.kills).toEqual([[10, true]]) }) @@ -139,12 +164,10 @@ win32('WindowsProcessInspector over the real koffi bindings', () => { it('reports unreadable identities for absent processes and no-ops tree signalling', () => { const inspector = createWindowsProcessInspector() - expect(inspector.snapshot().alive({ pid: 0x7FFFFFFF, started: 'absent' })).toBe(false) + expect(inspector.isAlive({ pid: 0x7FFFFFFF, started: 'absent' })).toBe(false) expect(() => { inspector.signalGroup(0x7FFFFFFF, 'SIGKILL') }).not.toThrow() expect(() => { inspector.signalGroup(0x7FFFFFFF, 'SIGTERM') }).not.toThrow() expect(() => { inspector.signalGroup(0, 'SIGKILL') }).not.toThrow() - expect(() => { - inspector.signalProcess({ pid: 0x7FFFFFFF, started: 'absent' }, 'SIGKILL', inspector.snapshot()) - }).not.toThrow() + expect(() => { inspector.signalProcess({ pid: 0x7FFFFFFF, started: 'absent' }, 'SIGKILL') }).not.toThrow() }) }) diff --git a/packages/terminal/terminal-bash/tests/session.spec.ts b/packages/terminal/terminal-bash/tests/session.spec.ts index 02ce95db02..b28c579375 100644 --- a/packages/terminal/terminal-bash/tests/session.spec.ts +++ b/packages/terminal/terminal-bash/tests/session.spec.ts @@ -34,6 +34,7 @@ class FakeInspector implements ProcessInspector { alive: (identity: ProcessIdentity) => this.alive.has(identity.pid), } } + isAlive(identity: ProcessIdentity) { return this.alive.has(identity.pid) } signalGroup(pgid: number, signal: TerminalSignal) { if (this.throwGroup) throw new Error('group failed') this.groups.push([pgid, signal])