runtime: discard host settlements before facade conversion - #336
Merged
Conversation
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.
Summary
Fixes #328
Fixes #329
Move host-import facade result conversion behind the boundary's delivery decision. Observe refused Promise settlements while preserving call-site
NeedsJspi; discard cancelled/poisoned settlements before resource transfer or producer creation. End facade borrow scopes when cancellation/refusal releases their canonical lenders, retaining settlement cleanup as an idempotent backstop.Semantic basis
Component Model pin:
7c676115e93cd7d54c1732d95c54c6a3de7c5ae0.CanonicalABI.md, Tasks/FuncInst calling convention: the callee's cancellation callback may resolve withNone; the callback is requested at most once while unresolved.definitions.py:Store.invokewraps the callee's cancellation callback.canon_lower.on_resolve(None)resolves cancellation withoutlower_flat_values; the value branch alone lowers results.Subtask.resolverejects a second resolution.canon_subtask_cancelrequests cancellation and delivers the terminal event;Subtask.deliver_resolvereleases lenders.lower_own,lower_stream, andlower_futureadd recipient handles, so calling them for discarded delivery is observably different from skipping them.Concurrency.md§Cancellation: cancelled-before-returned means arguments were passed but no values returned, with borrows released.contracts/embedder-api.md§Functions and async / Cancellation and discard anddocs/architecture.md§6 choose prompt cancellation on the JS host's behalf. This repair enforces that existing policy before facade ownership conversion, includingComponentExceptionpayloads.deferCancelandabortableretain their existing policy;future<T>-typed import returns remain future sources rather than async call completion.contracts/intrinsics.mdrequires lender unwind even for non-poisoning capability failures. Per-instance poisoning and JS Promise/JSPI capability refusal are embedding policies, not additional spec cancellation states.Wasmtime comparison
Inspected the repository's Wasmtime pin
4675ee16b703b33948073a5ff6b961367371e7a1:func/host.rs::call_async_lowervalidates scope exit, then callslower_rawonly forSome(result).concurrent.rs::Instance::first_pollpassesNonewhen abort wins and publishesReturnCancelled. When completion wins,Some(result)lowers normally; this is not a claim of identical JS/native race policy.subtask_cancelrequests abort through the host-task join handle;JoinHandle::runsupplies theNoneoutcome.Lowerimplementations reachguest_resource_lower_ownthrough result lowering. Thus the cancelled outcome skips recipient result-handle insertion rather than inserting and disposing. This says nothing about host registrations or producer work performed independently before return. Successful settlement followsSome(result)and lowers normally.Executed comparison: built the pinned tree with scratch
CARGO_TARGET_DIR, then rancargo test -p component-async-tests --test test_all -- cancel(10 passed) andcargo test -p component-async-tests --test test_all -- borrowing resource(5 passed). These exercise native cancellation/terminal-state and resource/borrowing behavior. The no-registration-on-cancel conclusion is source-backed, not a bespoke native resource-table assertion.The comparison rests on the explicit cancel-to-
None-to-skipped-lowering path, not Rust Future droppability alone. Wasmtime's dropped host future and this runtime's continuing JS operation differ; the existing JS contract chooses to discard delivery.NeedsJspi, Promise adoption, and per-instance poisoning have no exact Wasmtime analogue. Wasmtime is corroboration; the pinned spec and facade contract govern this repair.Verification
Real-Wasm facade regressions demonstrated the base failures directly: both sync-refusal modes left unhandled rejection or registered an undelivered resource; cancelled/poisoned fulfillment and ComponentException payloads registered resources; discarded stream/nested-future results started both producers. Counters check absence of conversion, not conversion followed by disposal.
Guest-wrapper non-invalidation is source-derived assurance: discard bypasses the carrier's conversion, hence
fromHostand guesttakeRep. Existing resource tests separately covertakeRepinvalidation. A real-Wasm host import returning a locally defined guest resource cannot be expressed through the supported facade: the pinned spec rejects imports depending on exported resource types (Explainer.mdexternal dependency rules;test/validation/external-visibility.wast), and imported-component substitution is outside the facade's supported imports. No host-bound wrapper stand-in is claimed as equivalent coverage.just fmt-check: passed.just gates < /dev/null: passed end to end after installing the worktree's missing Playwright browsers. Includes all six Chromium/Firefox conformance realms and six OPFS smokes. WAT regeneration matches both committed Wasm fixtures byte-for-byte. Independent production, test/fixture, and authority-reasoning reviews passed.Auto-merge is armed with a merge commit under repository defaults.