From 28f747d775c95cf64d006f6bdaf8b1b17e4ac2f2 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Sun, 2 Aug 2026 19:58:40 +0800 Subject: [PATCH] test(code-runtime-python): cover the reply-pump closed-loop guard; align setsid docs The closed-loop reply-pump guard now ships with a deterministic regression test: a worker thread abandons a binding so its loop closes, the host answers that call before a later binding, and the pump must survive the closed-loop call_soon_threadsafe to deliver the later reply (host-gated ordering makes it deterministic; unguarding the pump hangs the later binding to the wall clock). Align the quiescence self-description with the shipped setsid limitation: teardown()'s JSDoc and the Agent Note's Problem line now qualify "no subprocess outlives the fiber" to subprocesses that stay in the child's process group, with a setsid()-escape exception pointing at the README. Tighten the setsid-orphan fixture's self-timeout to 5s and its upper-bound assertion to <4000ms so a failed deadline backstop is a sharper red. Register the new regression tests in the note. --- ...-runtime-python-settlement-fixes.i18n.yaml | 4 +- ...31-code-runtime-python-settlement-fixes.md | 4 +- ...code-runtime-python-settlement-fixes.zh.md | 4 +- .../code-runtime-python/src/index.ts | 6 +- .../code-runtime-python/tests/runtime.spec.ts | 78 ++++++++++++++++++- 5 files changed, 85 insertions(+), 11 deletions(-) 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 f24c1ec5d3..0738766e06 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: 40e5df01748889a00c19202da3e6565794383efa -2026-07-31-code-runtime-python-settlement-fixes.zh.md: aed7bdd63c4555a12e0692d5876ed8058c14133c +2026-07-31-code-runtime-python-settlement-fixes.md: 620a4180f63e738c8ee6954d903f437aac3d068f +2026-07-31-code-runtime-python-settlement-fixes.zh.md: ab31e8fbfe68a559b2e88690a8dfadc8840d0d5c 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 40e5df0174..620a4180f6 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 @@ -6,7 +6,7 @@ English | [中文](2026-07-31-code-runtime-python-settlement-fixes.zh.md) ## Problem -The CPython subprocess backend for Code Mode, built on the [fd-3 frame protocol](../architecture/2026-07-31-code-runtime-python-fd3-protocol.md), resolves every program outcome as a `CodeRunResult`, rejects `run()` only for seam misuse, and disposes to quiescence so no subprocess outlives the fiber. A sequence of review passes surfaced defects that broke those contracts in ways unit coverage did not catch — each hid behind a `/* v8 ignore */`, a captured-callable that read as a fix but was not, a memory effect invisible through the seam, a load-time bound that double-counted, a process-group escalation that a survivor could outlast, or a cross-event-loop completion that silently deadlocked. Each fix ships with a test that fails without it. +The CPython subprocess backend for Code Mode, built on the [fd-3 frame protocol](../architecture/2026-07-31-code-runtime-python-fd3-protocol.md), resolves every program outcome as a `CodeRunResult`, rejects `run()` only for seam misuse, and disposes to quiescence so no subprocess that stays in the child's process group outlives the fiber (a descendant that escapes the group with `setsid()` is the documented exception — see the package README's Known Limitations). A sequence of review passes surfaced defects that broke those contracts in ways unit coverage did not catch — each hid behind a `/* v8 ignore */`, a captured-callable that read as a fix but was not, a memory effect invisible through the seam, a load-time bound that double-counted, a process-group escalation that a survivor could outlast, or a cross-event-loop completion that silently deadlocked. Each fix ships with a test that fails without it. ## Decision @@ -46,7 +46,7 @@ Also in `py/bootstrap.py`, a binding reply Future is created on the loop that ra - `tests/boot-write-failure.spec.ts` mocks `spawn` so the fd-3 pipe throws on the boot write — the one path a real subprocess cannot be coerced into — and asserts `run()` resolves a `worker-exit` rather than rejecting. Isolated in its own spec so the real-subprocess suite is untouched. - `tests/residual-detach.spec.ts` unit-tests `detachResidual`: the carried copy equals the residual, owns a backing store sized to its own length (fixture kept above Node's Buffer pool threshold), and does not share the source frame's `ArrayBuffer`. -- `tests/runtime.spec.ts` — the output-cap case asserts the `ceiling - envelope` bound (268435392) and its message. A daemon-thread case drives four threads emitting unterminated writes through settlement's flush. The same-group reap case spawns a SIGTERM-ignoring same-group descendant that releases the pipes and bumps a heartbeat file; the test asserts the heartbeat STOPS after the grace-window SIGKILL — an assertion robust whether the killed descendant is reaped or lingers as a zombie, so it holds where PID 1 does not wait() orphans. The cross-loop case runs a binding from a worker thread's own `asyncio.run` loop while the main coroutine yields with `await asyncio.sleep`, asserting the reply round-trips instead of timing out. The inherited-soft-limit case runs the interpreter through a `ulimit -S -t` wrapper that sets a CPU soft limit below `cpuSeconds` and asserts the applied `RLIMIT_CPU` soft is the inherited value, not the configured one (CPU rather than address space, since macOS ignores `ulimit -v`). A companion case inherits a 1 s CPU soft, has the program trap SIGXCPU and busy-loop past it, and asserts the settlement recheck reports a timeout — proving the recheck uses the effective soft, not the configured `cpuSeconds`. +- `tests/runtime.spec.ts` — the output-cap case asserts the `ceiling - envelope` bound (268435392) and its message. A daemon-thread case drives four threads emitting unterminated writes through settlement's flush. The same-group reap case spawns a SIGTERM-ignoring same-group descendant that releases the pipes and bumps a heartbeat file; the test asserts the heartbeat STOPS after the grace-window SIGKILL — an assertion robust whether the killed descendant is reaped or lingers as a zombie, so it holds where PID 1 does not wait() orphans. A dispose-after-resolve case asserts `dispose()` of a completed run with a same-group survivor returns only after the survivor stops executing (proving the run stays in `live` until its group is reaped). A deadline case busy-blocks the event loop past both timers and asserts the survivor's heartbeat freezes (proving the poll's deadline arm sends SIGKILL itself rather than cancelling the unfired escalation). The cross-loop case runs a binding from a worker thread's own `asyncio.run` loop while the main coroutine yields with `await asyncio.sleep`, asserting the reply round-trips instead of timing out; a companion case abandons a thread's call so its loop closes, then answers it before a later binding — asserting the pump survives the closed-loop `call_soon_threadsafe` (host-gated ordering makes it deterministic, fail-before hangs the later binding to the wall clock). The inherited-soft-limit case runs the interpreter through a `ulimit -S -t` wrapper that sets a CPU soft limit below `cpuSeconds` and asserts the applied `RLIMIT_CPU` soft is the inherited value, not the configured one (CPU rather than address space, since macOS ignores `ulimit -v`); a companion inherits a 1 s CPU soft, has the program trap SIGXCPU and busy-loop past it, and asserts the settlement recheck reports a timeout — proving the recheck uses the effective soft, not the configured `cpuSeconds`. A control-heavy-diagnostic case raises a NUL-flood exception under a small `maxValueBytes` and asserts the serialized frame fits (proving the diagnostic is metered by serialized cost). A non-integer-budget case asserts a fractional `maxLogBytes`/`maxValueBytes` rejects at load. ## Alternatives considered 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 aed7bdd63c..ab31e8fbfe 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 @@ -6,7 +6,7 @@ Status: implemented ## Problem -用于 Code Mode 的 CPython 子进程后端建立在 [fd-3 帧协议](../architecture/2026-07-31-code-runtime-python-fd3-protocol.md)之上，把每个程序结果都 resolve 成一个 `CodeRunResult`，仅在 seam 被误用时才 reject `run()`，并且会 dispose 到完全停稳，从而没有任何子进程存活得比 fiber 更久。一连串审查暴露出一些缺陷，它们以单元测试覆盖率无法捕获的方式破坏了这些契约：每一个都藏在一处 `/* v8 ignore */` 之后、一个读起来像修复但实际并非修复的捕获可调用对象之后、一处透过 seam 不可见的内存效应之后、一处重复计数的加载期上界之后、一处存活者能够熬过的进程组升级之后，或者一处静默死锁的跨事件循环完成之后。每处修复都附带一个在缺少它时会失败的测试。 +用于 Code Mode 的 CPython 子进程后端建立在 [fd-3 帧协议](../architecture/2026-07-31-code-runtime-python-fd3-protocol.md)之上，把每个程序结果都 resolve 成一个 `CodeRunResult`，仅在 seam 被误用时才 reject `run()`，并且会 dispose 到完全停稳，从而没有任何留在子进程自己进程组内的子进程存活得比 fiber 更久（一个用 `setsid()` 逃出该进程组的后代是有文档记载的例外——见该包 README 的 Known Limitations）。一连串审查暴露出一些缺陷，它们以单元测试覆盖率无法捕获的方式破坏了这些契约：每一个都藏在一处 `/* v8 ignore */` 之后、一个读起来像修复但实际并非修复的捕获可调用对象之后、一处透过 seam 不可见的内存效应之后、一处重复计数的加载期上界之后、一处存活者能够熬过的进程组升级之后，或者一处静默死锁的跨事件循环完成之后。每处修复都附带一个在缺少它时会失败的测试。 ## Decision @@ -46,7 +46,7 @@ Status: implemented - `tests/boot-write-failure.spec.ts` 对 `spawn` 做 mock，使 fd-3 管道在引导写入时抛出异常（这是真实子进程无法被迫进入的唯一路径），并断言 `run()` resolve 出一个 `worker-exit` 而非 reject。它被隔离在自己的 spec 中，因此真实子进程测试套件不受影响。 - `tests/residual-detach.spec.ts` 对 `detachResidual` 做单元测试：向前传递的副本与残余数据相等、拥有一个大小与其自身长度一致的底层存储（fixture 保持在 Node 的 Buffer 池阈值之上），并且不与源帧的 `ArrayBuffer` 共享。 -- `tests/runtime.spec.ts`：output-cap 用例断言 `ceiling - envelope` 上界（268435392）及其消息。一个 daemon 线程用例驱动四个线程穿过结算的 flush 发出未结束的写入。same-group 回收用例 spawn 一个忽略 SIGTERM 的同进程组后代，它释放管道并递增一个心跳文件；该测试断言在宽限窗口的 SIGKILL 之后心跳停止：无论被杀死的后代是被回收还是作为僵尸进程滞留，这个断言都成立，因此它在 PID 1 不 wait() 孤儿进程的环境下同样成立。cross-loop 用例在主协程通过 `await asyncio.sleep` 让出时，从一个工作线程自己的 `asyncio.run` 事件循环运行一个绑定，断言该回复完成往返而不是超时。inherited-soft-limit 用例通过一个 `ulimit -S -t` 包装脚本运行解释器，将 CPU 软限制设为低于 `cpuSeconds`，并断言实际应用的 `RLIMIT_CPU` 软限制是继承来的值，而不是配置的值（用 CPU 而非地址空间，因为 macOS 忽略 `ulimit -v`）。一个配套用例继承 1 秒的 CPU 软限制，让程序捕获 SIGXCPU 并忙循环越过它，断言结算复查报告 timeout——证明复查用的是实际生效的软限制，而不是配置的 `cpuSeconds`。 +- `tests/runtime.spec.ts`：output-cap 用例断言 `ceiling - envelope` 上界（268435392）及其消息。一个 daemon 线程用例驱动四个线程穿过结算的 flush 发出未结束的写入。same-group 回收用例 spawn 一个忽略 SIGTERM 的同进程组后代，它释放管道并递增一个心跳文件；该测试断言在宽限窗口的 SIGKILL 之后心跳停止：无论被杀死的后代是被回收还是作为僵尸进程滞留，这个断言都成立，因此它在 PID 1 不 wait() 孤儿进程的环境下同样成立。一个 dispose-after-resolve 用例断言，对一个已完成、且存在同进程组存活者的运行调用 `dispose()`，只有在该存活者停止执行之后才返回（证明该运行会一直留在 `live` 中，直到它的进程组被回收）。一个 deadline 用例忙阻塞事件循环越过两个定时器，断言该存活者的心跳冻结（证明轮询的截止时间分支自身发送 SIGKILL，而不是取消尚未触发的升级）。cross-loop 用例在主协程通过 `await asyncio.sleep` 让出时，从一个工作线程自己的 `asyncio.run` 事件循环运行一个绑定，断言该回复完成往返而不是超时；一个配套用例放弃某个线程的调用，使其事件循环关闭，随后在一个后续绑定之前回答它——断言 pump 在关闭事件循环上的 `call_soon_threadsafe` 之后仍然存活（由宿主门控的顺序使其具有确定性，未修复时会把后续绑定拖到墙钟上挂起）。inherited-soft-limit 用例通过一个 `ulimit -S -t` 包装脚本运行解释器，将 CPU 软限制设为低于 `cpuSeconds`，并断言实际应用的 `RLIMIT_CPU` 软限制是继承来的值，而不是配置的值（用 CPU 而非地址空间，因为 macOS 忽略 `ulimit -v`）。一个配套用例继承 1 秒的 CPU 软限制，让程序捕获 SIGXCPU 并忙循环越过它，断言结算复查报告 timeout——证明复查用的是实际生效的软限制，而不是配置的 `cpuSeconds`。一个 control-heavy-diagnostic 用例在一个较小的 `maxValueBytes` 之下抛出一个 NUL 洪泛异常，断言序列化后的帧能放得下（证明该诊断是按序列化开销计量的）。一个 non-integer-budget 用例断言一个小数的 `maxLogBytes`／`maxValueBytes` 在加载期被拒绝。 ## Alternatives considered diff --git a/packages/code-runtime/code-runtime-python/src/index.ts b/packages/code-runtime/code-runtime-python/src/index.ts index 879df1d855..d86ab71893 100644 --- a/packages/code-runtime/code-runtime-python/src/index.ts +++ b/packages/code-runtime/code-runtime-python/src/index.ts @@ -527,7 +527,11 @@ export class PythonCodeRuntime extends CodeRuntime { /** * Dispose to quiescence: fail every in-flight run as aborted and AWAIT each - * child's exit so no subprocess outlives the fiber. + * child's exit so no subprocess that stays in the child's process group + * outlives the fiber. A descendant that escaped the group with `setsid()` / + * `start_new_session=True` is unreachable by `kill(-pid)` and is the documented + * exception (see the package README's Known Limitations); the process-group + * teardown reaps everything that stays in the group. */ private async teardown(): Promise { this.disposed = true 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 7c81c1f429..9dc6dfe382 100644 --- a/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts +++ b/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts @@ -1973,8 +1973,13 @@ describe('PythonCodeRuntime — budgets, termination, disposal', () => { program: [ 'import subprocess, sys', // Orphan in a fresh session, inheriting our stdout/stderr/fd 3, alive - // well past the close-deadline so `close` cannot fire on its own. - 'subprocess.Popen([sys.executable, "-c", "import time; time.sleep(10)"],', + // past the close-deadline so `close` cannot fire on its own. Its own + // 5 s self-exit is the leak ceiling AND the discriminator: it must stay + // ABOVE the < 4000 ms upper-bound assertion below, so if the deadline + // backstop failed to settle, settlement could only come from this + // self-exit at ~5 s and blow the bound — a sharper signal than the wall + // ceiling would give. + 'subprocess.Popen([sys.executable, "-c", "import time; time.sleep(5)"],', ' start_new_session=True)', 'return "escaped"', ].join('\n'), @@ -1986,9 +1991,9 @@ describe('PythonCodeRuntime — budgets, termination, disposal', () => { expect(result.error).toBeUndefined() expect(result.value).toBe('escaped') // Settlement waited for the backstop (graceMs + CLOSE_REAP_MARGIN_MS ≈ 2.1s), - // not the wall-clock ceiling — proving the deadline, not the ceiling, fired. + // not the orphan's 5 s self-exit — proving the deadline, not a fallback, fired. expect(elapsed).toBeGreaterThanOrEqual(1_500) - expect(elapsed).toBeLessThan(5_000) + expect(elapsed).toBeLessThan(4_000) }, 8000) it('reaps a same-group child that ignores SIGTERM and releases the pipes before close', async () => { @@ -2520,6 +2525,71 @@ describe('PythonCodeRuntime — hostile peer', () => { expect(seen).toEqual([{ from: 'thread' }]) }, 15_000) + it('keeps the reply pump alive when a late reply targets a closed thread loop', async () => { + // A binding called from a worker thread that ABANDONS the call (its + // `asyncio.run` is cancelled) leaves the pending entry holding that thread's + // loop, which `asyncio.run` closes on return. When the host later answers + // that call, `_pump_replies` schedules the completion onto the closed loop — + // `call_soon_threadsafe` raises `RuntimeError('Event loop is closed')`. + // Unguarded, that RuntimeError ends the pump task and strands every later + // reply; the guard drops the moot reply and keeps the pump serving. + // + // The ordering is a STRUCTURAL guarantee, not a timing window: the worker + // closes its loop before the main coroutine signals `closed`; the host + // answers the abandoned `slow` call (hitting the closed loop) before it + // answers `release`, because `release`'s handler only resolves `slow` first + // and then yields a microtask. So the pump provably meets the closed loop on + // `slow`'s reply before it must deliver `release`'s. Fail-before: the pump + // dies on `slow`, `release`'s reply is never read, and `await tools.release` + // hangs to the (small) maxWallMs as a timeout. + let releaseSlow!: () => void + const slowGate = new Promise((resolve) => { releaseSlow = resolve }) + const { runtime } = await setup({ maxWallMs: 6_000 }) + const result = await runtime.run({ + program: [ + 'import asyncio, threading', + 'closed = threading.Event()', + 'def worker():', + ' async def body():', + // Abandon the call: wait_for cancels it, but the pending host-side entry + // survives (dispatch does not pop on cancellation), holding this loop. + ' try:', + ' await asyncio.wait_for(tools.slow({}), timeout=0.1)', + ' except asyncio.TimeoutError:', + ' pass', + ' asyncio.run(body())', // closes the thread's loop on return + ' closed.set()', + 't = threading.Thread(target=worker)', + 't.start()', + 'while not closed.is_set():', + ' await asyncio.sleep(0.02)', + // The loop is closed. Now the host answers slow (dead-loop reply) then + // release; the pump must survive the first to deliver the second. + 'after = await tools.release({})', + 'return after', + ].join('\n'), + bindings: tools({ + slow: async () => { + // Answer only once the worker has closed its loop AND the main + // coroutine is awaiting release, so this reply reaches the pump against + // the closed loop. + await slowGate + return 'late' + }, + release: async () => { + // Let slow's reply be written first, then yield a microtask so the + // pump processes the dead-loop reply before release's own reply lands. + releaseSlow() + await new Promise(resolve => setImmediate(resolve)) + return 'released' + }, + }), + }) + expect(result.error).toBeUndefined() + // The pump survived the closed-loop reply and delivered the later binding. + expect(result.value).toBe('released') + }, 15_000) + it('round-trips an exactly representable large integer through a binding echo', async () => { // The reply serializer must print BigInt digits for a beyond-safe // integral double: String(2**60) emits a rounded form, and the child