diff --git a/packages/subagent/subagent-acp/src/run.ts b/packages/subagent/subagent-acp/src/run.ts index fba0403739..f264261b0e 100644 --- a/packages/subagent/subagent-acp/src/run.ts +++ b/packages/subagent/subagent-acp/src/run.ts @@ -90,15 +90,41 @@ export const DEFAULT_DISPOSE_EOF_GRACE_MS = 6_000 /** Default POSIX grace between SIGTERM and SIGKILL on dispose (the `disposeGraceMs` config). */ export const DEFAULT_DISPOSE_GRACE_MS = 3_000 -/** Bounded whole-tree exit wait: polls the handle's tree liveness until it exits or `ms` elapses. */ -async function treeExitsWithin(child: SubprocessHandle, ms: number): Promise { - const controller = new AbortController() - const timer = setTimeout(() => { controller.abort() }, ms) - try { - return await child.waitForExit(controller.signal) - } finally { - clearTimeout(timer) +/** Largest delay Node schedules without collapsing it to one millisecond. */ +const MAX_TIMER_DELAY_MS = 2_147_483_647n + +function scaledFiniteMilliseconds(ms: number, scale: number): bigint { + const whole = Math.floor(ms) + return BigInt(whole) * BigInt(scale) + + BigInt(Math.ceil((ms - whole) * scale)) +} + +/** + * Bounded whole-tree exit wait across Node-safe timer segments. + * @param child - process tree whose liveness is authoritative. + * @param ms - positive finite base window in milliseconds. + * @param scale - integer multiplier applied without Number overflow. + */ +async function treeExitsWithin( + child: SubprocessHandle, + ms: number, + scale = 1, +): Promise { + let remaining = scaledFiniteMilliseconds(ms, scale) + while (remaining > 0n) { + const chunk = remaining > MAX_TIMER_DELAY_MS + ? MAX_TIMER_DELAY_MS + : remaining + remaining -= chunk + const controller = new AbortController() + const timer = setTimeout(() => { controller.abort() }, Number(chunk)) + try { + if (await child.waitForExit(controller.signal)) return true + } finally { + clearTimeout(timer) + } } + return false } /** @@ -125,7 +151,7 @@ export async function disposeAcpChild(child: SubprocessHandle, eofGraceMs: numbe // (this plugin passes disposeGraceMs there), so the bound covers both the // escalation window and an equal confirmation window after the SIGKILL. child.terminate() - if (!(await treeExitsWithin(child, graceMs * 2))) { + if (!(await treeExitsWithin(child, graceMs, 2))) { throw new Error('ACP child process tree did not exit within its dispose windows') } } diff --git a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts index f2cbeda27b..1a8bc577c1 100644 --- a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts +++ b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { Context } from 'cordis' import Loader from '@cordisjs/plugin-loader' import { chmodSync, existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs' @@ -190,6 +190,72 @@ describe('disposeAcpChild (the backend-owned teardown ladder over seam verbs)', await expect(disposeAcpChild(never, 20, 20)).rejects.toThrow(/did not exit within its dispose windows/) }) + it('keeps an oversized finite escalation window instead of collapsing it to one millisecond', async () => { + vi.useFakeTimers() + try { + let waitCount = 0 + let reportExited!: (exited: boolean) => void + const terminate = vi.fn() + const waitForExit = vi.fn((signal?: AbortSignal) => { + waitCount += 1 + return new Promise((resolve) => { + signal?.addEventListener('abort', () => { resolve(false) }, { once: true }) + if (waitCount === 2) reportExited = resolve + }) + }) + const child: Parameters[0] = { + pid: 1, + stdin: undefined, + stdout: undefined, + stderr: undefined, + collected: {}, + done: new Promise(() => {}), + terminate, + waitForExit, + } + const disposal = disposeAcpChild(child, 0.25, Number.MAX_VALUE) + await vi.advanceTimersByTimeAsync(1) + expect(terminate).toHaveBeenCalledOnce() + expect(waitForExit).toHaveBeenCalledTimes(2) + const escalationSignal = waitForExit.mock.calls[1]?.[0] + await vi.advanceTimersByTimeAsync(1) + expect(escalationSignal?.aborted).toBe(false) + reportExited(true) + await expect(disposal).resolves.toBeUndefined() + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } + }) + + it('chains a doubled grace beyond one Node timer segment', async () => { + vi.useFakeTimers() + try { + const waitForExit = vi.fn((signal?: AbortSignal) => new Promise((resolve) => { + signal?.addEventListener('abort', () => { resolve(false) }, { once: true }) + })) + const child: Parameters[0] = { + pid: 1, + stdin: undefined, + stdout: undefined, + stderr: undefined, + collected: {}, + done: new Promise(() => {}), + terminate: vi.fn(), + waitForExit, + } + const disposal = disposeAcpChild(child, 0.25, 1_073_741_823.75) + const rejected = expect(disposal).rejects.toThrow(/did not exit within its dispose windows/) + await vi.advanceTimersByTimeAsync(1) + await vi.advanceTimersByTimeAsync(2_147_483_647) + expect(waitForExit).toHaveBeenCalledTimes(3) + await vi.advanceTimersByTimeAsync(1) + await rejected + } finally { + vi.useRealTimers() + } + }) + it('observes a spawn-level rejection and returns without a process to reap', async () => { const child = spawnSubprocess({ argv: ['bash', '-c', 'true'], diff --git a/packages/subagent/subagent-codex/src/run.ts b/packages/subagent/subagent-codex/src/run.ts index 21f22d8a1b..811f7c8f98 100644 --- a/packages/subagent/subagent-codex/src/run.ts +++ b/packages/subagent/subagent-codex/src/run.ts @@ -24,6 +24,47 @@ import { CodexAppServerWire } from './wire.ts' /** Default POSIX grace between subprocess termination tiers. */ export const DEFAULT_DISPOSE_GRACE_MS = 3_000 +/** Largest delay Node schedules without collapsing it to one millisecond. */ +const MAX_TIMER_DELAY_MS = 2_147_483_647n + +/** + * Bound final exit observation at twice a positive finite grace without + * narrowing the public config to Node's single-timer integer range. + */ +function doubledGraceWindow(graceMs: number): { + readonly signal: AbortSignal + readonly cancel: () => void +} { + const whole = Math.floor(graceMs) + let remaining = BigInt(whole) * 2n + + BigInt(Math.ceil((graceMs - whole) * 2)) + const controller = new AbortController() + let timer: ReturnType | undefined + const arm = (): void => { + const chunk = remaining > MAX_TIMER_DELAY_MS + ? MAX_TIMER_DELAY_MS + : remaining + remaining -= chunk + timer = setTimeout(() => { + timer = undefined + if (remaining === 0n) { + controller.abort() + } else { + arm() + } + }, Number(chunk)) + } + arm() + return { + signal: controller.signal, + cancel: () => { + if (timer === undefined) return + clearTimeout(timer) + timer = undefined + }, + } +} + /** Fully resolved inputs for one Codex app-server run. */ export interface CodexRunSpec { /** Parent Session workspace, also supplied to `thread/start`. */ @@ -88,8 +129,13 @@ export async function disposeCodexChild( // A concurrently closed stdin does not change tree ownership below. } child.terminate() - if (!(await child.waitForExit(AbortSignal.timeout(graceMs * 2)))) { - throw new Error('subagent-codex: app-server process tree did not exit within its dispose window') + const exitWindow = doubledGraceWindow(graceMs) + try { + if (!(await child.waitForExit(exitWindow.signal))) { + throw new Error('subagent-codex: app-server process tree did not exit within its dispose window') + } + } finally { + exitWindow.cancel() } await child.done } diff --git a/packages/subagent/subagent-codex/src/wire.ts b/packages/subagent/subagent-codex/src/wire.ts index 4e920113e3..f933c1a04b 100644 --- a/packages/subagent/subagent-codex/src/wire.ts +++ b/packages/subagent/subagent-codex/src/wire.ts @@ -17,12 +17,17 @@ type JsonObject = Record interface Deferred { readonly promise: Promise readonly resolve: (value: T) => void + readonly reject: (reason?: unknown) => void } function deferred(): Deferred { let resolve!: (value: T) => void - const promise = new Promise((settle) => { resolve = settle }) - return { promise, resolve } + let reject!: (reason?: unknown) => void + const promise = new Promise((settle, fail) => { + resolve = settle + reject = fail + }) + return { promise, resolve, reject } } function object(value: unknown, label: string): JsonObject { @@ -93,7 +98,7 @@ async function raceAbort(pending: Promise, signal: AbortSignal): Promise() + private readonly fatal = deferred() private threadId: string | undefined private turnId: string | undefined private pendingTurnId: string | undefined @@ -111,6 +116,10 @@ export class CodexAppServerWire { output: Writable, ) { this.transport = new JsonRpcLineTransport(input, output) + // Fatal protocol state can arrive after the current guarded operation has + // already settled. Keep the shared rejection observed without inserting + // another promise-adoption hop into active races. + void this.fatal.promise.catch(() => {}) this.transport.onRequest((method, params) => this.handleServerRequest(method, params)) this.transport.onNotification((method, params) => { try { @@ -157,9 +166,8 @@ export class CodexAppServerWire { * Create the run's private ephemeral thread and retain its identity. * @param cwd - parent Session workspace. * @param signal - unpublished-start cancellation. - * @returns the app-server thread id. */ - async startThread(cwd: string, signal: AbortSignal): Promise { + async startThread(cwd: string, signal: AbortSignal): Promise { const response = object(await this.guarded(this.transport.request('thread/start', { cwd, ephemeral: true, @@ -170,7 +178,6 @@ export class CodexAppServerWire { throw new Error('subagent-codex: app-server did not create an ephemeral thread') } this.threadId = id - return id } /** @@ -249,15 +256,12 @@ export class CodexAppServerWire { } private async guarded(pending: Promise, signal: AbortSignal): Promise { - const withFatal = Promise.race([ - pending, - this.fatal.promise.then((error): Promise => Promise.reject(error)), - ]) + const withFatal = Promise.race([this.fatal.promise, pending]) return raceAbort(withFatal, signal) } private fail(error: Error): void { - this.fatal.resolve(error) + this.fatal.reject(error) } private readonly onInputError = (error: Error): void => { diff --git a/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts b/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts index 6cc4461e06..8e6c7ebd51 100644 --- a/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts +++ b/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts @@ -210,7 +210,7 @@ async function initializeWire(): Promise<{ const starting = wire.startThread(process.cwd(), new AbortController().signal) const threadStart = await child.peer.nextMethod('thread/start') child.peer.respond(threadStart, { thread: { id: 'thread-1', ephemeral: true } }) - await expect(starting).resolves.toBe('thread-1') + await starting return { child, wire } } @@ -369,8 +369,9 @@ describe('CodexAppServerWire', () => { { type: 'text', text: 'second', text_elements: [] }, ], }) + child.peer.respond(turnStart, { turn: { id: 'turn-1' } }) + await nextTask() child.peer.send( - { id: turnStart.id, result: { turn: { id: 'turn-1' } } }, { method: 'turn/started', params: { threadId: 'thread-1', turn: { id: 'turn-1' } }, @@ -516,6 +517,20 @@ describe('CodexAppServerWire', () => { } }) + it('keeps an unsupported request authoritative over an early terminal in the same chunk', async () => { + const { child, wire } = await initializeWire() + const result = wire.runTurn(['task'], new AbortController().signal, () => false) + const turnStart = await child.peer.nextMethod('turn/start') + child.peer.send( + { id: turnStart.id, result: { turn: { id: 'turn-1' } } }, + { id: 'future-request', method: 'future/request', params: {} }, + agentMessage('early answer', 'final_answer'), + turnCompleted('completed'), + ) + await expect(result).rejects.toThrow('unsupported app-server request') + wire.close() + }) + it('gives local cancellation precedence over a remote completed turn', async () => { const { child, wire } = await initializeWire() let cancelled = false @@ -1038,6 +1053,37 @@ describe('disposeCodexChild', () => { expect(child.waitForExit).toHaveBeenCalledTimes(1) }) + it('accepts fractional and larger-than-Node grace windows', async () => { + for (const graceMs of [0.25, Number.MAX_VALUE]) { + const child = fakeChild() + const wire = new CodexAppServerWire(child.handle.stdout!, child.handle.stdin!) + await expect(disposeCodexChild(wire, child.handle, graceMs)) + .resolves.toBeUndefined() + const signal = vi.mocked(child.waitForExit).mock.calls[0]?.[0] + expect(signal?.aborted).toBe(false) + } + }) + + it('chains a doubled grace window beyond one Node timer segment', async () => { + vi.useFakeTimers() + try { + const child = fakeChild({ exitOnTerminate: false }) + const wire = new CodexAppServerWire(child.handle.stdout!, child.handle.stdin!) + const disposal = disposeCodexChild( + wire, + child.handle, + 1_073_741_823.75, + ) + const rejected = expect(disposal) + .rejects.toThrow('did not exit within its dispose window') + await vi.advanceTimersByTimeAsync(2_147_483_647) + await vi.advanceTimersByTimeAsync(1) + await rejected + } finally { + vi.useRealTimers() + } + }) + it('contains a concurrently closed stdin error', async () => { const child = fakeChild() const wire = new CodexAppServerWire(child.handle.stdout!, child.handle.stdin!) diff --git a/packages/subprocess/subprocess-local/src/spawn.ts b/packages/subprocess/subprocess-local/src/spawn.ts index 90d460c2c5..932daa2c59 100644 --- a/packages/subprocess/subprocess-local/src/spawn.ts +++ b/packages/subprocess/subprocess-local/src/spawn.ts @@ -55,6 +55,47 @@ function sleepTick(): Promise { return sleepMs(15) } +/** Largest delay Node schedules without collapsing it to one millisecond. */ +const MAX_TIMER_DELAY_MS = 2_147_483_647n + +/** + * Schedule a positive finite millisecond delay across as many Node-safe timer + * segments as necessary. Fractional milliseconds round up so a grace never + * expires earlier than configured. + * @param delayMs - positive finite delay in milliseconds. + * @param callback - work to run after the complete delay. + * @returns a handle that cancels the active segment and all future segments. + */ +export function scheduleFiniteTimeout( + delayMs: number, + callback: () => void, +): { cancel(): void } { + let remaining = BigInt(Math.ceil(delayMs)) + let timer: ReturnType | undefined + const arm = (): void => { + const chunk = remaining > MAX_TIMER_DELAY_MS + ? MAX_TIMER_DELAY_MS + : remaining + remaining -= chunk + timer = setTimeout(() => { + timer = undefined + if (remaining === 0n) { + callback() + } else { + arm() + } + }, Number(chunk)) + } + arm() + return { + cancel(): void { + if (timer === undefined) return + clearTimeout(timer) + timer = undefined + }, + } +} + let spillCounter = 0 let defaultSpillDir: string | undefined @@ -341,7 +382,9 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter const stdoutCollector = collectStream(outMode, child.stdout, 'stdout') const stderrCollector = collectStream(errMode, child.stderr, 'stderr') - let graceTimer: NodeJS.Timeout | undefined + let graceTimer: ReturnType | undefined + let treeExitObserved = false + let treeExitObservation: Promise | undefined let settled = false // Failed spawns use pid -1 so signalling remains a no-op. @@ -349,6 +392,9 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter /** Whether the detached tree's root (or POSIX group) is still alive. */ const treeAlive = (): boolean => { + /* v8 ignore next -- only a timer callback already queued when the observer settles can enter here; + the guard is the final defense against probing an id after its tree was confirmed absent. */ + if (treeExitObserved) return false if (pid <= 0) return false if (platform === 'win32') { // Windows has no group-liveness probe; the direct child's exit is the @@ -371,26 +417,47 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter } } + /** + * Start or reuse the handle's single whole-tree exit observer. The first + * confirmed absence is a permanent no-more-signals boundary: it cancels a + * pending escalation before this process-group id can be reused. + */ + const observeTreeExit = (): Promise => { + treeExitObservation ??= (async () => { + while (treeAlive()) await sleepTick() + treeExitObserved = true + graceTimer?.cancel() + graceTimer = undefined + })() + return treeExitObservation + } + // The escalation's tier primitive (not on the handle — terminate() is the // only consumer-facing termination verb). Guards on TREE liveness, not // outcome settlement: a TERM-trapping helper can outlive the settled direct // child and must stay signalable, while a fully-dead tree (possible pid // reuse) must not be re-signalled by a later tier. const kill = (sig: NodeJS.Signals): void => { + /* v8 ignore next -- the shared exit observer cancels the ordinary dead-tree timer; + this remains the timer/death race guard and cannot be staged deterministically. */ if (!treeAlive()) return signalTree(platform, pid, sig, child, taskkill) } const terminate = (): void => { - if (graceTimer !== undefined) return // escalation already in flight - if (!treeAlive()) return + if (treeExitObserved || graceTimer !== undefined) return + // Observe from the first termination tier onward, even when inherited + // pipes delay `done` and no consumer has begun its own teardown wait. + void observeTreeExit() + // oxlint-disable-next-line typescript/no-unnecessary-condition -- observer can record absence before its first await. + if (treeExitObserved) return kill('SIGTERM') // The escalation must survive direct-child settlement — the leader dying // does not mean the tree died — so settle does not clear this timer, and // kill() re-probes tree liveness before force-killing. It stays ref'd: // the pending SIGKILL is a commitment, and a parent exiting before it // fires would orphan a trapped survivor. Self-bounds at graceMs. - graceTimer = setTimeout(() => { kill('SIGKILL') }, spec.graceMs) + graceTimer = scheduleFiniteTimeout(spec.graceMs, () => { kill('SIGKILL') }) } // The caller owns timeout classification; this layer only reacts to abort. @@ -405,7 +472,7 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter } const done = new Promise((resolve, reject) => { - let pipeDrainTimer: NodeJS.Timeout | undefined + let pipeDrainTimer: ReturnType | undefined const settle = (exitCode: number | null, signal: NodeJS.Signals | null): void => { if (settled) return settled = true @@ -428,23 +495,37 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter // A surviving descendant that inherited a pipe must not hold the // outcome open indefinitely: after exit, the same bounded grace that // governs kills also bounds the close wait. - pipeDrainTimer = setTimeout(() => { settle(exitCode, signal) }, spec.graceMs) + pipeDrainTimer = scheduleFiniteTimeout(spec.graceMs, () => { + settle(exitCode, signal) + }) }) child.on('close', settle) function cleanup(): void { // graceTimer deliberately NOT cleared: the SIGKILL escalation must be // able to reach tree survivors after the direct child settles. - if (pipeDrainTimer !== undefined) clearTimeout(pipeDrainTimer) + pipeDrainTimer?.cancel() spec.signal?.removeEventListener('abort', onAbort) } }) const waitForExit = async (signal?: AbortSignal): Promise => { - while (treeAlive()) { - if (signal?.aborted) return false - await sleepTick() + const observed = observeTreeExit() + if (treeExitObserved) return true + if (signal?.aborted) return false + if (signal === undefined) { + await observed + return true + } + const aborted = Promise.withResolvers() + const onAbort = (): void => { aborted.resolve(false) } + signal.addEventListener('abort', onAbort, { once: true }) + /* v8 ignore next -- closes the event-loop race between the preceding aborted check and listener registration. */ + if (signal.aborted) onAbort() + try { + return await Promise.race([observed.then(() => true), aborted.promise]) + } finally { + signal.removeEventListener('abort', onAbort) } - return true } return { diff --git a/packages/subprocess/subprocess-local/tests/spawn.spec.ts b/packages/subprocess/subprocess-local/tests/spawn.spec.ts index 491756f01f..ad2fc0f30a 100644 --- a/packages/subprocess/subprocess-local/tests/spawn.spec.ts +++ b/packages/subprocess/subprocess-local/tests/spawn.spec.ts @@ -2,7 +2,13 @@ import { mkdtempSync, readFileSync, statSync, unlinkSync } from 'node:fs' import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' import { describe, expect, it, vi } from 'vitest' -import { killGroup, OutputCollector, spawnSubprocess, taskkillProcessTree } from '../src/spawn.ts' +import { + killGroup, + OutputCollector, + scheduleFiniteTimeout, + spawnSubprocess, + taskkillProcessTree, +} from '../src/spawn.ts' import type { SubprocessHandle, SubprocessOutputReader } from '@deepseek-ai/dsh-subprocess' const { failNextClose, failNextUnlink } = vi.hoisted(() => ({ @@ -101,6 +107,30 @@ async function waitForPidFile(path: string, timeoutMs = 5_000): Promise throw new Error(`pid file ${path} was not written after ${timeoutMs}ms`) } +describe('scheduleFiniteTimeout', () => { + it('rounds fractions up, chains Node-safe segments, and cancels idempotently', async () => { + vi.useFakeTimers() + try { + const fired = vi.fn() + const chained = scheduleFiniteTimeout(2_147_483_647.25, fired) + await vi.advanceTimersByTimeAsync(2_147_483_647) + expect(fired).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(1) + expect(fired).toHaveBeenCalledOnce() + chained.cancel() + + const cancelled = vi.fn() + const timer = scheduleFiniteTimeout(0.25, cancelled) + timer.cancel() + timer.cancel() + await vi.advanceTimersByTimeAsync(1) + expect(cancelled).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) +}) + describe('spawnSubprocess', () => { it('captures stdout on success', async () => { const result = await finish(spawnSubprocess(spec('echo hello'))) @@ -164,6 +194,65 @@ describe('spawnSubprocess', () => { expect(result.signal).toBe('SIGKILL') }) + it('cancels a larger-than-Node escalation timer once SIGTERM removes the tree', async () => { + const running = spawnSubprocess(spec('echo ready; sleep 60', { + graceMs: Number.MAX_VALUE, + })) + await waitForStdout(running, 'ready\n') + running.terminate() + const result = await running.done + expect(result.signal).toBe('SIGTERM') + await expect(running.waitForExit()).resolves.toBe(true) + }) + + it('cancels escalation when the terminated group vanishes before collected pipes drain', async () => { + const pidFile = join(spillDir, `escaped-pipe-holder-${Date.now()}.pid`) + const graceMs = 160 + const childScript = ` + const { spawn } = require('node:child_process') + const { writeFileSync } = require('node:fs') + const helper = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + detached: true, + stdio: ['ignore', 1, 2], + }) + writeFileSync(${JSON.stringify(pidFile)}, String(helper.pid)) + helper.unref() + setInterval(() => {}, 1000) + ` + const running = spawnSubprocess({ + ...spec('unused', { graceMs }), + argv: [process.execPath, '-e', childScript], + }) + const helper = await waitForPidFile(pidFile) + const realKill: typeof process.kill = process.kill.bind(process) + let termAt = 0 + let forceSignals = 0 + const killSpy = vi.spyOn(process, 'kill').mockImplementation((target, signal) => { + if (target !== -running.pid) return realKill(target, signal) + if (signal === 'SIGTERM') { + termAt = Date.now() + return realKill(target, signal) + } + if (signal === 'SIGKILL') { + forceSignals += 1 + return true + } + if (signal === 0 && termAt !== 0 && Date.now() - termAt < graceMs / 2) { + throw Object.assign(new Error('simulated vanished process group'), { code: 'ESRCH' }) + } + return true // Before TERM the original group is live; later its pgid is reused. + }) + try { + running.terminate() + await running.done + expect(forceSignals).toBe(0) + } finally { + killSpy.mockRestore() + process.kill(helper, 'SIGKILL') + await waitGone(helper) + } + }) + it('terminates the whole process group (grandchildren die too)', async () => { // The subshell writes the sleep's pid then waits on it; terminating the // group must take the sleep down with bash. @@ -627,7 +716,6 @@ describe('coverage seams', () => { it('terminate() after the tree died delivers no termination signal', async () => { const running = spawnSubprocess(spec('true')) await running.done - await running.waitForExit() const spy = vi.spyOn(process, 'kill') try { running.terminate() @@ -636,6 +724,21 @@ describe('coverage seams', () => { } finally { spy.mockRestore() } + await running.waitForExit() + }) + + it('repeated terminate after exit never probes or signals a reused process group', async () => { + const running = spawnSubprocess(spec('sleep 60')) + running.terminate() + await running.done + await running.waitForExit() + const spy = vi.spyOn(process, 'kill').mockImplementation(() => true) + try { + running.terminate() + expect(spy).not.toHaveBeenCalled() + } finally { + spy.mockRestore() + } }) it('waitForExit on a failed spawn reports exited immediately', async () => {