From c56fcc8af75c2873d7afab26fd3df4411f52f6cb Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 20:53:57 -0500 Subject: [PATCH] =?UTF-8?q?docs(scripting):=20#789=20=C2=A712.6=20?= =?UTF-8?q?=E2=80=94=20review=20follow-ups=20and=20their=20windev=20/=20ma?= =?UTF-8?q?cOS=20re-run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk --- ...-09-out-of-process-script-runner-design.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/docs/plans/2026-11-09-out-of-process-script-runner-design.md b/docs/plans/2026-11-09-out-of-process-script-runner-design.md index beeadb7e3..4e6f19209 100644 --- a/docs/plans/2026-11-09-out-of-process-script-runner-design.md +++ b/docs/plans/2026-11-09-out-of-process-script-runner-design.md @@ -1352,6 +1352,34 @@ phases, base = `f109e5f2`, runs alternated base / new): and the restricted-token worker. On macOS: 104 passed, 5 skipped (109). Unit classes `BlockingPipeStreamTests`, `ProtocolTests` and `HandshakeTests`: 67 / 67 on both. +**Review follow-ups (same slice).** Blocking I/O moves two failure modes onto dedicated threads and producers: +- **Reader faults.** The host reader runs completion work inline (slot release, load bookkeeping). The frame read and + its handling are now caught separately: only the read's I/O exceptions mean the pipe ended. Anything the handling + throws kills that worker as `ProtocolFault`, so its outstanding runs answer "worker restarted" (BadResourceUnavailable) + and the shard restarts. Before, an exception escaping a dedicated thread would end the host process, and an + `InvalidOperationException` was swallowed as a clean pipe end, leaving the worker unread until a `Crash` 2 s later. + The worker's reader has the same catch-all (exit `ProtocolFault`). +- **A worker that talks but does not listen.** A full pipe blocks the producer that is draining the writer: a seam + submitting, a deploy loading scripts, a Cancel. Heartbeats keep the silence check quiet, and with only loads queued + nothing else fires. The heartbeat monitor now kills a worker whose pipe write has been blocked for longer than + `max(10 s, silence limit)` (`ScriptWorkerFrameWriter.WriteInProgressFor`, `KilledUnresponsive`). That fails the write + and releases the producer. The one known way a healthy worker's reader could wedge is closed too: `Cancel` now runs a + script's `ctx.CancellationToken` callbacks on the pool, not on the worker's reader. `StopAsync` sends `Shutdown` off + the caller, so its 1 s budget holds on a full pipe. +- `WindowsBlockingPipeStream` refuses the async surface, because `Stream`'s base versions serialise a write behind a + waiting read. +- Tests: `PipeFaultTests` covers a throwing completion callback (the worker ends `ProtocolFault`, the sibling in flight + gets BadResourceUnavailable, and the shard serves again) and a worker that stops reading while it heartbeats (killed + `KilledUnresponsive` within the bound, and the blocked deploy producer returns). The second uses the + `Hello.test_stall_reader_on_diag` seam. `BlockingPipeStreamTests` adds dispose with a write blocked on a full pipe, + and with a read and a write both pending. The protocol, handshake and pipe classes now carry `Category=ScriptWorker`. + Mutation checks: with no stall kill the stall test is red (timeout); with a catch-all that does not fault, the + callback test is red (`Crash`). +- Re-run: `Category=ScriptWorker` on windev: 179 passed, 2 skipped by design (181). On macOS: 176 passed, 5 skipped + (181). windev perf after the fixes (alternated new / base / new): 10k/s vs InProcess +55 / +68 (base +95 in the same + session); idle p50 391 / 365 µs (base 517); closed loop 51k / 54k evals/s (base 57k). No regression against the + figures above. + **Still missed:** Windows host CPU vs InProcess (+53 to +78 against ≤ 25) and Windows idle p50 (363–386 µs against ≤ 150 µs; InProcess alone is about 105 µs on this VM). Next candidates: drop the dispatcher wake when the caller's continuation is known to be cheap (needs a seam-level guarantee, the reason it exists is a slow consumer), or measure