fix(desktop): SIGKILL-escalate the owned SSH backend when it survives the graceful quit wait (#91668 remainder)
The #95085 quit teardown kills the owned serve --isolated before the SSH tunnel closes, but a backend mid-turn (in-flight LLM call, live MCP children) can ride out SIGTERM past cleanupStale's 5s graceful wait. The old code then gave up (threw, kept the lockfile) and before-quit's 6s race closed SSH anyway — reparenting the still-running serve to pid 1: the reported leak, now specific to quit-during-active-turn. Escalate to kill -9 with a confirmed-exit wait; only an unkillable pid (D-state, permissions) still throws and preserves the lock record so the next connect's reap pass retries.
This commit is contained in:
@@ -1365,3 +1365,52 @@ test('remote SSH ownership capability requires both secure bootstrap flags', asy
|
||||
const unsupported = fakeSsh([[/serve --help/, 'NO\n']])
|
||||
assert.equal(await remoteSupportsSshOwnership(unsupported, '/x/hermes'), false)
|
||||
})
|
||||
|
||||
test('cleanupStale escalates to SIGKILL when the backend survives the graceful wait (#91668 quit-during-active-turn)', async () => {
|
||||
// A serve mid-turn (in-flight LLM call, live MCP children) can ride out
|
||||
// SIGTERM well past the 5s graceful wait. Before-quit races the whole
|
||||
// teardown against 6s and then closes SSH — so a give-up here reparents
|
||||
// the still-running backend to pid 1: exactly the #91668 leak. The
|
||||
// graceful-wait failure must escalate to SIGKILL and still drop the lock.
|
||||
const ssh = fakeSsh([
|
||||
[/print\("OWNED"/, 'OWNED\n'],
|
||||
[(cmd: string) => /kill 9 &&/.test(cmd), new Error('exit 1: pid alive after graceful wait')]
|
||||
])
|
||||
|
||||
await cleanupStale(ssh, OWNERSHIP_ID, {
|
||||
pid: 9,
|
||||
spawnNonce: SPAWN_NONCE,
|
||||
hermesPath: '/x/hermes',
|
||||
logPath: spawnLogPath(OWNERSHIP_ID, SPAWN_NONCE)
|
||||
})
|
||||
|
||||
assert.ok(
|
||||
ssh.calls.some(c => /kill -9 9\b/.test(c)),
|
||||
'must escalate to SIGKILL after the graceful wait fails'
|
||||
)
|
||||
assert.ok(
|
||||
ssh.calls.some(c => /rm -f .*backend\.lock\.json/.test(c)),
|
||||
'lockfile must still be dropped after the forced kill'
|
||||
)
|
||||
})
|
||||
|
||||
test('cleanupStale keeps the lockfile when even SIGKILL cannot confirm the pid died', async () => {
|
||||
const ssh = fakeSsh([
|
||||
[/print\("OWNED"/, 'OWNED\n'],
|
||||
[(cmd: string) => /kill 9 &&/.test(cmd), new Error('exit 1: pid alive after graceful wait')],
|
||||
[(cmd: string) => /kill -9 9\b/.test(cmd), new Error('exit 1: unkillable (D-state)')]
|
||||
])
|
||||
|
||||
await assert.rejects(
|
||||
cleanupStale(ssh, OWNERSHIP_ID, {
|
||||
pid: 9,
|
||||
spawnNonce: SPAWN_NONCE,
|
||||
hermesPath: '/x/hermes',
|
||||
logPath: spawnLogPath(OWNERSHIP_ID, SPAWN_NONCE)
|
||||
}),
|
||||
/Could not terminate/
|
||||
)
|
||||
|
||||
// The record must survive so the next connect's reap pass retries.
|
||||
assert.ok(!ssh.calls.some(c => /rm -f .*backend\.lock\.json/.test(c)))
|
||||
})
|
||||
|
||||
@@ -506,11 +506,27 @@ async function cleanupStale(ssh, ownershipId, lock, pidAlive = true) {
|
||||
).trim()
|
||||
|
||||
void result
|
||||
} catch (cause) {
|
||||
const error: any = new Error('Could not terminate the stale SSH backend.')
|
||||
error.kind = 'transient-transport-error'
|
||||
error.cause = cause
|
||||
throw error
|
||||
} catch {
|
||||
// A backend mid-turn (in-flight LLM call, live MCP children) can ride
|
||||
// out SIGTERM past the 5s graceful wait — and before-quit races this
|
||||
// whole teardown against 6s before closing SSH, so giving up here
|
||||
// reparents the still-running serve to pid 1: the #91668 leak, now on
|
||||
// the quit-during-active-turn path. Escalate to SIGKILL and require a
|
||||
// confirmed exit before treating the record as reclaimed.
|
||||
try {
|
||||
await ssh.exec(
|
||||
`kill -9 ${Number(lock.pid)} 2>/dev/null; ` +
|
||||
`i=0; while kill -0 ${Number(lock.pid)} 2>/dev/null; do ` +
|
||||
`i=$((i+1)); [ "$i" -ge 20 ] && exit 1; sleep 0.1; done`
|
||||
)
|
||||
} catch (cause) {
|
||||
// Even SIGKILL could not confirm death (D-state, permissions). Keep
|
||||
// the lockfile so the next connect's reap pass retries.
|
||||
const error: any = new Error('Could not terminate the stale SSH backend.')
|
||||
error.kind = 'transient-transport-error'
|
||||
error.cause = cause
|
||||
throw error
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user