fix: remote probe watchdog kills the probe's process group, not just its child
The watchdog SIGKILLed only the direct child of the wrapper. A remote `hermes` launcher that runs the CLI without exec leaves the hung grandchild alive after its parent dies — exactly the broken-launcher class from #110478 — so the probe still orphaned a process on the remote. Enabling job control (`set -m`) around the probe start puts it in its own process group; the watchdog now kills `-$__htp` first and the direct pid as the fallback for shells that cannot turn job control on without a tty (dash), where behaviour is unchanged. Review finding: watchdog SIGKILLs only its direct child; a non-exec launcher leaves the hung grandchild orphaned.
This commit is contained in:
@@ -1845,7 +1845,7 @@ test('probes run under the remote watchdog so a hung CLI cannot orphan (#110478)
|
||||
|
||||
assert.equal(await remoteSupportsSshOwnership(helpSsh, '/x/hermes'), true)
|
||||
assert.ok(helpProbe.includes('kill -9'), 'ownership probe wrapped in the remote watchdog')
|
||||
assert.ok(helpProbe.includes('$( ('), 'watchdog nested around the inner serve --help')
|
||||
assert.ok(/\$\(.*\(.*serve --help.*\) <\/dev\/null &/.test(helpProbe), 'watchdog nested around the inner serve --help')
|
||||
})
|
||||
|
||||
test('cleanupStale escalates to SIGKILL when the backend survives the graceful wait (#91668 quit-during-active-turn)', async () => {
|
||||
|
||||
@@ -1122,4 +1122,25 @@ test('withRemoteTimeout kills a hung probe remotely instead of orphaning it (#11
|
||||
const { stdout: strays } = await execFileAsync('sh', ['-c', `ps -eo args | grep "[s]leep ${hungSecs}$" || true`])
|
||||
|
||||
assert.equal(strays.trim(), '', 'killed probe left no orphan process')
|
||||
|
||||
// … including the grandchild of a launcher that runs the CLI without exec
|
||||
// (the broken-launcher class of #110478). Needs a shell with job control
|
||||
// off a tty; bash has it, dash does not.
|
||||
const bash = await execFileAsync('sh', ['-c', 'command -v bash || true']).then(r => r.stdout.trim())
|
||||
|
||||
if (bash) {
|
||||
const grandSecs = hungSecs + 1
|
||||
const launcher = `sh -c 'sleep ${grandSecs}; echo done'`
|
||||
|
||||
const err2: any = await execFileAsync(bash, ['-c', withRemoteTimeout(launcher, 1)]).then(
|
||||
() => null,
|
||||
e => e
|
||||
)
|
||||
|
||||
assert.ok(err2 && err2.code !== 0, 'hung launcher must exit non-zero')
|
||||
|
||||
const { stdout: grandStrays } = await execFileAsync('sh', ['-c', `ps -eo args | grep "[s]leep ${grandSecs}$" || true`])
|
||||
|
||||
assert.equal(grandStrays.trim(), '', 'watchdog killed the launcher’s grandchild too')
|
||||
}
|
||||
})
|
||||
|
||||
@@ -348,9 +348,13 @@ function buildInteractiveSshArgs(conn, remoteCwd, connectTimeoutMs?, remoteComma
|
||||
function withRemoteTimeout(remoteCommand, timeoutSecs = REMOTE_PROBE_TIMEOUT_SECS) {
|
||||
const secs = Number.isFinite(timeoutSecs) && timeoutSecs > 0 ? Math.floor(timeoutSecs) : REMOTE_PROBE_TIMEOUT_SECS
|
||||
|
||||
// Job control (`set -m`) puts the probe in its own process group so the
|
||||
// watchdog can also reach a grandchild left behind by a launcher that runs
|
||||
// the CLI without exec. Shells that cannot enable it without a tty fall
|
||||
// back to killing the direct child.
|
||||
return (
|
||||
`(${remoteCommand}) </dev/null & __htp=$!; ` +
|
||||
`(sleep ${secs} </dev/null >/dev/null 2>&1; kill -9 $__htp 2>/dev/null) & __htw=$!; ` +
|
||||
`set -m 2>/dev/null; (${remoteCommand}) </dev/null & __htp=$!; set +m 2>/dev/null; ` +
|
||||
`(sleep ${secs} </dev/null >/dev/null 2>&1; kill -9 -- -$__htp 2>/dev/null; kill -9 $__htp 2>/dev/null) & __htw=$!; ` +
|
||||
`wait $__htp; __htrc=$?; ` +
|
||||
`kill $__htw 2>/dev/null; wait $__htw 2>/dev/null; ` +
|
||||
`exit $__htrc`
|
||||
|
||||
Reference in New Issue
Block a user