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 }