mirror of
https://github.com/deepseek-ai/deepseek-harness.git
synced 2026-09-11 04:00:38 +00:00
fix(code-runtime-python): clear stray buffers on truncation and drain the original std streams
Addresses the review's two carried warnings and the comment suggestion: - Once the ledger truncates, every arm that marks it (admit()'s two ceilings and the child-marker frame arm) now clears both stray pipes' buffered output wholesale, so the end-path flushStray sees empty buffers instead of concat+decoding doomed data near a 256 MiB maxLogBytes; captureStray's newline loop re-checks the flag before re-retaining the residual. - The child runs with -u (unbuffered), so sys.__stdout__/sys.__stderr__ writes are visible to stray capture immediately; the settlement flush still drains the original std streams before the done frame as a guard. A regression test writes through sys.__stdout__/sys.__stderr__ without an explicit flush and asserts both bytes land in logs. C-ext stdio remains an accepted residual, recorded in the README Known Limitations (en + zh). - The ledger-comment arithmetic now states the exact boundary (serializes to exactly maxLogBytes; without the reserved byte it would be maxLogBytes + 1) in both host and child. Note (en + zh) registers the stray-clear and -u/settlement-drain mechanisms and the new test; pairings re-recorded; corpus passes 1029.
This commit is contained in:
@@ -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 packages/code-runtime/code-runtime-python/README.md
|
||||
README.md: fbb37d36523d12357a30c319409afc69ce64e487
|
||||
README.zh.md: 10141bc634e523b1fa079280039b0e5eda7e45d8
|
||||
README.md: 25d56e1e611a04523d933a16784e51a91d689fa3
|
||||
README.zh.md: 978a875c815d33dcfadfd5114ee8b88dbde4fc92
|
||||
|
||||
@@ -37,6 +37,8 @@ No direct invalidation; the named consumer owns any request-prefix changes.
|
||||
- **The cross-language guard covers executed values and frame field sets, not field types** — `tests/protocol-mirror.e2e.ts` compares `PROTOCOL_FD`, the log truncation marker, and each `TypedDict`'s required and optional fields against a real `python3`. Comparing field types across TypeScript and Python has no mechanical equivalent here, so review plus the backend's real-subprocess suite owns type-level drift.
|
||||
- **`RLIMIT_AS` is not enforced on macOS** — the dyld shared cache mapped into every process at exec exceeds any practical address-space cap, and the kernel rejects the `setrlimit` call, so `addressSpaceMb` is skipped there. `cpuSeconds` and `maxWallMs` still bound every run.
|
||||
- **PID-reuse protection is inert on macOS** — `readProcessStart` reads `/proc/<pid>/stat`, which Darwin does not provide, so the identity re-check that guards `killGroup` against signalling a recycled pgid always passes there; `killGroup` signals the pgid without the identity re-check on macOS rather than paying a `ps` fork on a teardown path. The process-group teardown and the `closeDeadline` bound still contain the run.
|
||||
- **C-ext stdio buffers are not drained at settlement.** The child runs with `-u` (unbuffered), so `sys.__stdout__`/`sys.__stderr__` and `os.write` bytes are visible to the host's stray capture immediately; but a C extension's private C-stdio (`FILE*`) buffering is outside the interpreter, and its unwritten bytes are lost when the host SIGTERMs the child after the done frame. Model code should flush C-level stdio explicitly before returning if it must survive.
|
||||
|
||||
- **A truncated log's serialized array runs to `maxLogBytes` plus the marker.** The truncation marker is envelope, not payload — it rides uncharged so it can always be emitted — and the outer-array envelope is reserved one byte in the ledger. A truncated run with admitted entries therefore serializes its `logs` array to at most `maxLogBytes + marker + 1`; the marker alone fits any admissible budget (the 64-byte floor guarantees it).
|
||||
- **A descendant that calls `setsid()` / `start_new_session=True` escapes teardown.** Termination signals the child's process group with `kill(-pid)`; a descendant that moves itself into a fresh session is no longer in that group and no signal reaches it. If it also releases the inherited stdout/stderr/fd-3 pipes, the leader's `close` still settles the run, and after the `closeDeadline` bound the fiber goes quiescent while that orphan keeps running. This is the containment boundary, not a security one — model code has bash-equivalent trust, and a bash tool can `setsid` away just the same. Reaching such an orphan would require tracking every descendant pid (as the bash-local backend's process-inspector does) and is deferred; the process-group teardown reaps everything that stays in the group.
|
||||
- **A combined log-and-value peak is not modelled by the load gate.** Each budget is checked against `addressSpaceMb` on its own. A model daemon thread that keeps writing while the completion value is metered and framed can refill the log pending toward `maxLogBytes` during that window, so the two peaks add in a way no gate admits or rejects. A gate over `(maxLogBytes + maxValueBytes)` was considered and deferred: its discriminating case cannot be scheduled deterministically under `RLIMIT_AS`, so the gate would only prove its own arithmetic. When the combined peak is reached the run dies as `worker-exit` -- containment holds and only the failure classification is degraded.
|
||||
|
||||
@@ -37,6 +37,8 @@ host 与 CPython 子进程在子进程的 fd 3 上交换一个无版本号的 JS
|
||||
- **跨语言 guard 覆盖执行值与帧字段集,但不覆盖字段类型** —— `tests/protocol-mirror.e2e.ts` 使用真实 `python3` 比较 `PROTOCOL_FD`、日志截断标记,以及每个 `TypedDict` 的必填和可选字段。跨 TypeScript 与 Python 比较字段类型在此没有机械等价物,因此类型级漂移由 review 加后端真子进程套件负责。
|
||||
- **`RLIMIT_AS` 在 macOS 上不施加** —— 在 exec 时映射进每个进程的 dyld 共享缓存超过任何实际的地址空间上限,内核会拒绝该 `setrlimit` 调用,故 `addressSpaceMb` 在那里被跳过。`cpuSeconds` 与 `maxWallMs` 仍约束每一次运行。
|
||||
- **PID 复用防护在 macOS 上失效** —— `readProcessStart` 读取 `/proc/<pid>/stat`,Darwin 不提供它,因此防止 `killGroup` 对已回收的 pgid 发信号的同一性复检在那里恒通过;`killGroup` 在 macOS 上不经同一性复检直接对 pgid 发信号,而非在拆卸路径上付出一次 `ps` fork。进程组拆卸与 `closeDeadline` 上界仍约束该次运行。
|
||||
- **C 扩展的 stdio 缓冲在结算时不被排空。** 子进程以 `-u`(无缓冲)运行,因此 `sys.__stdout__`/`sys.__stderr__` 与 `os.write` 的字节立即可见;但 C 扩展私有的 C-stdio(`FILE*`)缓冲在解释器之外,其未写出的字节会在宿主于 done 帧后 SIGTERM 子进程时丢失。模型代码若需保留,应在返回前显式 flush C 层 stdio。
|
||||
|
||||
- **截断日志的序列化数组会到 `maxLogBytes` 加标记为止。** 截断标记是 envelope 而非 payload——它不计费地随行,因此总能发出——而外层数组外壳在账本中预留了一字节。因此带已放行条目的截断运行,其 `logs` 数组序列化后至多为 `maxLogBytes + marker + 1`;标记单独能放进任何可接受的预算(64 字节下限保证这一点)。
|
||||
- **调用 `setsid()` / `start_new_session=True` 的后代会逃出 teardown。** 终止是用 `kill(-pid)` 向子进程的进程组发信号;一个把自己移入新会话的后代已不在该进程组内,任何信号都到不了它。若它同时释放了继承而来的 stdout/stderr/fd-3 管道,leader 的 `close` 仍会结算该次运行,在 `closeDeadline` 到界之后 fiber 变为完全停稳,而那个孤儿仍在运行。这是 containment 边界,而非安全边界——模型代码具有等同 bash 的信任级别,一个 bash 工具同样能 `setsid` 逃逸。要够到这样的孤儿需要追踪每一个后代 pid(如 bash-local 后端的 process-inspector 所做),此项已推迟;进程组 teardown 会回收所有留在组内的进程。
|
||||
- **日志与完成值的叠加峰值未被加载门建模。** 每项预算都是各自对照 `addressSpaceMb` 检查的。模型的 daemon 线程可以在完成值被计量并分帧的窗口内持续写入、把日志 pending 重填到接近 `maxLogBytes`,于是两个峰值以任何门都不曾放行也不曾拒绝的方式相加。对 `(maxLogBytes + maxValueBytes)` 设门的方案经评估后推迟:它的判别用例无法在 `RLIMIT_AS` 之下确定性地构造出来,因此该门只能证明自己的算术。叠加峰值被触及时该次运行死为 `worker-exit`——containment 仍然成立,只是失败分类失真。
|
||||
|
||||
@@ -102,7 +102,8 @@ class LogBuffer:
|
||||
# JSON-string cost plus one separator byte, and the serialized outer
|
||||
# logs array adds one more byte of envelope (two brackets and n-1 commas
|
||||
# over n entries' separators), so a result that exactly exhausts the
|
||||
# ledger would serialize to max_bytes + 1. Reserving that byte keeps an
|
||||
# ledger serializes to exactly max_bytes; WITHOUT the reserved byte it
|
||||
# would serialize to max_bytes + 1. Reserving that byte keeps an
|
||||
# admitted result within the configured cap; the truncation-marker entry
|
||||
# is envelope, not payload, and rides uncharged (``_max_bytes`` stays the
|
||||
# configured value for the marker's message text).
|
||||
@@ -947,6 +948,18 @@ async def _run(channel: ProtocolChannel) -> None:
|
||||
sys.stdout = _LogStream(logs) # type: ignore[assignment]
|
||||
sys.stderr = _LogStream(logs) # type: ignore[assignment]
|
||||
out_stream, err_stream = sys.stdout, sys.stderr
|
||||
# The ORIGINAL std streams are bound here, before the program runs, so the
|
||||
# settlement flush can drain bytes a program wrote through them without an
|
||||
# explicit flush. The bootstrap only replaces `sys.stdout`/`sys.stderr` with
|
||||
# the `_LogStream`; `sys.__stdout__`/`sys.__stderr__` (and C-ext stdio
|
||||
# layered on the same fds) are untouched, and their block-buffered bytes are
|
||||
# lost when the host SIGTERMs the child right after the done frame — the
|
||||
# default SIGTERM disposition terminates without interpreter finalization.
|
||||
# Binding the names here (before the program) makes them immune to a
|
||||
# `sys.__stdout__ = boom` rebind in model code; `None` under `-S`-style
|
||||
# redirects is guarded at flush time.
|
||||
_stdout_orig = sys.__stdout__
|
||||
_stderr_orig = sys.__stderr__
|
||||
|
||||
# 6. Compile the program as the body of an async function, matching the
|
||||
# seam contract (`CodeRunRequest.program` is an async-function body: top-level
|
||||
@@ -1123,7 +1136,9 @@ async def _run(channel: ProtocolChannel) -> None:
|
||||
# already pushed still reports the truncation. Same rule as
|
||||
# `_make_failure_reporter`: a settled verdict must not be swallowed by the
|
||||
# reporting that follows it.
|
||||
for _flush in (flush_out, flush_err):
|
||||
for _flush in (flush_out, flush_err, _stdout_orig, _stderr_orig):
|
||||
if _flush is None:
|
||||
continue
|
||||
try:
|
||||
_flush()
|
||||
except _BaseException: # noqa: BLE001 -- swallow ONLY the log tail; `done` must reach the host; `_BaseException` is a pre-program local
|
||||
|
||||
@@ -967,7 +967,15 @@ export class PythonCodeRuntime extends CodeRuntime {
|
||||
let child: ChildProcessWithoutNullStreams
|
||||
let proto: Duplex | null
|
||||
try {
|
||||
child = spawn(resolvePythonBin(this.config.pythonBin), ['-I', bootstrapPath], {
|
||||
// `-u` keeps the interpreter's own stdout/stderr UNBUFFERED: a program
|
||||
// that writes through `sys.__stdout__`/`sys.__stderr__` (or C-stdio
|
||||
// layered on the same fds) must have those bytes visible to the host's
|
||||
// stray capture immediately — a block-buffered wrapper would otherwise
|
||||
// hold them until an explicit flush, and the host SIGTERMs the child
|
||||
// right after the done frame, before any finalization-time flush could
|
||||
// run. The `_LogStream` replacement of `sys.stdout`/`sys.stderr` is
|
||||
// unaffected (it is a Python object, not the C-level stdio buffer).
|
||||
child = spawn(resolvePythonBin(this.config.pythonBin), ['-u', '-I', bootstrapPath], {
|
||||
env: {},
|
||||
detached: true, // Own process group — kill(-pid, sig) reaches subprocesses the model program spawns.
|
||||
stdio: ['pipe', 'pipe', 'pipe', 'pipe'],
|
||||
@@ -1000,12 +1008,25 @@ export class PythonCodeRuntime extends CodeRuntime {
|
||||
// The ledger starts one byte below maxLogBytes: each entry is charged its
|
||||
// JSON-string cost plus one separator byte, and the serialized outer logs
|
||||
// array adds one more byte of envelope (two brackets and n-1 commas over n
|
||||
// entries' separators), so a result that exactly exhausts the ledger would
|
||||
// entries' separators), so a result that exactly exhausts the ledger
|
||||
// serializes to exactly maxLogBytes; WITHOUT the reserved byte it would
|
||||
// serialize to maxLogBytes + 1. Reserving that byte keeps an admitted
|
||||
// result within the configured cap; the truncation-marker entry is
|
||||
// envelope, not payload, and rides uncharged.
|
||||
let logBudget = this.config.maxLogBytes - 1
|
||||
let logsTruncated = false
|
||||
// Drop a pipe's buffered stray output wholesale: once the ledger has
|
||||
// truncated, every byte of it would be no-op'd by admit(), so retaining
|
||||
// it (and later Buffer.concat+decoding it in flushStray) would spend host
|
||||
// memory on output that can never be admitted. Called from every arm that
|
||||
// marks the ledger truncated — admit()'s two ceilings and the child-marker
|
||||
// frame arm — so the end-path flushStray sees empty buffers and exits.
|
||||
const clearStray = (stray: StrayBuffer): void => {
|
||||
stray.chunks = []
|
||||
stray.blocks = []
|
||||
stray.cost = 0
|
||||
stray.utf8 = { expected: 0, width: 0, lowerFirst: 0, upperFirst: 0 }
|
||||
}
|
||||
const admit = (text: string): void => {
|
||||
// Post-truncation admits are no-ops: once the ledger has truncated, the
|
||||
// marker is the last entry. Reachable within one `data` callback — a
|
||||
@@ -1036,6 +1057,10 @@ export class PythonCodeRuntime extends CodeRuntime {
|
||||
if (text.length + 3 > logBudget) {
|
||||
logsTruncated = true
|
||||
logs.push(logTruncationMarker(this.config.maxLogBytes))
|
||||
// Release the buffered stray pipes: their bytes can never be
|
||||
// admitted now (see clearStray).
|
||||
clearStray(strayOut)
|
||||
clearStray(strayErr)
|
||||
return
|
||||
}
|
||||
// Past the lower bound, measure the exact serialized cost without
|
||||
@@ -1046,6 +1071,8 @@ export class PythonCodeRuntime extends CodeRuntime {
|
||||
if (measured === undefined) {
|
||||
logsTruncated = true
|
||||
logs.push(logTruncationMarker(this.config.maxLogBytes))
|
||||
clearStray(strayOut)
|
||||
clearStray(strayErr)
|
||||
return
|
||||
}
|
||||
logBudget -= measured + 1
|
||||
@@ -1113,6 +1140,11 @@ export class PythonCodeRuntime extends CodeRuntime {
|
||||
// The residual begins at a character boundary (a newline is never
|
||||
// inside a multibyte sequence), so its cost and UTF-8 state recompute
|
||||
// cleanly from a fresh walk.
|
||||
// A line admitted inside the loop may have exhausted the ledger and
|
||||
// cleared this pipe (see clearStray); the re-retain below must not
|
||||
// resurrect the doomed residual.
|
||||
// oxlint-disable-next-line typescript/no-unnecessary-condition -- admit() (a closure) sets it.
|
||||
if (logsTruncated) return
|
||||
stray.chunks = detachResidual(buffered)
|
||||
stray.utf8 = { expected: 0, width: 0, lowerFirst: 0, upperFirst: 0 }
|
||||
stray.cost = accrueStrayCost(buffered, stray.utf8)
|
||||
@@ -1370,6 +1402,8 @@ export class PythonCodeRuntime extends CodeRuntime {
|
||||
// describes the run.
|
||||
if (!logsTruncated) {
|
||||
logsTruncated = true
|
||||
clearStray(strayOut)
|
||||
clearStray(strayErr)
|
||||
// The host's OWN marker, never the frame's text. `truncated` is
|
||||
// attacker-reachable, so trusting the text let a program write
|
||||
// `{"type":"log","truncated":true,"text":<1 MiB>}` and land all
|
||||
|
||||
@@ -4304,6 +4304,31 @@ describe('PythonCodeRuntime — hostile peer', () => {
|
||||
expect(result.logs.join('')).toContain('stray stderr')
|
||||
})
|
||||
|
||||
it('flushes bytes written through sys.__stdout__/sys.__stderr__ before the done frame', async () => {
|
||||
// The bootstrap only replaces sys.stdout/sys.stderr with the _LogStream;
|
||||
// sys.__stdout__/sys.__stderr__ are the original block-buffered wrappers
|
||||
// over fd 1/2. A program that writes through them without an explicit flush
|
||||
// would lose those bytes when the host SIGTERMs the child right after the
|
||||
// done frame (the default SIGTERM disposition terminates without
|
||||
// interpreter finalization). The settlement flush now drains the original
|
||||
// std streams before sending the done frame, so the bytes land in the
|
||||
// kernel pipe buffer and the host's stray capture records them.
|
||||
const { runtime } = await setup()
|
||||
const result = await runtime.run({
|
||||
program: [
|
||||
'import sys',
|
||||
'sys.__stdout__.write("orig stdout\\n")',
|
||||
'sys.__stderr__.write("orig stderr\\n")',
|
||||
'return "done"',
|
||||
].join('\n'),
|
||||
bindings: [],
|
||||
})
|
||||
expect(result.error).toBeUndefined()
|
||||
expect(result.value).toBe('done')
|
||||
expect(result.logs.join('')).toContain('orig stdout')
|
||||
expect(result.logs.join('')).toContain('orig stderr')
|
||||
}, 15_000)
|
||||
|
||||
it('escalates to SIGKILL when the program traps SIGTERM and ignores the grace period', async () => {
|
||||
// A program that traps SIGTERM should still die: the kill() escalation
|
||||
// fires SIGKILL after graceMs. The full run reports either timeout (wall)
|
||||
|
||||
Reference in New Issue
Block a user