mirror of
https://github.com/deepseek-ai/deepseek-harness.git
synced 2026-08-29 04:26:38 +00:00
fix(subprocess): fence each signal against current process state
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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/<pid>/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.
|
||||
|
||||
|
||||
@@ -30,19 +30,23 @@ Status: implemented
|
||||
|
||||
每个调用方捕获一次快照,并从中回答本次流程的全部问题。`LocalTerminalHandle.descendants()` 取一次快照,从中读取树与会话,并用同一个 `alive` 过滤幸存者,因此一次就绪轮询无论有多少子进程都只读一次表。`waitForMembers` 每一轮轮询各捕获一次新快照,因为它的用途正是观察变化。
|
||||
|
||||
`signalProcess(identity, signal, observed)` 接收调用方的观察,而不是自己去读表。PID 复用围栏保留,而 `signalMembers` 现在为整轮信号只捕获一次,而不是每个成员各一次。把观察显式传入正是 Linux 拆卸不退化的原因:那里的 `alive` 由快照已经付过代价的一次 `/proc` 遍历回答,而不是每个成员各遍历一次。
|
||||
发信号不共用这份观察。`ProcessInspector.isAlive(identity)` 用各平台最窄的按标识来源回答当前状态——Linux 读一个 `/proc/<pid>/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` 来编排扫描的测试替身,现在替换对应的按问题读取钩子,其编排行为与调用计数保持不变。
|
||||
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
|
||||
@@ -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<ProcessIdentity[]> {
|
||||
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<void> {
|
||||
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 })
|
||||
|
||||
@@ -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 */
|
||||
|
||||
@@ -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: () => {},
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
}
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -16,10 +16,12 @@ function fakeInternals() {
|
||||
const entries: ProcessEntry[] = []
|
||||
const states = new Map<number, WindowsProcessState>()
|
||||
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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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])
|
||||
|
||||
Reference in New Issue
Block a user