The process demux in packages/core/src/sandbox/harness/demux.ts buffers a sink nobody reads without bound. drainUntil serves one stream by reading the shared source, and enqueues the other stream's chunks as a side effect — so a caller that reads one stream and neither reads nor cancel()s the other accumulates the whole of that other stream in memory. A large sandbox log can exhaust the worker.
Raised by cubic on #7 and deliberately not fixed there: a correct fix needs a decision this codebase has not made. The code carries a standing hazard note recording the same thing.
What was tried, and what it measured
A per-sink occupancy cap (8 MiB) was implemented and then withdrawn. Two defects, both measured by instrumenting the demux to record peak per-sink bytes across eight interleavings with 1 MiB chunks, both streams read to completion.
The accounting was inflated. pull does not re-fire per chunk, so a running total keeps charging chunks the consumer has already read. Deriving occupancy from 1 - controller.desiredSize plus a mirrored FIFO of chunk sizes corrected it — the 2:1 interleaving's peak fell from 3.0 MiB to 1.0 MiB.
With the accounting corrected, the overshoot was still real. A consumer lagging one macrotask per read peaks at 9 MiB and gets errored — on a stream it is reading to completion. No sink applies backpressure to the shared reader, so a live-but-slower consumer falls behind without bound, and any fixed occupancy cap must eventually fire on a working stream.
The obvious escape does not work either. Erroring only a sink whose consumer has taken nothing fails unfixably: microtasks starve macrotasks completely — measured at 100,000 microtask turns before a single setTimeout(0) fired — so a consumer on a macrotask cadence takes nothing while the drain runs, and "took nothing" cannot distinguish a slow consumer from an absent one.
Why the naive fixes are wrong
Honoring the other sink's desiredSize reintroduces the hang the current design exists to avoid: one shared reader feeds both sinks, and drainUntil enqueues the other source's events on purpose so a consumer reading a single stream to completion does not deadlock.
| Option |
Cost |
| Block on the other sink's demand |
Hangs any single-stream consumer |
| Drop past a cap with a truncation marker |
The harness byte streams have no representation for one — the same reason truncated events are already dropped |
| Error past a cap |
Withdrawn above: fires on working streams |
What closing this actually requires
One of:
- A contract obligation on callers — every consumer must read both streams or
cancel() the one it does not need. That makes pausing the drain legitimate rather than a hang, and backpressure becomes implementable. It is a public-contract change, which is why it is not a review-fix decision.
- A spill destination that is not the heap — bound memory without dropping data or erroring.
Exposure today
Narrow. run reads both streams concurrently via Promise.all, so it never accumulates. Only a spawn caller that consumes one stream and abandons the other without cancelling is affected.
Notes
src/sandbox/harness/** is vendored from chatbot-pf/pleaseworks and relicensed to Apache-2.0, so whichever way this is decided is also a divergence decision.- Two lagging-consumer tests in
test/sandbox/harness/process.demux.test.ts pin the property the cap broke — any future guard has to keep them green.
The process demux in
packages/core/src/sandbox/harness/demux.tsbuffers a sink nobody reads without bound.drainUntilserves one stream by reading the shared source, and enqueues the other stream's chunks as a side effect — so a caller that reads one stream and neither reads norcancel()s the other accumulates the whole of that other stream in memory. A large sandbox log can exhaust the worker.Raised by cubic on #7 and deliberately not fixed there: a correct fix needs a decision this codebase has not made. The code carries a standing hazard note recording the same thing.
What was tried, and what it measured
A per-sink occupancy cap (8 MiB) was implemented and then withdrawn. Two defects, both measured by instrumenting the demux to record peak per-sink bytes across eight interleavings with 1 MiB chunks, both streams read to completion.
The accounting was inflated.
pulldoes not re-fire per chunk, so a running total keeps charging chunks the consumer has already read. Deriving occupancy from1 - controller.desiredSizeplus a mirrored FIFO of chunk sizes corrected it — the 2:1 interleaving's peak fell from 3.0 MiB to 1.0 MiB.With the accounting corrected, the overshoot was still real. A consumer lagging one macrotask per read peaks at 9 MiB and gets errored — on a stream it is reading to completion. No sink applies backpressure to the shared reader, so a live-but-slower consumer falls behind without bound, and any fixed occupancy cap must eventually fire on a working stream.
The obvious escape does not work either. Erroring only a sink whose consumer has taken nothing fails unfixably: microtasks starve macrotasks completely — measured at 100,000 microtask turns before a single
setTimeout(0)fired — so a consumer on a macrotask cadence takes nothing while the drain runs, and "took nothing" cannot distinguish a slow consumer from an absent one.Why the naive fixes are wrong
Honoring the other sink's
desiredSizereintroduces the hang the current design exists to avoid: one shared reader feeds both sinks, anddrainUntilenqueues the other source's events on purpose so a consumer reading a single stream to completion does not deadlock.truncatedevents are already droppedWhat closing this actually requires
One of:
cancel()the one it does not need. That makes pausing the drain legitimate rather than a hang, and backpressure becomes implementable. It is a public-contract change, which is why it is not a review-fix decision.Exposure today
Narrow.
runreads both streams concurrently viaPromise.all, so it never accumulates. Only aspawncaller that consumes one stream and abandons the other without cancelling is affected.Notes
src/sandbox/harness/**is vendored fromchatbot-pf/pleaseworksand relicensed to Apache-2.0, so whichever way this is decided is also a divergence decision.test/sandbox/harness/process.demux.test.tspin the property the cap broke — any future guard has to keep them green.