lift: settle an idle-exited async export on task.return, not thread drain - #315
Merged
Conversation
…rain `backgroundCompletion` (#310) resolved the host's Promise from `Task.onFinished` — the moment the task's last thread unregistered. A callback-ABI export that keeps spawned futures alive for the instance's life (wit-bindgen `spawn_local`: an event loop, a driver, an accept loop) never reaches that moment, so a lift that went idle before `task.return` and was later woken by another driver had its results captured and its Promise left pending forever. Silent hang; found while tracing #312. The reference's answer event is `on_resolve`, called from `Task.return_`; `lift_and_run` keeps ticking afterwards for OTHER work, not to produce the result. wasmtime's `call_concurrent` is the same shape. So: the lift's own `onResolve` closure now fires a one-shot hook that settles the Promise, and an already-resolved task settles at once (generalizing the old `threads.length === 0` case). No deferral to "no wasm call in flight": `driveDone`'s liveness clauses keep a DRIVER driving, and on this path there is no lift driver left to stop. `Task.onFinished` and its `unregisterThread` firing block are deleted; nothing else used them. Test: runtime/tests/lift_background_return_test.ts — an export goes idle with an immortal thread and a parked returner, a later export call drives the store, the returner calls `task.return`, the first Promise resolves while the immortal thread is still parked. Red on main, green with the fix. Closes #313.
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 #313.
backgroundCompletion(#310) resolved an idle-exited async lift onTask.onFinished— the task's last thread unregistering. A callback-ABI export that keeps spawned futures alive for the instance's life (wit-bindgenspawn_local: event loop, driver, accept loop) never reaches that moment, so a lift that went idle beforetask.returnand was later woken by another driver had its results captured and its host Promise left pending forever. Silent hang, found while tracing #312.The reference's answer event is
on_resolve, called fromTask.return_(definitions.py);lift_and_runkeeps ticking afterwards for other work, not to produce the result. wasmtime'scall_concurrenthas the same shape. The lift's ownonResolveclosure now fires a one-shot hook that settles the Promise; an already-resolved task settles at once (generalizing the oldthreads.length === 0case). No deferral to "no wasm call in flight":driveDone'smidWasmCall/hopParkedclauses keep a driver driving under a suspended activation, and on this path there is no lift driver left to stop.Task.onFinishedand itsunregisterThreadfiring block are deleted — this obsoletes them and nothing else used them.Test:
runtime/tests/lift_background_return_test.ts, store-level in the style oflift_done_verdict_test.ts. An export goes idle with an immortal thread and a returner parked on a flag; a later export call drives the store; the returner callstask.return; the first Promise resolves while the immortal thread is still parked (asserted, so drain is not what settled it). Red on main ("expected {v:undefined}, got pending"), green with the fix; also green underPOLYENGINE_SCHED_SEED=1and7.Gates:
just test-runtime— 739 passed, 0 failed. Goldens untouched; nobreaking/*label (bug fix).Automerge armed.