fix: settle cancel-during-stream stalls (readiness livelock, heap-guard wedge, buffered durable updates) - #1917
Open
WyvernMonarch wants to merge 4 commits into
Conversation
added 4 commits
August 27, 2026 17:44
The session readiness broker is level-triggered: `complete_wake` rearms a wake whenever any capability still carries flags, and flags only clear on an explicit acknowledgement. `dispatch_ready_batch_callbacks` deliberately left a `TargetMissing` observation unacknowledged, assuming the guest was merely mid-registration. When the target never appears — teardown already ran `unregisterCapabilityReadiness`, or the capability generation is permanently stale — that assumption livelocks the session: dispatch misses, the observation stays unacknowledged, `complete_wake` rearms immediately, and the loop spins at full CPU without ever reaching a turn boundary, so the turn never terminates and cancellation never settles. Give a missing target exactly one free retry instead. The registration race the old behavior tolerates resolves within a single wake; the second consecutive miss of the same revision is acknowledged, which clears the level flags and lets the loop idle. A capability that registers later still receives delivery, because new data bumps the revision and republishes readiness. The regression test drives the same bookkeeping without a V8 isolate and asserts the wake counters stop growing. Reverting the acknowledgement makes it spin: ~640k wake/batch/complete cycles per second on one capability.
Dropping a socket alias removed its readiness subscriber but left the capability in that VM's readiness broker. The guest runs `unregisterCapabilityReadiness` on the same teardown, so any level flags still held for that capability now have no reachable dispatch target: they keep the session's wake lane armed and the broker's handle table populated against a capability that can never be acknowledged normally. Hand the removed target back out of `unregister` so the RAII drop can also call `remove_readiness`, which clears the broker entry through the existing `SessionReadyBroker::remove_capability`. Re-registration is unaffected: it swaps the previous identity directly and never routes through `unregister`.
V8 raises the near-heap-limit callback as soon as a full GC cannot fit the live set under the configured cap. A healthy guest sitting at its high-water mark reaches that without being a heap bomb, and terminating there kills guest JS mid-frame: the termination exception is uncatchable, skips `finally` so bridge invariants latch, and — when it lands outside an `Execute`, the only place that calls `cancel_terminate_execution` — leaves the isolate terminating. Every later guest call on that isolate then returns nothing, which the readiness turn reads as a missing dispatch target and retires the capability's level flags against. The session does not fail: it goes idle forever, with the sidecar parked and no error on any lane. Grant the headroom the callback already returns before terminating, and terminate only when the heap reaches the raised limit too. The guest stays bounded at `limit + NEAR_HEAP_LIMIT_HEADROOM_BYTES` — tighter than before, where every callback both terminated and raised the limit again. Also surface an out-of-band termination observed during a readiness dispatch as the termination it is, so a guest that genuinely cannot fit reports a terminated execution instead of idling.
`DurableUpdateSink::handle_notification` buffered every non-message
`session/update` — `tool_call`, `tool_call_update`, `plan`, `mode`,
`available_commands_update` — whenever a message chunk had already opened
the completion buffer, and committed the buffer only at the next message
boundary.
Any agent that streams text before calling a tool therefore withholds its
whole durable stream until the *post-tool* message arrives. Pi prints a
banner chunk on some turns, so roughly a third of prompt turns delivered
`tool_call_update { in_progress }` for a `sleep 60` only when the command
finished, together with every other update of the turn. A caller that
waits for that boundary before cancelling never observes a live turn:
`session/cancel` then races prompt completion and reports
`no_active_prompt`.
Commit the in-progress text run first, then the update. The durable
sequence is unchanged — `coalesce_completed_message` already ends the
text run it is building at any non-text update, so the same events are
emitted in the same order, only in more (smaller) batches — and the
update now reaches the host the moment the adapter produces it.
The regression test drives an ACP adapter that emits one message chunk,
then the tool updates, then holds the turn open for two seconds. It
records the live event sink and fails if the `in_progress` update lands
at turn end: on the previous behavior it arrives at 2.06s of a 2.06s
turn.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cancelling a prompt while a bash or filesystem stream is active can stall the session until the host force-disposes the VM. In our stress reproducer (5 cancel iterations per run, linux/arm64 containers) the stall reproduced in 18 of 40 baseline runs (~45%); with this branch it is 0 of 32, and a cancellation/accounting probe went from 12/20 to 20/20 GO.
The stall turned out to be a chain of four defects; each fix unmasked the next, so they are validated together but committed separately.
1.
fix(v8-runtime): bound unacknowledged readiness retries— the livelockThe readiness broker is level-triggered:
complete_wake(crates/runtime/src/readiness.rs) re-arms a wake whenever a capability still has non-empty flags, and flags clear only on acknowledgement.dispatch_ready_batch_callbacks(crates/v8-runtime/src/session.rs) deliberately left aTargetMissingobservation unacknowledged, assuming the miss was transient. When the guest's dispatch map permanently lacks thecapabilityId:capabilityGenerationkey (teardown raced the flags, or readiness was published before registration with a generation that never matches), the result is an infinite hot loop: wake → miss → unacknowledged → instant re-arm — a full core spun, the turn never reaches a terminal boundary, andsession/cancelnever settles.The fix tracks consecutive misses per
(capability, generation, revision)and acknowledges on the second miss of the same revision, so the loop dies. A genuine transient race keeps one free retry, and a live capability recovers regardless: new data bumps the revision and republishes readiness (mark_readybumps unconditionally, and a stale-revision ack is a no-op, so no real wake can be lost).Regression test (no VM needed): a readable capability with interest enabled and no JS registration; asserts
Delivered/Rearmedwake metrics stop growing after the bounded miss (they grow without bound before the fix) and that delivery resumes after the target registers and new data is published.2.
fix(native-sidecar): retire readiness capabilities on socket teardownTeardown hygiene for the same machinery: dropping a socket's readiness registration now also retires the capability broker-side (
remove_capability), instead of leaving level flags behind for a target that will never return. Unit-tested at theunregisterseam.3.
fix(v8-runtime): stop the heap guard from silently wedging a guestFound while validating: V8's near-heap-limit callback fired for a healthy guest at its high-water mark and terminated it mid-frame; when the termination lands outside
Execute, the isolate is left terminating and every later guest call returns nothing — which the readiness turn then reads as a missing dispatch target (feeding defect 1), while the session sits idle forever with no error on any lane. The fix grants the headroom the callback already computes and terminates only when the raised limit is also exhausted, and surfaces an out-of-band termination observed during a readiness dispatch as a terminated execution instead of silence.4.
fix(acp): stream durable session updates that follow a message chunkDurableUpdateSink::handle_notificationbuffered every non-messagesession/updateonce a message chunk had opened the completion buffer, committing only at the next message boundary. An agent that streams a text banner before calling a tool therefore withheldtool_call_update { in_progress }(and everything else) until the tool finished — so a caller waiting for a live turn before cancelling raced prompt completion and gotno_active_prompt. The fix commits the in-progress text run first, then the update; the durable sequence is unchanged, only batched smaller. The regression test fails on the old behavior with thein_progressupdate arriving at 2.06 s of a 2.06 s turn.Validation
cargo testgreen acrossagentos-runtime(60),agentos-v8-runtime --lib(155),native-sidecar state::(8),agentos-sidecarsuites;rustfmt --checkclean on all touched files.Notes for maintainers
forward_runtime_wake_locked(crates/v8-runtime/src/session.rs)try_recvs a wake from the capacity-1 lane and drops it if the capacity-1 executor lane is full, while the broker staysOutstanding— a silent-coalesce liveness hazard. We could not construct a reproducing schedule, so it is intentionally left out of this PR.session::tests::begin_destroy_session_removes_entry_before_finishis flaky under heavy machine load at base (fails ~7/10 under load, 155/155 when idle, untouched by this diff) — likely worth its own issue.🤖 Generated with Claude Code