diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml index 40492a3937..f24c1ec5d3 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml @@ -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/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md -2026-07-31-code-runtime-python-settlement-fixes.md: b51fb2e9c28d07efa1691b1036b7b6e899b21062 -2026-07-31-code-runtime-python-settlement-fixes.zh.md: a59e5457f2469e86ed4120ce47ed8a5d8ae08f4f +2026-07-31-code-runtime-python-settlement-fixes.md: 40e5df01748889a00c19202da3e6565794383efa +2026-07-31-code-runtime-python-settlement-fixes.zh.md: aed7bdd63c4555a12e0692d5876ed8058c14133c diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md index b51fb2e9c2..40e5df0174 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md +++ b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md @@ -30,7 +30,7 @@ The load-time check that rejects a `maxLogBytes`/`maxValueBytes` larger than one ### Same-group survivors are reaped before the fiber goes quiescent -A model program can leave a descendant in the child's OWN process group (no `setsid`, so `kill(-pid)` reaches it) that ignores SIGTERM but releases the inherited stdout/stderr/fd-3 pipes. The leader then exits, its `close` fires because the pipes drained, and settlement runs while that descendant is still alive. `kill()` arms an `unref`'d SIGKILL timer after SIGTERM; the fix is that `settle()` no longer resolves the run's `finished` promise immediately when an escalation is in flight. Instead, when `killing` is set and the process group is not yet empty (`process.kill(-pid, 0)` does not throw ESRCH), it polls the group on a REF'd timer, bounded by `graceMs + CLOSE_REAP_MARGIN_MS`, and resolves `finished` only once the group has emptied. The ref'd poll is the load-bearing part: it keeps the host event loop alive until the SIGKILL has actually reaped the group, so even a short-lived host — a one-shot headless run, a config subprocess — cannot exit and reparent the survivor to init. In the normal case (the leader was the only member) the first probe returns ESRCH and settlement resolves with zero added latency. `teardown()` awaits each run's `finished`, so disposal is genuinely quiescent, matching its JSDoc. +A model program can leave a descendant in the child's OWN process group (no `setsid`, so `kill(-pid)` reaches it) that ignores SIGTERM but releases the inherited stdout/stderr/fd-3 pipes. The leader then exits, its `close` fires because the pipes drained, and settlement runs while that descendant is still alive. `kill()` arms an `unref`'d SIGKILL timer after SIGTERM; the fix is that `settle()` no longer resolves the run's `finished` promise — nor drops the run from `live` — immediately when an escalation is in flight. Instead, when `killing` is set and the process group is not yet empty (`process.kill(-pid, 0)` does not throw ESRCH), it polls the group on a REF'd timer, bounded by `graceMs + CLOSE_REAP_MARGIN_MS`, and both drops the run from `live` and resolves `finished` only once the group has emptied. The ref'd poll is the load-bearing part: it keeps the host event loop alive until the SIGKILL has actually reaped the group, so even a short-lived host — a one-shot headless run, a config subprocess — cannot exit and reparent the survivor to init. Deferring the `live` removal is what makes a `dispose()` racing a just-resolved `run()` still await the survivor: dropping the run from `live` at settlement (before the reap) would let teardown snapshot an empty set and return while the descendant lived. In the normal case (the leader was the only member) the first probe returns ESRCH and settlement finalizes with zero added latency. `teardown()` awaits each run's `finished`, so disposal is genuinely quiescent, matching its JSDoc — including for a run that already resolved. Settlement also CANCELS the SIGKILL timer the moment the group is confirmed empty (the normal path, and when the poll sees the survivor gone). Leaving it armed would expose a PID-reuse hazard: a `kill(-pid)` left pending for up to `graceMs` after the leader was reaped could hit a RECYCLED pgid once the kernel reused the leader's pid, SIGKILLing an unrelated group (`killGroup` swallowing ESRCH does not help — the danger is precisely the kill that SUCCEEDS against a reused group). Clearing it on the empty probe bounds the reuse window to only the genuine-survivor case, where the group cannot be empty to reuse. diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md index a59e5457f2..aed7bdd63c 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md @@ -30,7 +30,7 @@ Status: implemented ### Same-group survivors are reaped before the fiber goes quiescent -模型程序可能在子进程自己的进程组里(没有 `setsid`,因此 `kill(-pid)` 能到达它)留下一个后代,它忽略 SIGTERM,但释放了继承而来的 stdout/stderr/fd-3 管道。随后 leader 退出,由于管道已被抽空,它的 `close` 触发,于是结算在那个后代仍存活时运行。`kill()` 在 SIGTERM 之后装设一个 `unref` 的 SIGKILL 定时器;本次修复是,当有一次升级正在进行时,`settle()` 不再立即 resolve 该次运行的 `finished` promise。取而代之的是,当 `killing` 被置位且进程组尚未为空时(`process.kill(-pid, 0)` 不抛出 ESRCH),它在一个 ref 的定时器上轮询该进程组,以 `graceMs + CLOSE_REAP_MARGIN_MS` 为界,仅当进程组已清空后才 resolve `finished`。这个 ref 的轮询是承重部分:它让宿主事件循环保持存活,直到 SIGKILL 真正回收了该进程组,因此即使是一个短命的宿主(一次性的 headless 运行、一个配置子进程)也无法退出并把存活者 reparent 给 init。在正常情况下(leader 是唯一成员),第一次探测返回 ESRCH,结算以零附加延迟完成 resolve。`teardown()` 会 await 每次运行的 `finished`,因此 dispose 是真正完全停稳的,与其 JSDoc 相符。 +模型程序可能在子进程自己的进程组里(没有 `setsid`,因此 `kill(-pid)` 能到达它)留下一个后代,它忽略 SIGTERM,但释放了继承而来的 stdout/stderr/fd-3 管道。随后 leader 退出,由于管道已被抽空,它的 `close` 触发,于是结算在那个后代仍存活时运行。`kill()` 在 SIGTERM 之后装设一个 `unref` 的 SIGKILL 定时器;本次修复是,当有一次升级正在进行时,`settle()` 既不立即 resolve 该次运行的 `finished` promise,也不立即把该运行从 `live` 中移除。取而代之的是,当 `killing` 被置位且进程组尚未为空时(`process.kill(-pid, 0)` 不抛出 ESRCH),它在一个 ref 的定时器上轮询该进程组,以 `graceMs + CLOSE_REAP_MARGIN_MS` 为界,仅当进程组已清空后才把该运行从 `live` 移除并 resolve `finished`。这个 ref 的轮询是承重部分:它让宿主事件循环保持存活,直到 SIGKILL 真正回收了该进程组,因此即使是一个短命的宿主(一次性的 headless 运行、一个配置子进程)也无法退出并把存活者 reparent 给 init。把 `live` 的移除推迟,正是让一个与刚返回的 `run()` 竞争的 `dispose()` 仍会 await 该存活者的原因:若在回收之前就把运行从 `live` 移除,teardown 会快照到一个空集合并在后代仍存活时返回。在正常情况下(leader 是唯一成员),第一次探测返回 ESRCH,结算以零附加延迟完成收尾。`teardown()` 会 await 每次运行的 `finished`,因此 dispose 是真正完全停稳的,与其 JSDoc 相符——包括对一个已经 resolve 的运行也是如此。 结算还会在进程组被确认为空的那一刻取消 SIGKILL 定时器(正常路径,以及轮询看到存活者已消失时)。让它继续处于装设状态会暴露一个 PID 复用隐患:一个在 leader 被回收后仍挂起长达 `graceMs` 的 `kill(-pid)`,可能在内核复用了 leader 的 pid 之后击中一个被回收(recycled)的 pgid,从而 SIGKILL 掉一个无关的进程组(`killGroup` 吞掉 ESRCH 并无帮助——危险恰恰是那次针对被复用进程组成功执行的 kill)。在空进程组探测时清除它,把复用窗口收窄到只剩真正存在存活者的情形,此时进程组不可能为空以供复用。 diff --git a/packages/code-runtime/code-runtime-python/src/index.ts b/packages/code-runtime/code-runtime-python/src/index.ts index d662e36e07..575a9076e6 100644 --- a/packages/code-runtime/code-runtime-python/src/index.ts +++ b/packages/code-runtime/code-runtime-python/src/index.ts @@ -1032,20 +1032,7 @@ export class PythonCodeRuntime extends CodeRuntime { const settle = (result: Omit): void => { if (resolved) return resolved = true - // The grace-window SIGKILL timer is intentionally NOT cleared here: a - // same-group descendant that ignored SIGTERM but released the pipes lets - // `close` fire (and settle() run) while it is still alive, so the pending - // SIGKILL must remain armed to reap it (see kill()). The timer is - // `unref`'d; quiescence does not depend on it firing during host lifetime - // — `finished` (below) is withheld until the group is confirmed empty. if (closeDeadline !== undefined) clearTimeout(closeDeadline) - // Drop from `live` only at settlement (close / pid-less spawn failure), - // NOT at finish(): between finish() and the child's `close` the child - // may sit in the SIGTERM grace window, and a concurrent teardown() - // snapshot of `this.live` must still see it so disposal awaits its exit - // ("no subprocess outlives the fiber"). teardown's own settle() on an - // already-finished run hits the resolved guard as a no-op. - this.live.delete(live) // The child has exited by now (settle runs on `close`, or on a spawn // that produced no pid), so its staging directory is no longer read and // this run's copy goes away with it. Removed SYNCHRONOUSLY, before @@ -1063,29 +1050,41 @@ export class PythonCodeRuntime extends CodeRuntime { // checked-in scripts. } resolve({ ...result, logs }) + // Mark the fiber quiescent for THIS run: drop it from `live` and resolve + // `finished` (what teardown awaits). Deferred until the process group is + // actually empty — dropping from `live` before then would let a + // `dispose()` that races a just-resolved run() snapshot an empty `live` + // and return while a same-group survivor is still alive, making teardown's + // "no subprocess outlives the fiber" false for that window. Keeping the + // run in `live` until the group is reaped is exactly what makes a + // concurrent teardown await it. + const finalize = (): void => { + this.live.delete(live) + finishResolve() + } // `finished` is what teardown awaits to honor "no subprocess outlives the // fiber". When no escalation ran (normal completion, no kill) or the - // group is already empty, cancel the pending SIGKILL and resolve now. + // group is already empty, cancel the pending SIGKILL and finalize now. // Clearing it is what bounds the PID-reuse hazard: an armed `kill(-pid)` // left to fire up to graceMs later could hit a RECYCLED pgid once the // kernel reused the leader's pid, SIGKILLing an unrelated group. So the // timer stays armed only while a real survivor exists — a same-group // descendant that ignored SIGTERM but released the pipes, still alive // here because its `close` is what got us to settle. In that case - // withhold `finished` and poll the group on REF'd timers (a short-lived + // withhold finalize and poll the group on REF'd timers (a short-lived // host would otherwise exit before the unref'd SIGKILL fired, reparenting // the survivor to init), clearing the timer the moment the group empties; // the wait is bounded by the same graceMs + margin the escalation uses. if (!killing || groupEmpty()) { if (graceTimer !== undefined) clearTimeout(graceTimer) - finishResolve() + finalize() return } const deadline = Date.now() + this.config.graceMs + CLOSE_REAP_MARGIN_MS const pollGroup = (): void => { if (groupEmpty() || Date.now() >= deadline) { if (graceTimer !== undefined) clearTimeout(graceTimer) - finishResolve() + finalize() return } setTimeout(pollGroup, GROUP_REAP_POLL_MS) diff --git a/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts b/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts index 60e2ec386f..de958ef11d 100644 --- a/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts +++ b/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts @@ -2053,6 +2053,55 @@ describe('PythonCodeRuntime — budgets, termination, disposal', () => { } expect(still).toBe(true) }, 20_000) + + it('dispose awaits reaping of a same-group survivor from a completed run', async () => { + // The quiescence contract also holds for a run that ALREADY resolved: the run + // stays tracked in `live` until its process group is reaped, so a `dispose()` + // that races a just-returned run() still awaits the survivor rather than + // snapshotting an empty `live` and returning while it lives. Here the run + // completes (leaving a SIGTERM-ignoring same-group descendant), then dispose() + // is called; the heartbeat must be stale BY THE TIME dispose() resolves — + // proving teardown waited for the reap, not merely that the reap eventually + // happened. + const handoff = await mkdtemp(join(tmpdir(), 'dsh-dispose-quiesce-')) + const readyMarker = join(handoff, 'ready') + const heartbeat = join(handoff, 'heartbeat') + const { runtime, fiber } = await setup({ maxWallMs: 10_000, graceMs: 300 }) + const result = await runtime.run({ + program: [ + 'import subprocess, sys, os, time', + `marker = ${JSON.stringify(readyMarker)}`, + `heartbeat = ${JSON.stringify(heartbeat)}`, + 'code = ("import signal, sys, time\\n"', + ' "signal.signal(signal.SIGTERM, signal.SIG_IGN)\\n"', + ' "open(sys.argv[1], \'w\').close()\\n"', + ' "end = time.time() + 30\\n"', + ' "while time.time() < end:\\n"', + ' " open(sys.argv[2], \'w\').close()\\n"', + ' " time.sleep(0.05)\\n")', + 'child = subprocess.Popen([sys.executable, "-c", code, marker, heartbeat],', + ' stdin=subprocess.DEVNULL,', + ' stdout=subprocess.DEVNULL,', + ' stderr=subprocess.DEVNULL)', + 'deadline = time.time() + 5', + 'while not os.path.exists(marker) and time.time() < deadline:', + ' time.sleep(0.02)', + 'return "spawned"', + ].join('\n'), + bindings: [], + }) + expect(result.error).toBeUndefined() + expect(existsSync(readyMarker)).toBe(true) + // dispose() must not return until the group is reaped. After it resolves, the + // heartbeat must already be stale: read its mtime, wait past the heartbeat + // interval, and confirm it did not advance — the descendant is no longer + // executing (reaped or zombie), so teardown was genuinely quiescent. + await fiber.dispose() + const mtime = (): number => { try { return statSync(heartbeat).mtimeMs } catch { return 0 } } + const afterDispose = mtime() + await new Promise(resolve => setTimeout(resolve, 500)) + expect(mtime()).toBe(afterDispose) + }, 20_000) }) describe('PythonCodeRuntime — hostile peer', () => {