runtime: bound the driver's speculative resume gate to the sole driver (#239) - #240
Merged
Conversation
#239) `driveAsync`'s awaiting-race took a speculative `Store.pendingResumptions` entry and held it across `Promise.race([chosenTag, ...others])`, where `others` includes `store.pendingHostCalls` — an await bounded only by when the HOST answers. That set is a store-wide scheduling gate: `Store.tick` refuses while it is non-empty and every `driveAsync` yields at its top under a 10,000-hop bound. So a SECOND driver on the same store spun there and died in ~311ms on driveAsync: a resumed-activation claim was never released (the activation neither parked, finished, nor trapped) an internal-bug detector firing on a perfectly ordinary suspended guest. This is the same-store half of #210, which fixed only the cross-store half by making the gate per-store; `tests/cross_store_driver_test.ts`'s header already described the entry as "held for the entire duration of a guest's wait on a slow host import". The reported route was a detached `spawn_local` guest task dropping an in-flight async-import future — wit-bindgen's specified cancellation path, which issues `subtask.cancel` and parks the frame mid-activation with no export call outstanding, leaving the settlement pump holding the entry. It reduces much further: two concurrent export calls and one slow suspending import are enough, with no cancellation and no detached task anywhere. Fix, in two parts: * the speculative entry is taken only by the SOLE driver (`storeDriverDepth(store) === 1`). It gates other `Store.tick` callers, never the awaiting loop itself, so it can only ever constrain concurrent drivers and `HostActivity.pump`'s drain; the resumption invariant does not rest on it (see the `driverDepth` note's mechanisms (a) and (b)). * a per-store driver-arrival one-shot rides every park in the loop, fired by any `driveAsync` that finds itself at depth > 1. The incumbent wakes within a microtask, drops the entry on its way out, and re-evaluates `done()` — which is the stand-down the fallback pumps were always supposed to perform, now prompt rather than "whenever the host happens to answer". The `driverDepth` block's "bounded overlap window" is bounded for the first time. Tests. `same_store_driver_test.ts` is the #210 probe's same-store sibling: the arriving driver returns promptly, the gate still gates a sole driver, the entry is dropped while a second driver is live, and — the #158 property — the release removes only the entry it added. `examples/guests/cancel-import` is a new wit-bindgen fixture carrying all three routes into the wedge, driven end-to-end by `e2e_cancel_import_test.ts`. Every test was verified to fail against the reverted fix. Not fixed here, and unchanged by this diff: a host-import subtask's `on_cancel` is a no-op, so a guest's `subtask.cancel` on an in-flight host import still blocks until that import completes naturally (measured: cancel issued at t=312ms, returned at t=4010ms for a 4000ms import). Filed separately.
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
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.
Fixes #239.
What was actually wrong
driveAsync's awaiting-race took a speculativeStore.pendingResumptionsentry and held it across
Promise.race([chosenTag, ...others]), whereothersincludesstore.pendingHostCalls. That await is bounded only bywhen the host answers.
Store.pendingResumptionsis a store-wide scheduling gate:Store.tickrefuses while it is non-empty, and every
driveAsyncyields at its top whilestore.hasPendingResumptions()holds, under a 10,000-hop bound. So asecond driver on the same store spun there and died in ~311ms with
— an internal-bug detector firing on a perfectly ordinary suspended guest.
This is the same-store half of #210, which fixed only the cross-store half by
making the gate per-store.
tests/cross_store_driver_test.ts's header alreadydescribed the entry as "held for the entire duration of a guest's wait on a
slow host import"; nobody asked what happens when the second driver is on the
store actually doing the waiting.
The reported shape is not the minimal one
#239 reports a detached
spawn_localguest task dropping an in-flightasync-import future — wit-bindgen's specified cancellation path, which issues
subtask.cancel, parks the frame mid-activation, and leaves the settlementpump holding the entry with no export call outstanding. That reproduces, and
so does a strictly smaller shape with no cancellation and no detached task
at all:
The issue's own variation matrix corroborates the diagnosis: V4 and V4c did
not wedge precisely because an export call's driver was live, so the fallback
pump never became a second driver.
The fix
Two parts, both in
runtime/src/exec/boundary.ts:The speculative entry is taken only by the sole driver
(
storeDriverDepth(store) === 1). It gates otherStore.tickcallers andnever the awaiting loop itself, so it can only ever constrain concurrent
drivers and
HostActivity.pump's synchronous drain. The resumptioninvariant does not rest on it — that is the
driverDepthnote's mechanisms(a)
resumeWith's synchronousstore.awaitingdeletion and (b)tagAwait'sper-promise memoization, plus the resumption site's own membership /
promise-identity /
dispatchableTailre-checks.A per-store driver-arrival one-shot rides every park in the loop, fired
by any
driveAsyncthat finds itself at depth > 1. The incumbent wakeswithin a microtask, drops the entry on its way out of the race, and
re-evaluates
done()— which is exactly the stand-down the fallback pumpswere always supposed to perform, now prompt instead of "whenever the host
happens to answer". The
driverDepthblock's "bounded overlap window" isbounded for the first time.
The 10,000-hop assert is retained: with the entry's lifetime bounded by
another driver's arrival, it is once again the internal-bug detector it was
meant to be.
Tests
runtime/tests/same_store_driver_test.ts— the #210 probe's same-storesibling. Four properties: the arriving driver returns promptly; the gate still
gates a sole driver (the CONTROL, mirroring the cross-store test's tail); the
entry is dropped while a second driver is live; and the release removes only
the entry it added — the #158 property, which a blanket-clear "fix" would
violate while passing the other three.
examples/guests/cancel-import— a new wit-bindgen guest fixture carryingevery route into the wedge (poll-once-then-drop, timeout race, detached
mid-frame park, two concurrent export calls, plus a
pinghealth poll),driven end-to-end by
runtime/tests/integration/e2e_cancel_import_test.ts.Every new test was verified to fail against the reverted fix with the exact
assertion above, and the #158 test against a
pendingResumptions.clear()mutation.
Not fixed here
A host-import subtask's
on_cancelis a no-op (subtask.onCancel = () => {}),so a guest's
subtask.cancelon an in-flight host import still blocks untilthat import completes naturally — measured with this fixture: cancel issued at
t=312ms, returned at t=4010ms for a 4000ms import. That is why polymorph-iroh's
dial timeout would still be unusable even with the wedge gone, and it needs an
embedder-API decision rather than a scheduler fix. Filed separately as #241.
This diff does not change it in either direction.
Gates
just gates— exit 0. Notable lanes: runtime 651/0; conformance 1257/0 with noexpectation changes (1254/0 under seed 1); sched-seeds 648/0 under both seeds;
sm/node/bun pinned shells all "OK, matches expectation"; chromium + firefox
including worker and shared-worker realms; smoke-tls PASS; smoke-c0 legs 1-4
PASS.
No published-surface change, so no
breaking/*label and no version bump; theconventions goldens are byte-identical.