mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 16:03:17 +00:00
fix(terminal): require fresh process snapshots
This commit is contained in:
@@ -711,13 +711,13 @@
|
||||
"ssh",
|
||||
"runtime"
|
||||
],
|
||||
"coverageNotes": "Live macOS Electron tests prove exact local PTY disappearance after parked-tab close and detached-pgid descendant death after agent close. Deterministic tests cover daemon and SSH routing, local/daemon pending-snapshot ownership, duplicate-kill coalescing, bounded/coalesced process-table reads, same-second PID ambiguity, ordinary runtime close ownership, unified-only hydration, split ownership, pane detach transfer, restart alias hydration, and late-hook suppression; live Linux, Windows, WSL, SSH, and remote-runtime process evidence remains pending.",
|
||||
"coverageNotes": "Live macOS Electron tests prove exact local PTY disappearance after parked-tab close and detached-pgid descendant death after agent close. Deterministic tests cover daemon and SSH routing, local/daemon pending-snapshot ownership, duplicate-kill completion sharing, bounded/fresh/coalesced process-table reads, source-scan timestamp identity, same-second PID ambiguity, ordinary runtime close ownership, unified-only hydration, split ownership, pane detach transfer, restart alias hydration, and late-hook suppression; live Linux, Windows, WSL, SSH, and remote-runtime process evidence remains pending.",
|
||||
"motivatingLinks": [
|
||||
"https://github.com/stablyai/orca/pull/8628",
|
||||
"https://github.com/stablyai/orca/pull/8706"
|
||||
],
|
||||
"invariant": "Close permanently removes the owned provider session, agent descendants, and resume authority even when no TerminalPane is mounted; a terminating id cannot reattach or start duplicate cleanup; process-table work is bounded and coalesced; detach and park preserve ownership; aliases prevent a detached agent's immutable physical pane key from being retired with its former tab.",
|
||||
"oracle": "Capture the exact PTY before parking, prove it remains listed while the view is absent, close through the product state boundary, and poll the provider inventory until that exact ID disappears; an agent-marked PTY's detached-pgid child is alive before close and absent afterward; unit tests reject pending-capture reattach, coalesce duplicate teardown and 20 process-table requests, bound both read phases, avoid ambiguous SIGKILL, and assert canonical owner dedupe, exact pane tombstones, chained detach transfer, and restart alias restoration.",
|
||||
"invariant": "Close permanently removes the owned provider session, agent descendants, and resume authority even when no TerminalPane is mounted; a terminating id cannot reattach or start duplicate cleanup, and duplicate callers await the same completion; process-table work is bounded, fresh for each post-start request, and same-turn coalesced; detach and park preserve ownership; aliases prevent a detached agent's immutable physical pane key from being retired with its former tab.",
|
||||
"oracle": "Capture the exact PTY before parking, prove it remains listed while the view is absent, close through the product state boundary, and poll the provider inventory until that exact ID disappears; an agent-marked PTY's detached-pgid child is alive before close and absent afterward; unit tests reject pending-capture reattach, share duplicate teardown completion, coalesce 20 same-turn process-table requests, force a successor scan for later arrivals, retain the source scan's timestamp, bound both read phases, avoid ambiguous SIGKILL, and assert canonical owner dedupe, exact pane tombstones, chained detach transfer, and restart alias restoration.",
|
||||
"commands": [
|
||||
"pnpm dlx node@24 ./node_modules/vitest/vitest.mjs run --config config/vitest.config.ts src/main/agent-hooks/server-pane-authority.test.ts src/main/ipc/agent-hooks.test.ts src/main/ipc/agent-pane-authority-ownership.test.ts src/main/ipc/pty-management.test.ts src/main/persistence.test.ts src/renderer/src/store/slices/agent-pane-authority.test.ts src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts src/renderer/src/store/slices/terminal-tab-retirement.test.ts src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/pty-descendant-termination.test.ts src/main/daemon/session.test.ts src/main/daemon/terminal-host.test.ts src/main/providers/local-pty-provider.test.ts",
|
||||
@@ -766,9 +766,9 @@
|
||||
{
|
||||
"file": "src/main/pty-descendant-termination.test.ts",
|
||||
"assertions": [
|
||||
"20 concurrent and short-sequential process-table requests execute one fresh scan",
|
||||
"20 same-turn process-table requests execute one fresh scan while later arrivals receive a successor scan",
|
||||
"snapshot and escalation readers stop at their deadline",
|
||||
"capture-second process identities are never escalated with SIGKILL"
|
||||
"the source scan timestamp survives request resolution and capture-second identities are never escalated with SIGKILL"
|
||||
]
|
||||
},
|
||||
{
|
||||
@@ -810,7 +810,7 @@
|
||||
},
|
||||
"performanceBudget": {
|
||||
"required": true,
|
||||
"evidence": "The close path is user-triggered and bounded by canonical live-owner indexing. Production ps has a 1s kill timeout; an in-flight plus 50ms completed-snapshot coordinator makes 20 concurrent or short-sequential requests execute one fresh process-table read, and escalation uses the same coordinator and deadline. Kill-all store scale remains covered by terminal-session.kill-all-surface-cleanup."
|
||||
"evidence": "The close path is user-triggered and bounded by canonical live-owner indexing. Production ps has a 1s kill timeout; 20 same-turn requests execute one fresh process-table read, while a request arriving after a scan starts queues one successor rather than consuming stale signal targets. Completed tables are never reused. Escalation uses the same bounded coordinator, and kill-all store scale remains covered by terminal-session.kill-all-surface-cleanup."
|
||||
},
|
||||
"promotionCriteria": [
|
||||
"Accumulate 100 clean runs or 14 days on required CI platforms.",
|
||||
|
||||
@@ -78,10 +78,12 @@ could invalidate intentional long-lived `worktree-sleep` checkpoints.
|
||||
terminal's process group (nohup-style survivors remain user intent there).
|
||||
For **agent sessions**, close/kill additionally terminates the snapshotted
|
||||
descendant tree — including detached-pgid children the PTY's SIGHUP cannot
|
||||
reach — via `pty-descendant-termination.ts` (bounded/coalesced snapshot,
|
||||
SIGTERM, grace window, then identity-safe SIGKILL). A session is marked as
|
||||
terminating before capture so reattach cannot race teardown. Windows and
|
||||
SSH-hosted PTYs keep the previous foreground-tree contract for now.
|
||||
reach — via `pty-descendant-termination.ts` (bounded fresh snapshot with
|
||||
same-turn coalescing, SIGTERM, grace window, then identity-safe SIGKILL).
|
||||
Completed process tables are never reused as signal targets, and identity
|
||||
checks use the source scan's timestamp. A session is marked as terminating
|
||||
before capture so reattach cannot race teardown. Windows and SSH-hosted PTYs
|
||||
keep the previous foreground-tree contract for now.
|
||||
- Changing agent-provider resume commands or permission flags.
|
||||
- Making a UI close wait for a remote process to exit before the tab disappears.
|
||||
- Replacing worktree sleep with tab close. Sleep remains resumable by design.
|
||||
|
||||
@@ -49,6 +49,9 @@ function createMockSubprocess(): SubprocessHandle & {
|
||||
|
||||
type DaemonServerPrivate = {
|
||||
server: Server | null
|
||||
host: {
|
||||
kill: (sessionId: string, opts?: { immediate?: boolean }) => void | Promise<void>
|
||||
}
|
||||
clients: Map<
|
||||
string,
|
||||
{
|
||||
@@ -289,6 +292,36 @@ describe('DaemonServer', () => {
|
||||
expect(result).toBeDefined()
|
||||
})
|
||||
|
||||
it('does not acknowledge kill until asynchronous teardown completes', async () => {
|
||||
await startServer()
|
||||
const daemon = server as unknown as DaemonServerPrivate
|
||||
let finishKill!: () => void
|
||||
const teardown = new Promise<void>((resolve) => {
|
||||
finishKill = resolve
|
||||
})
|
||||
const kill = vi.spyOn(daemon.host, 'kill').mockReturnValue(teardown)
|
||||
|
||||
let acknowledged = false
|
||||
const routed = daemon
|
||||
.routeRequest('client-1', {
|
||||
id: 'kill-1',
|
||||
type: 'kill',
|
||||
payload: { sessionId: 'agent-session', immediate: true }
|
||||
})
|
||||
.then((result) => {
|
||||
acknowledged = true
|
||||
return result
|
||||
})
|
||||
|
||||
await Promise.resolve()
|
||||
expect(kill).toHaveBeenCalledWith('agent-session', { immediate: true })
|
||||
expect(acknowledged).toBe(false)
|
||||
|
||||
finishKill()
|
||||
await expect(routed).resolves.toEqual({})
|
||||
expect(acknowledged).toBe(true)
|
||||
})
|
||||
|
||||
it('handles getCwd', async () => {
|
||||
await startServer()
|
||||
const c = await connectClient()
|
||||
|
||||
@@ -474,7 +474,8 @@ describe('TerminalHost', () => {
|
||||
})
|
||||
|
||||
it('coalesces duplicate immediate kill while descendant capture is pending', async () => {
|
||||
killWithDescendantSweepMock.mockReturnValue(new Promise<void>(() => {}))
|
||||
const sweep = new Promise<void>(() => {})
|
||||
killWithDescendantSweepMock.mockReturnValue(sweep)
|
||||
await host.createOrAttach({
|
||||
sessionId: 'agent-duplicate-kill',
|
||||
cols: 80,
|
||||
@@ -483,10 +484,15 @@ describe('TerminalHost', () => {
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
host.kill('agent-duplicate-kill', { immediate: true })
|
||||
host.kill('agent-duplicate-kill', { immediate: true })
|
||||
const first = host.kill('agent-duplicate-kill', { immediate: true })
|
||||
// The root can exit while the descendant scan is pending. Duplicate RPCs
|
||||
// still own the original completion even after the session was reaped.
|
||||
lastSubprocess._onExitCb?.(0)
|
||||
const second = host.kill('agent-duplicate-kill', { immediate: true })
|
||||
|
||||
expect(killWithDescendantSweepMock).toHaveBeenCalledOnce()
|
||||
expect(first).toBe(sweep)
|
||||
expect(second).toBe(first)
|
||||
expect(lastSubprocess.forceKill).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -6,8 +6,8 @@ import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery
|
||||
import { buildStartupCommandSubmission } from '../../shared/startup-command-submission'
|
||||
import type { SessionInfo, TakePendingOutputResult, TerminalSnapshot } from './types'
|
||||
import { SessionNotFoundError } from './types'
|
||||
import { killWithDescendantSweep } from '../pty-descendant-termination'
|
||||
import type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract'
|
||||
import { TerminalImmediateKillCoordinator } from './terminal-immediate-kill'
|
||||
|
||||
export type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract'
|
||||
|
||||
@@ -42,6 +42,7 @@ export type TerminalHostOptions = {
|
||||
|
||||
export class TerminalHost {
|
||||
private sessions = new Map<string, Session>()
|
||||
private immediateKills = new TerminalImmediateKillCoordinator()
|
||||
private killedTombstones = new Map<string, number>()
|
||||
private spawnSubprocess: TerminalHostOptions['spawnSubprocess']
|
||||
private onFinalCheckpoint: TerminalHostOptions['onFinalCheckpoint']
|
||||
@@ -197,6 +198,10 @@ export class TerminalHost {
|
||||
}
|
||||
|
||||
kill(sessionId: string, opts: { immediate?: boolean } = {}): void | Promise<void> {
|
||||
const pending = opts.immediate ? this.immediateKills.get(sessionId) : undefined
|
||||
if (pending) {
|
||||
return pending
|
||||
}
|
||||
const session = this.getAliveSession(sessionId)
|
||||
this.recordTombstone(sessionId)
|
||||
if (opts.immediate) {
|
||||
@@ -207,16 +212,7 @@ export class TerminalHost {
|
||||
// funnels through Session.handleSubprocessExit -> onExit -> reapSession.
|
||||
this.reapSession(sessionId)
|
||||
}
|
||||
if (!session.launchAgent) {
|
||||
finish()
|
||||
return
|
||||
}
|
||||
if (!session.beginTermination()) {
|
||||
return
|
||||
}
|
||||
// Why: agent tool children live in detached process groups the shell's
|
||||
// death never signals; the bounded snapshot defers teardown at most ~1s.
|
||||
return killWithDescendantSweep(session.pid, finish)
|
||||
return this.immediateKills.kill(sessionId, session, finish)
|
||||
}
|
||||
session.kill()
|
||||
}
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
import { killWithDescendantSweep } from '../pty-descendant-termination'
|
||||
import type { Session } from './session'
|
||||
|
||||
/** Owns asynchronous agent teardown by session id so every duplicate caller
|
||||
* observes the same completion, even if the root exits and is reaped first. */
|
||||
export class TerminalImmediateKillCoordinator {
|
||||
private operations = new Map<string, Promise<void>>()
|
||||
|
||||
get(sessionId: string): Promise<void> | undefined {
|
||||
return this.operations.get(sessionId)
|
||||
}
|
||||
|
||||
kill(sessionId: string, session: Session, finish: () => void): void | Promise<void> {
|
||||
if (!session.launchAgent) {
|
||||
finish()
|
||||
return
|
||||
}
|
||||
if (!session.beginTermination()) {
|
||||
return
|
||||
}
|
||||
|
||||
// Why: agent tool children live in detached process groups the shell's
|
||||
// death never signals; the bounded snapshot defers teardown at most ~1s.
|
||||
const operation = Promise.resolve(killWithDescendantSweep(session.pid, finish))
|
||||
this.operations.set(sessionId, operation)
|
||||
const clearOperation = (): void => {
|
||||
if (this.operations.get(sessionId) === operation) {
|
||||
this.operations.delete(sessionId)
|
||||
}
|
||||
}
|
||||
void operation.then(clearOperation, clearOperation)
|
||||
return operation
|
||||
}
|
||||
}
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
killWithDescendantSweep,
|
||||
parseProcessTable,
|
||||
terminateDescendantSnapshot,
|
||||
type ProcessTableCapture,
|
||||
type ProcessTableRow
|
||||
} from './pty-descendant-termination'
|
||||
|
||||
@@ -34,6 +35,18 @@ function row(
|
||||
return { pid, ppid, pgid, startedAt }
|
||||
}
|
||||
|
||||
function tableCapture(rows: ProcessTableRow[], capturedAtMs = CAPTURED_AT_MS): ProcessTableCapture {
|
||||
return { rows, capturedAtMs }
|
||||
}
|
||||
|
||||
function deferred<T>(): { promise: Promise<T>; resolve: (value: T) => void } {
|
||||
let resolve!: (value: T) => void
|
||||
const promise = new Promise<T>((res) => {
|
||||
resolve = res
|
||||
})
|
||||
return { promise, resolve }
|
||||
}
|
||||
|
||||
function snapshot(
|
||||
descendants: ProcessTableRow[],
|
||||
rootPgid: number | null = 10,
|
||||
@@ -92,11 +105,10 @@ describe('captureDescendantSnapshot', () => {
|
||||
})
|
||||
|
||||
it('resolves the descendant tree on POSIX', async () => {
|
||||
const readTable = vi.fn().mockResolvedValue([row(10, 1, 10), row(20, 10, 20)])
|
||||
const readTable = vi.fn().mockResolvedValue(tableCapture([row(10, 1, 10), row(20, 10, 20)]))
|
||||
const result = await captureDescendantSnapshot(10, {
|
||||
readTable,
|
||||
platform: 'darwin',
|
||||
now: () => CAPTURED_AT_MS
|
||||
platform: 'darwin'
|
||||
})
|
||||
expect(result).toEqual(snapshot([row(20, 10, 20)]))
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
@@ -121,7 +133,7 @@ describe('captureDescendantSnapshot', () => {
|
||||
})
|
||||
|
||||
it('degrades to null when ps hangs past the timeout instead of blocking teardown', async () => {
|
||||
const readTable = vi.fn().mockReturnValue(new Promise<ProcessTableRow[]>(() => {}))
|
||||
const readTable = vi.fn().mockReturnValue(new Promise<ProcessTableCapture>(() => {}))
|
||||
const pending = captureDescendantSnapshot(10, {
|
||||
readTable,
|
||||
platform: 'darwin',
|
||||
@@ -134,8 +146,7 @@ describe('captureDescendantSnapshot', () => {
|
||||
it('gives the production ps subprocess a hard SIGKILL timeout', async () => {
|
||||
const result = await captureDescendantSnapshot(10, {
|
||||
platform: 'darwin',
|
||||
timeoutMs: 321,
|
||||
now: () => CAPTURED_AT_MS
|
||||
timeoutMs: 321
|
||||
})
|
||||
expect(result).not.toBeNull()
|
||||
expect(execFileMock).toHaveBeenCalledWith(
|
||||
@@ -145,6 +156,23 @@ describe('captureDescendantSnapshot', () => {
|
||||
expect.any(Function)
|
||||
)
|
||||
})
|
||||
|
||||
it('records the identity boundary before ps starts even when it crosses a second', async () => {
|
||||
vi.setSystemTime(CAPTURED_AT_MS + 900)
|
||||
execFileMock.mockImplementation((...args: unknown[]) => {
|
||||
const callback = args.at(-1) as (error: Error | null, stdout: string) => void
|
||||
vi.setSystemTime(CAPTURED_AT_MS + 1_100)
|
||||
callback(
|
||||
null,
|
||||
['10 1 10 Tue Jul 14 12:00:00 2026', '20 10 20 Tue Jul 14 12:00:00 2026'].join('\n')
|
||||
)
|
||||
})
|
||||
|
||||
const result = await captureDescendantSnapshot(10, { platform: 'darwin' })
|
||||
|
||||
expect(result?.capturedAtMs).toBe(CAPTURED_AT_MS + 900)
|
||||
expect(result?.descendants).toEqual([row(20, 10, 20, 'Tue Jul 14 12:00:00 2026')])
|
||||
})
|
||||
})
|
||||
|
||||
describe('terminateDescendantSnapshot', () => {
|
||||
@@ -159,7 +187,7 @@ describe('terminateDescendantSnapshot', () => {
|
||||
const sendSignal = vi.fn()
|
||||
terminateDescendantSnapshot(snapshot([row(20, 10, 20), row(30, 20, 30)]), {
|
||||
sendSignal,
|
||||
readTable: vi.fn().mockResolvedValue([])
|
||||
readTable: vi.fn().mockResolvedValue(tableCapture([]))
|
||||
})
|
||||
expect(sendSignal.mock.calls).toEqual([
|
||||
[20, 'SIGTERM'],
|
||||
@@ -176,7 +204,9 @@ describe('terminateDescendantSnapshot', () => {
|
||||
// belongs to a different (recycled) process with a different start time.
|
||||
const readTable = vi
|
||||
.fn()
|
||||
.mockResolvedValue([survivor, { ...recycled, startedAt: 'Tue Jul 14 09:00:00 2026' }])
|
||||
.mockResolvedValue(
|
||||
tableCapture([survivor, { ...recycled, startedAt: 'Tue Jul 14 09:00:00 2026' }])
|
||||
)
|
||||
terminateDescendantSnapshot(snapshot([exited, survivor, recycled]), { sendSignal, readTable })
|
||||
sendSignal.mockClear()
|
||||
await vi.advanceTimersByTimeAsync(DESCENDANT_KILL_GRACE_MS)
|
||||
@@ -200,12 +230,12 @@ describe('terminateDescendantSnapshot', () => {
|
||||
expect(sendSignal).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not SIGKILL when second-resolution start identity is ambiguous', async () => {
|
||||
it('uses the source scan boundary when a caller crosses into the next second', async () => {
|
||||
const sameSecond = row(20, 10, 20, 'Tue Jul 14 12:00:00 2026')
|
||||
const sendSignal = vi.fn()
|
||||
terminateDescendantSnapshot(snapshot([sameSecond]), {
|
||||
terminateDescendantSnapshot(snapshot([sameSecond], 10, CAPTURED_AT_MS + 900), {
|
||||
sendSignal,
|
||||
readTable: vi.fn().mockResolvedValue([sameSecond])
|
||||
readTable: vi.fn().mockResolvedValue(tableCapture([sameSecond], CAPTURED_AT_MS + 3_000))
|
||||
})
|
||||
sendSignal.mockClear()
|
||||
await vi.advanceTimersByTimeAsync(DESCENDANT_KILL_GRACE_MS)
|
||||
@@ -216,7 +246,7 @@ describe('terminateDescendantSnapshot', () => {
|
||||
const sendSignal = vi.fn()
|
||||
terminateDescendantSnapshot(snapshot([row(20, 10, 20)]), {
|
||||
sendSignal,
|
||||
readTable: vi.fn().mockReturnValue(new Promise<ProcessTableRow[]>(() => {}))
|
||||
readTable: vi.fn().mockReturnValue(new Promise<ProcessTableCapture>(() => {}))
|
||||
})
|
||||
sendSignal.mockClear()
|
||||
await vi.advanceTimersByTimeAsync(DESCENDANT_KILL_GRACE_MS + DESCENDANT_SNAPSHOT_TIMEOUT_MS)
|
||||
@@ -226,31 +256,51 @@ describe('terminateDescendantSnapshot', () => {
|
||||
})
|
||||
|
||||
describe('createProcessTableSnapshotReader', () => {
|
||||
it('coalesces concurrent and short sequential teardown scans', async () => {
|
||||
let now = 1_000
|
||||
const readFresh = vi.fn().mockResolvedValue([row(10, 1, 10)])
|
||||
const readTable = createProcessTableSnapshotReader(readFresh, {
|
||||
reuseMs: 50,
|
||||
now: () => now
|
||||
})
|
||||
it('coalesces same-turn teardown requests onto one fresh scan', async () => {
|
||||
const capture = tableCapture([row(10, 1, 10)])
|
||||
const readFresh = vi.fn().mockResolvedValue(capture)
|
||||
const readTable = createProcessTableSnapshotReader(readFresh)
|
||||
|
||||
await Promise.all(Array.from({ length: 20 }, () => readTable(1_000)))
|
||||
await readTable(1_000)
|
||||
const results = await Promise.all(Array.from({ length: 20 }, () => readTable(1_000)))
|
||||
expect(results.every((result) => result === capture)).toBe(true)
|
||||
expect(readFresh).toHaveBeenCalledOnce()
|
||||
|
||||
now += 51
|
||||
await readTable(1_000)
|
||||
expect(readFresh).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('does not cache failed process-table reads', async () => {
|
||||
it('queues a post-request scan when another scan has already started', async () => {
|
||||
const firstGate = deferred<ProcessTableCapture>()
|
||||
const secondGate = deferred<ProcessTableCapture>()
|
||||
const readFresh = vi
|
||||
.fn()
|
||||
.mockReturnValueOnce(firstGate.promise)
|
||||
.mockReturnValueOnce(secondGate.promise)
|
||||
const readTable = createProcessTableSnapshotReader(readFresh)
|
||||
|
||||
const first = readTable()
|
||||
await Promise.resolve()
|
||||
expect(readFresh).toHaveBeenCalledOnce()
|
||||
|
||||
const laterA = readTable()
|
||||
const laterB = readTable()
|
||||
firstGate.resolve(tableCapture([row(10, 1, 10)], CAPTURED_AT_MS))
|
||||
await first
|
||||
await vi.waitFor(() => expect(readFresh).toHaveBeenCalledTimes(2))
|
||||
|
||||
const newer = tableCapture([row(20, 1, 20)], CAPTURED_AT_MS + 1_000)
|
||||
secondGate.resolve(newer)
|
||||
await expect(Promise.all([laterA, laterB])).resolves.toEqual([newer, newer])
|
||||
})
|
||||
|
||||
it('retries after failed process-table reads', async () => {
|
||||
const readFresh = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error('ps failed'))
|
||||
.mockResolvedValueOnce([])
|
||||
.mockResolvedValueOnce(tableCapture([]))
|
||||
const readTable = createProcessTableSnapshotReader(readFresh)
|
||||
await expect(readTable()).rejects.toThrow('ps failed')
|
||||
await expect(readTable()).resolves.toEqual([])
|
||||
await expect(readTable()).resolves.toEqual(tableCapture([]))
|
||||
expect(readFresh).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
@@ -266,13 +316,12 @@ describe('killWithDescendantSweep', () => {
|
||||
it('signals descendants after snapshot resolution, then kills the root', async () => {
|
||||
const events: string[] = []
|
||||
const sendSignal = vi.fn(() => events.push('descendant-term'))
|
||||
const readTable = vi.fn().mockResolvedValue([row(10, 1, 10), row(20, 10, 20)])
|
||||
const readTable = vi.fn().mockResolvedValue(tableCapture([row(10, 1, 10), row(20, 10, 20)]))
|
||||
const killRoot = vi.fn(() => events.push('root-kill'))
|
||||
const pending = killWithDescendantSweep(10, killRoot, {
|
||||
readTable,
|
||||
sendSignal,
|
||||
platform: 'darwin',
|
||||
now: () => CAPTURED_AT_MS
|
||||
platform: 'darwin'
|
||||
})
|
||||
expect(killRoot).not.toHaveBeenCalled()
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
|
||||
@@ -2,7 +2,6 @@ import { execFile } from 'node:child_process'
|
||||
|
||||
export const DESCENDANT_KILL_GRACE_MS = 2_000
|
||||
export const DESCENDANT_SNAPSHOT_TIMEOUT_MS = 1_000
|
||||
export const PROCESS_TABLE_REUSE_MS = 50
|
||||
// Why: a full process table on a busy host can exceed execFile's 1MB default;
|
||||
// truncation would silently drop descendants from the snapshot.
|
||||
const PS_MAX_BUFFER_BYTES = 32 * 1024 * 1024
|
||||
@@ -24,7 +23,13 @@ export type DescendantSnapshot = {
|
||||
capturedAtMs: number
|
||||
}
|
||||
|
||||
export type ProcessTableReader = (timeoutMs?: number) => Promise<ProcessTableRow[]>
|
||||
export type ProcessTableCapture = {
|
||||
rows: ProcessTableRow[]
|
||||
/** Start boundary of the scan that produced rows, never a later consumer's time. */
|
||||
capturedAtMs: number
|
||||
}
|
||||
|
||||
export type ProcessTableReader = (timeoutMs?: number) => Promise<ProcessTableCapture>
|
||||
export type SignalSender = (pid: number, signal: NodeJS.Signals) => void
|
||||
|
||||
export function parseProcessTable(psOutput: string): ProcessTableRow[] {
|
||||
@@ -48,7 +53,10 @@ export function parseProcessTable(psOutput: string): ProcessTableRow[] {
|
||||
|
||||
function readFreshProcessTable(
|
||||
timeoutMs = DESCENDANT_SNAPSHOT_TIMEOUT_MS
|
||||
): Promise<ProcessTableRow[]> {
|
||||
): Promise<ProcessTableCapture> {
|
||||
// Why: identity safety must use the boundary before ps starts. Stamping the
|
||||
// result later could make a capture-second PID look safe after a rollover.
|
||||
const capturedAtMs = Date.now()
|
||||
return new Promise((resolve, reject) => {
|
||||
execFile(
|
||||
'ps',
|
||||
@@ -63,49 +71,57 @@ function readFreshProcessTable(
|
||||
reject(error)
|
||||
return
|
||||
}
|
||||
resolve(parseProcessTable(stdout))
|
||||
resolve({ rows: parseProcessTable(stdout), capturedAtMs })
|
||||
}
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
/** Coalesces teardown bursts onto one process-table subprocess and briefly
|
||||
* reuses the completed table so sequential worktree cleanup stays O(1) scans. */
|
||||
/** Coalesces same-turn teardown bursts but never serves a completed or already
|
||||
* started scan to a later request, because stale PIDs are unsafe to signal. */
|
||||
export function createProcessTableSnapshotReader(
|
||||
readFresh: ProcessTableReader,
|
||||
deps: { reuseMs?: number; now?: () => number } = {}
|
||||
readFresh: ProcessTableReader
|
||||
): ProcessTableReader {
|
||||
const reuseMs = deps.reuseMs ?? PROCESS_TABLE_REUSE_MS
|
||||
const now = deps.now ?? Date.now
|
||||
let inFlight: Promise<ProcessTableRow[]> | null = null
|
||||
let cached: { rows: ProcessTableRow[]; expiresAt: number } | null = null
|
||||
let inFlight: Promise<ProcessTableCapture> | null = null
|
||||
let queued: { promise: Promise<ProcessTableCapture>; started: boolean } | null = null
|
||||
|
||||
return (timeoutMs) => {
|
||||
if (cached && now() <= cached.expiresAt) {
|
||||
return Promise.resolve(cached.rows)
|
||||
}
|
||||
if (inFlight) {
|
||||
return inFlight
|
||||
if (queued && !queued.started) {
|
||||
return queued.promise
|
||||
}
|
||||
|
||||
const request = readFresh(timeoutMs).then((rows) => {
|
||||
cached = { rows, expiresAt: now() + reuseMs }
|
||||
return rows
|
||||
})
|
||||
inFlight = request
|
||||
void request.then(
|
||||
() => {
|
||||
if (inFlight === request) {
|
||||
inFlight = null
|
||||
const prior = queued?.promise ?? inFlight
|
||||
const entry: { promise: Promise<ProcessTableCapture>; started: boolean } = {
|
||||
promise: Promise.resolve(undefined as never),
|
||||
started: false
|
||||
}
|
||||
entry.promise = Promise.resolve().then(async () => {
|
||||
if (prior) {
|
||||
try {
|
||||
await prior
|
||||
} catch {
|
||||
// The post-request scan below owns this caller's result.
|
||||
}
|
||||
},
|
||||
() => {
|
||||
}
|
||||
entry.started = true
|
||||
const request = readFresh(timeoutMs)
|
||||
inFlight = request
|
||||
try {
|
||||
return await request
|
||||
} finally {
|
||||
if (inFlight === request) {
|
||||
inFlight = null
|
||||
}
|
||||
}
|
||||
)
|
||||
return request
|
||||
})
|
||||
queued = entry
|
||||
const clearQueued = (): void => {
|
||||
if (queued === entry) {
|
||||
queued = null
|
||||
}
|
||||
}
|
||||
void entry.promise.then(clearQueued, clearQueued)
|
||||
return entry.promise
|
||||
}
|
||||
}
|
||||
|
||||
@@ -114,16 +130,16 @@ const readProcessTable = createProcessTableSnapshotReader(readFreshProcessTable)
|
||||
function readProcessTableBeforeDeadline(
|
||||
readTable: ProcessTableReader,
|
||||
timeoutMs: number
|
||||
): Promise<ProcessTableRow[] | null> {
|
||||
): Promise<ProcessTableCapture | null> {
|
||||
return new Promise((resolve) => {
|
||||
let settled = false
|
||||
const finish = (rows: ProcessTableRow[] | null): void => {
|
||||
const finish = (capture: ProcessTableCapture | null): void => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
settled = true
|
||||
clearTimeout(timer)
|
||||
resolve(rows)
|
||||
resolve(capture)
|
||||
}
|
||||
const timer = setTimeout(() => finish(null), timeoutMs)
|
||||
timer.unref?.()
|
||||
@@ -173,7 +189,6 @@ type SnapshotDeps = {
|
||||
readTable?: ProcessTableReader
|
||||
platform?: NodeJS.Platform
|
||||
timeoutMs?: number
|
||||
now?: () => number
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -195,12 +210,11 @@ export async function captureDescendantSnapshot(
|
||||
const timeoutMs = deps.timeoutMs ?? DESCENDANT_SNAPSHOT_TIMEOUT_MS
|
||||
// Why both layers: the deadline keeps injected/custom readers bounded while
|
||||
// the production execFile timeout actually kills a wedged ps subprocess.
|
||||
const capturedAtMs = (deps.now ?? Date.now)()
|
||||
const table = await readProcessTableBeforeDeadline(readTable, timeoutMs)
|
||||
if (!table) {
|
||||
const capture = await readProcessTableBeforeDeadline(readTable, timeoutMs)
|
||||
if (!capture) {
|
||||
return null
|
||||
}
|
||||
return collectDescendantRows(rootPid, table, capturedAtMs)
|
||||
return collectDescendantRows(rootPid, capture.rows, capture.capturedAtMs)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -273,11 +287,11 @@ export function terminateDescendantSnapshot(
|
||||
void readProcessTableBeforeDeadline(
|
||||
readTable,
|
||||
deps.timeoutMs ?? DESCENDANT_SNAPSHOT_TIMEOUT_MS
|
||||
).then((table) => {
|
||||
if (!table) {
|
||||
).then((capture) => {
|
||||
if (!capture) {
|
||||
return
|
||||
}
|
||||
const liveByPid = new Map(table.map((row) => [row.pid, row]))
|
||||
const liveByPid = new Map(capture.rows.map((row) => [row.pid, row]))
|
||||
for (const row of snapshot.descendants) {
|
||||
const live = liveByPid.get(row.pid)
|
||||
if (
|
||||
|
||||
Reference in New Issue
Block a user