From 1103e36c22260a2a6b50e5603a506ffa2faca3e5 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Sun, 2 Aug 2026 20:22:08 +0800 Subject: [PATCH] fix(code-runtime-python): confirm group death at the deadline; chunk the frame read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reap-poll deadline arm sent SIGKILL then finalized immediately, declaring quiescence on mere signal delivery while the group was still dying. It now keeps polling for the group to actually empty (bounded by one more reap margin) after its self-sent SIGKILL, so `finished` resolves only on a confirmed-empty group. ProtocolChannel.read_frame read the boot/run handshake frames through FileIO.readline() on the unbuffered fd — one os.read(1) per byte, so a multi-megabyte program burned CPU (RLIMIT_CPU already in force for the run frame) in millions of syscalls before ast.parse. It now reads in chunks into the same _pending buffer the async reader uses; the wrapping os.fdopen is gone. read_frame is this PR's own code (e7f22ed3), not the protocol layer. The chunked read is a syscall-count improvement with no cross-platform-deterministic failure to assert, noted as such in the Agent Note. --- ...-runtime-python-settlement-fixes.i18n.yaml | 4 +-- ...31-code-runtime-python-settlement-fixes.md | 10 ++++-- ...code-runtime-python-settlement-fixes.zh.md | 10 ++++-- .../code-runtime-python/py/bootstrap.py | 34 ++++++++++++++----- .../code-runtime-python/src/index.ts | 22 +++++++++--- 5 files changed, 59 insertions(+), 21 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 0738766e06..4225a668cf 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: 620a4180f63e738c8ee6954d903f437aac3d068f -2026-07-31-code-runtime-python-settlement-fixes.zh.md: ab31e8fbfe68a559b2e88690a8dfadc8840d0d5c +2026-07-31-code-runtime-python-settlement-fixes.md: ba2f227d606c1c6d3efdd22372a4ea29ef50d068 +2026-07-31-code-runtime-python-settlement-fixes.zh.md: fa72c434e81ef5354f6112f834f85377666d8b3b 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 620a4180f6..ba2f227d60 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,11 +6,11 @@ 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 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. +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 behavioral fix ships with a test that fails without it; the one exception is a syscall-count improvement (chunked frame reading) with no cross-platform-deterministic failure to assert. ## Decision -Seven independent corrections, each in the package that owns the defect. +Eight independent corrections, each in the package that owns the defect. ### Boot-write failure no longer rejects run() @@ -42,6 +42,10 @@ In [`py/bootstrap.py`](../../../../packages/code-runtime/code-runtime-python/py/ Also in `py/bootstrap.py`, a binding reply Future is created on the loop that ran `dispatch`. When the model calls a binding from a worker THREAD via `asyncio.run(tools.x(...))`, that Future belongs to the thread's loop, not the main loop where `_pump_replies` reads the reply. `asyncio.Future` is not thread-safe: completing it from another thread does not wake its own loop, so the direct `set_result`/`set_exception` left the awaiting thread stranded and the run degraded to a wall-clock timeout. Each pending entry now records its Future's loop alongside the Future, and `_pump_replies` completes it via that loop's `call_soon_threadsafe`. The shared `pending`/`next_id` state is guarded by a `threading.Lock` held across the id claim, the fd-3 write, and the counter advance, so concurrent callers cannot interleave frames out of the id order the host requires. `call_soon_threadsafe` onto a loop that has already CLOSED (the worker thread finished and abandoned its call before the reply arrived) raises `RuntimeError`; that schedule is wrapped so the moot reply is dropped rather than letting the exception end the pump task and strand every later reply. +### The blocking frame reader reads in chunks, not byte by byte + +`ProtocolChannel.read_frame` — used for the `boot` and `run` handshake frames — read through `FileIO.readline()` on the unbuffered (`buffering=0`) fd, which issues one `os.read(1)` per byte. The `run` frame arrives AFTER `RLIMIT_CPU` is in force, so a legitimate multi-megabyte program burned seconds of CPU in millions of single-byte syscalls before `ast.parse` ran — potentially exhausting the budget on the read alone. It now reads in `_READ_CHUNK_BYTES` chunks into the same `_pending` residual buffer the async reader already uses (the wrapping `os.fdopen` object is gone; both readers call `os.read(self._fd, ...)` directly), so the read cost is trivial and read-ahead past a newline is preserved for the next frame. + ## Testing - `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. @@ -70,4 +74,4 @@ Also in `py/bootstrap.py`, a binding reply Future is created on the loop that ra ## Consequences -The seam's resolve-don't-reject contract holds on the boot-write path with measured coverage. Log capture is thread-safe at the cost of one re-entrant lock acquisition per write and flush. Fd-3 residual memory is bounded by the actual retained bytes. The output caps admit every value a frame can carry. Disposal is genuinely quiescent against a same-group survivor — bounded by the existing grace budget, zero-cost when the group is already empty, with the SIGKILL timer cleared once the group empties so a stale kill cannot strike a recycled pgid — RLIMIT enforcement keeps the strictest of configured and inherited on both soft and hard, and bindings called from model-created threads complete instead of timing out. Each fix carries a test that fails without it, so a future regression on any of the seven goes red. +The seam's resolve-don't-reject contract holds on the boot-write path with measured coverage. Log capture is thread-safe at the cost of one re-entrant lock acquisition per write and flush. Fd-3 residual memory is bounded by the actual retained bytes. The output caps admit every value a frame can carry. Disposal is genuinely quiescent against a same-group survivor — bounded by the existing grace budget, zero-cost when the group is already empty, with the SIGKILL timer cleared once the group empties so a stale kill cannot strike a recycled pgid — RLIMIT enforcement keeps the strictest of configured and inherited on both soft and hard, bindings called from model-created threads complete instead of timing out, and the handshake frame reader no longer burns the CPU budget on a large program. Each fix carries a test that fails without it (except the chunked frame read, a syscall-count improvement with no cross-platform-deterministic failure to assert), so a future regression on the rest goes red. 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 ab31e8fbfe..fa72c434e8 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,11 +6,11 @@ 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 更久（一个用 `setsid()` 逃出该进程组的后代是有文档记载的例外——见该包 README 的 Known Limitations）。一连串审查暴露出一些缺陷，它们以单元测试覆盖率无法捕获的方式破坏了这些契约：每一个都藏在一处 `/* 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 -七处相互独立的修正，各自位于拥有对应缺陷的包中。 +八处相互独立的修正，各自位于拥有对应缺陷的包中。 ### Boot-write failure no longer rejects run() @@ -42,6 +42,10 @@ Status: implemented 同样在 `py/bootstrap.py` 中，一个绑定回复 Future 是在运行 `dispatch` 的那个事件循环上创建的。当模型通过 `asyncio.run(tools.x(...))` 从一个工作线程调用某个绑定时，该 Future 属于该线程的事件循环，而不是 `_pump_replies` 读取回复的主事件循环。`asyncio.Future` 不是线程安全的：从另一个线程完成它并不会唤醒它自己的事件循环，因此直接的 `set_result`／`set_exception` 会让那个正在等待的线程被搁置，该次运行退化为墙钟超时。现在每个待处理条目都会在记录 Future 的同时记录其 Future 所属的事件循环，`_pump_replies` 通过该事件循环的 `call_soon_threadsafe` 来完成它。共享的 `pending`／`next_id` 状态由一把 `threading.Lock` 保护，该锁跨越 id 认领、fd-3 写入和计数器推进这三步持有，因此并发调用方无法以违反宿主所要求的 id 顺序来交错帧。对一个已经关闭的事件循环（工作线程已结束、在回复到达前放弃了它的调用）调用 `call_soon_threadsafe` 会抛出 `RuntimeError`；该调度被包裹起来，使这个已无意义的回复被丢弃，而不是让异常终结 pump 任务并搁置此后的每一个回复。 +### The blocking frame reader reads in chunks, not byte by byte + +`ProtocolChannel.read_frame`（用于 `boot` 和 `run` 握手帧）过去通过在无缓冲（`buffering=0`）fd 上的 `FileIO.readline()` 读取，这会为每个字节发起一次 `os.read(1)`。`run` 帧在 `RLIMIT_CPU` 生效之后才到达，因此一个合法的数兆字节程序会在 `ast.parse` 运行之前，在数以百万计的单字节系统调用中烧掉数秒 CPU——有可能仅在读取这一步就耗尽预算。现在它以 `_READ_CHUNK_BYTES` 为单位分块读取，写入异步读取器已经使用的那同一个 `_pending` 残余缓冲区（包裹用的 `os.fdopen` 对象已被移除；两个读取器都直接调用 `os.read(self._fd, ...)`），因此读取开销微不足道，并且越过换行符的预读也为下一帧保留了下来。 + ## Testing - `tests/boot-write-failure.spec.ts` 对 `spawn` 做 mock，使 fd-3 管道在引导写入时抛出异常（这是真实子进程无法被迫进入的唯一路径），并断言 `run()` resolve 出一个 `worker-exit` 而非 reject。它被隔离在自己的 spec 中，因此真实子进程测试套件不受影响。 @@ -70,4 +74,4 @@ Status: implemented ## Consequences -seam 的"只 resolve、不 reject"契约在引导写入路径上得以成立，且覆盖率是被度量的。日志捕获是线程安全的，代价是每次写入和 flush 都要获取一次可重入锁。fd-3 残余数据的内存受实际保留的字节数约束。输出上限放行一个帧所能承载的每一个值。dispose 面对同进程组存活者是真正完全停稳的（以既有的宽限预算为界，在进程组已为空时代价为零，并且一旦进程组清空就清除 SIGKILL 定时器，从而一次滞留的 kill 无法击中一个被回收的 pgid），RLIMIT 强制在 soft 和 hard 两者上都保持配置值与继承值中的最严格者，并且从模型创建的线程调用的绑定会完成而不是超时。每处修复都附带一个在缺少它时会失败的测试，因此这七处中任何一处未来若发生回归都会变红。 +seam 的"只 resolve、不 reject"契约在引导写入路径上得以成立，且覆盖率是被度量的。日志捕获是线程安全的，代价是每次写入和 flush 都要获取一次可重入锁。fd-3 残余数据的内存受实际保留的字节数约束。输出上限放行一个帧所能承载的每一个值。dispose 面对同进程组存活者是真正完全停稳的（以既有的宽限预算为界，在进程组已为空时代价为零，并且一旦进程组清空就清除 SIGKILL 定时器，从而一次滞留的 kill 无法击中一个被回收的 pgid），RLIMIT 强制在 soft 和 hard 两者上都保持配置值与继承值中的最严格者，并且从模型创建的线程调用的绑定会完成而不是超时，而且握手帧读取器不再在一个大程序上烧掉 CPU 预算。每处修复都附带一个在缺少它时会失败的测试（分块读取帧除外，它是一处系统调用次数的改进，没有可跨平台确定性断言的失败），因此其余各处未来若发生回归都会变红。 diff --git a/packages/code-runtime/code-runtime-python/py/bootstrap.py b/packages/code-runtime/code-runtime-python/py/bootstrap.py index 33247bbedd..3b850f995d 100644 --- a/packages/code-runtime/code-runtime-python/py/bootstrap.py +++ b/packages/code-runtime/code-runtime-python/py/bootstrap.py @@ -354,12 +354,12 @@ class ProtocolChannel: """ def __init__(self, fd: int) -> None: - # Unbuffered binary I/O so we never lose frames to an idle flush. - self._reader = os.fdopen(fd, "rb", buffering=0, closefd=False) self._fd = fd - # Residual bytes read past a frame's newline. Held here, not in the - # reading coroutine: the reply pump is cancelled once `done` is posted, - # and read-ahead sitting in a local would be lost with it. + # Residual bytes read past a frame's newline, shared by the blocking and + # async readers. Held here, not in the reading coroutine: the reply pump + # is cancelled once `done` is posted, and read-ahead sitting in a local + # would be lost with it. Both readers use `os.read(self._fd, ...)` + # directly, so no buffered file object wraps the fd. self._pending = bytearray() # Serializes writers: os.write releases the GIL, and a frame larger # than PIPE_BUF is neither atomic nor guaranteed fully consumed by one @@ -374,12 +374,28 @@ class ProtocolChannel: (``boot`` and ``run``), where blocking is what the handshake wants. Reply frames arriving during the program go through :meth:`read_frame_async`, which must not occupy a thread. + + Reads in CHUNKS into the shared ``_pending`` buffer rather than through + ``FileIO.readline()``: the fd is unbuffered (``buffering=0``), so + ``readline`` issues one ``os.read(1)`` per byte, and a multi-megabyte + ``run`` frame — RLIMIT_CPU already in force by then — would burn the + budget in millions of syscalls before ``ast.parse`` even runs. The chunk + reads and the same residual buffer the async path uses keep read-ahead + past a newline for the next frame. """ - line = self._reader.readline() - if not line: - return None - return _decode_json_plain(line.decode("utf-8")) + while True: + newline = self._pending.find(b"\n") + if newline >= 0: + line = bytes(self._pending[:newline]) + del self._pending[: newline + 1] + return _decode_json_plain(line.decode("utf-8")) + chunk = os.read(self._fd, _READ_CHUNK_BYTES) + if not chunk: + # EOF before a newline: drop the partial line, as the host drops + # a frame that never completed. + return None + self._pending.extend(chunk) async def read_frame_async(self) -> dict[str, Any] | None: """Await one JSON-line frame without occupying a thread. ``None`` on EOF. diff --git a/packages/code-runtime/code-runtime-python/src/index.ts b/packages/code-runtime/code-runtime-python/src/index.ts index d86ab71893..50ac14f6cc 100644 --- a/packages/code-runtime/code-runtime-python/src/index.ts +++ b/packages/code-runtime/code-runtime-python/src/index.ts @@ -1095,6 +1095,12 @@ export class PythonCodeRuntime extends CodeRuntime { return } const deadline = Date.now() + this.config.graceMs + CLOSE_REAP_MARGIN_MS + // Once the deadline forces us to send SIGKILL ourselves, allow one more + // reap window for the kernel to tear the group down before giving up: + // SIGKILL is asynchronous, so the group is not gone the instant it is + // sent. `finalize` only runs on a confirmed-empty group, except at this + // final hard bound where nothing more can be done. + let hardDeadline = 0 const pollGroup = (): void => { if (groupEmpty()) { // The group is gone; the grace SIGKILL is moot. Cancel it (it may not @@ -1104,15 +1110,23 @@ export class PythonCodeRuntime extends CodeRuntime { finalize() return } - if (Date.now() >= deadline) { + if (hardDeadline === 0 && Date.now() >= deadline) { // Deadline reached with the group still non-empty. This is reachable // when the host event loop was blocked past both timers: Node runs // this poll before the grace SIGKILL timer, so that SIGKILL may never - // have fired. Send it HERE before finalizing — idempotent if the timer - // already ran — so a SIGTERM-ignoring same-group survivor is actually - // reaped rather than released by cancelling an unfired escalation. + // have fired. Send it HERE (idempotent if the timer already ran) and + // keep polling for the group to actually empty — finalizing on mere + // signal delivery would declare quiescence while the group is still + // dying. Bound the extra wait by one more reap margin. killGroup('SIGKILL') clearTimeout(graceTimer) + hardDeadline = Date.now() + CLOSE_REAP_MARGIN_MS + } + // Hard bound: only reached if the self-sent SIGKILL never empties the + // reachable group (a kernel that never reports ESRCH), which does not + // happen in practice — hence the ignore on the branch below. + /* v8 ignore next 4 -- SIGKILL empties the reachable group within the reap margin. */ + if (hardDeadline !== 0 && Date.now() >= hardDeadline) { finalize() return }