diff --git a/contracts/descriptor-ir.md b/contracts/descriptor-ir.md index 2aa8f54..fb92296 100644 --- a/contracts/descriptor-ir.md +++ b/contracts/descriptor-ir.md @@ -77,7 +77,7 @@ the implementation truth; the destination is now embedder-api.md's. |---|---| | bool | `boolean` | | s8..u32, f32, f64, char (as code point) | `number` | -| s64/u64 | `bigint` (range-checked at lower) | +| s64/u64 | `bigint` (**v0.2 correction**: NOT range-checked at lower — the interpreter wraps mod 2⁶⁴ like every other integer lane, matching definitions.py's `% 2**64`; the original claim never matched the implementation) | | string | `string`; lowering applies USVString replacement (`toWellFormed`) | | list | `Uint8Array` (always a copy, never a view into guest memory) | | other lists / tuples | `Array` | @@ -117,8 +117,11 @@ extension must land with fixtures. instantiate), so resource-type identity never leaks across instances. - Variant/option host shapes are settled for the interpreter but bindgen (§9) may want ergonomic variations — any change lands here first. -- `map` (in types.ts, from the reference) is not emitted by current - translators; keep behind a fixture-only flag. +- ~~`map` (in types.ts, from the reference) is not emitted by current + translators; keep behind a fixture-only flag.~~ **Closed (v0.2): stale.** + The shim enables `CM_MAP` and emits `map` types (translator-shim lib.rs + `features()`, plan.rs `ValTypeJson::Map`); the loader consumes them. `map` + despecializes to `list` per the reference. ## v0.1 amendments (post-M0 reality) @@ -130,3 +133,15 @@ extension must land with fixtures. 2. **Flattening contract validated as written**: computed `flattenFunctype` vs the options' `coreType` asserted at instantiate across the whole fixture corpus with zero mismatches. No change. + +## v0.2 amendments (2026-08-10 adversarial review, deltic#98) + +Documentation corrections only — no wire or behavior change: + +1. **s64/u64 "range-checked at lower" claim retracted** (host-mapping table + above): the interpreter has always wrapped bigint lanes mod 2⁶⁴ per + definitions.py `lower_flat`; the table asserted a check that never + existed. Host-side range *asserts* (host-precondition errors, not traps) + exist only on the scalar `storeInt` path as of deltic#96. +2. **`map` open item closed as stale** (struck through above): emitted by the + shim, consumed by the loader, exercised by the values suite. diff --git a/contracts/embedder-api.md b/contracts/embedder-api.md index 3352581..389ce84 100644 --- a/contracts/embedder-api.md +++ b/contracts/embedder-api.md @@ -19,7 +19,11 @@ pins u8 stream chunks as `Uint8Array` in both directions; amendment A6 examination", renumbered from a colliding second "A5"); amendment A7 (2026-08-11) makes component faults loud on host stream/future operations (`PeerTrappedError`, never a hang or a fake end-of-stream) -and limits host ends to one in-flight operation per direction.** This document supersedes `descriptor-ir.md`'s interim +and limits host ends to one in-flight operation per direction; amendment +A8 (2026-08-10, deltic#90/#97) makes `Future.drop()` before writing an +**abandonment** (total, never-throwing; a guest reader observes a trap at +its rendezvous, never DROPPED) and documents host `cancelRead` as +indistinguishable from end-of-stream by design.** This document supersedes `descriptor-ir.md`'s interim "host value mapping" table as the destination for host-facing value shapes. The runtime's *raw* boundary (`instance.exports`, `HostImports`) keeps the `definitions.py` interpreter shapes as an **internal** surface; the @@ -412,6 +416,25 @@ class DroppedError extends Error { … } // awaiting a dropped future rejects the same stream stays legal (they are different ends). Previously the second operation could "rendezvous" against the first one's parked buffer and report data as taken by a peer that never existed. +- **Dropping an unwritten future is abandonment, not DROPPED** (amendment + A8, deltic#90). The CABI forbids a writable future end from dropping + before delivering its value (definitions.py:1183-1184) — a guest doing + so traps. The host-side spelling: `Future.drop()`/`[Symbol.dispose]` on + a **lowered**, never-written future never throws and is idempotent; the + guest-held readable end observes a **trap at its rendezvous point** + ("the host dropped the writable end without writing a value") — pending + read, later read, or waitable-set delivery alike — never a DROPPED + event (which the CABI says a future reader cannot see) and never a + hang. An unlowered future (the guest never saw it) just releases state. + Producer failures (`Promise` rejection under `lowerFutureSource`) keep + their A7-era reporting: the in-flight call fails site-named via the + host-failure channel. +- **`cancelRead` is indistinguishable from end-of-stream — by design** + (amendment A8, deltic#97). A host-side `Stream.cancelRead()` settles the + in-flight `read` with an empty chunk, which `readable()`/the async + iterator present as clean EOS. The canceller is the same code observing + the end, so no discriminated signal is warranted; pinned by test. (A + *peer* fault is never presented this way — that is A7's rule.) ## Module wiring and instantiation diff --git a/contracts/intrinsics.md b/contracts/intrinsics.md index e72ac2f..741fc3f 100644 --- a/contracts/intrinsics.md +++ b/contracts/intrinsics.md @@ -120,7 +120,13 @@ core" is a feature, not a crash. restore `may_leave` on all component instances — FACT clears it around lift/lower and a trap skips its restore; without both unwinds the instance is unusable for post-trap re-entry, which this runtime - deliberately supports. + deliberately supports. **Scope clarification (2026-08-10, deltic#91): + the obligation covers every window that registers lenders, including + the prepare/start protocol** — `sync-start-call`'s inline lender scope + and `async-start-call`'s subtask-attached lenders release on every + non-success exit that does not poison the caller (trap rethrow AND + capability signals: `NeedsJspi` is expressly non-poisoning and must not + strand lenders). 3. **Host-trap preservation across nested barriers**: the trap trampoline must (re)record the pending trap before every throw, so the specific message survives arbitrarily nested adapter exception barriers. Residual, diff --git a/crates/translator-shim/src/lib.rs b/crates/translator-shim/src/lib.rs index 221d1c5..607c5cc 100644 --- a/crates/translator-shim/src/lib.rs +++ b/crates/translator-shim/src/lib.rs @@ -83,9 +83,32 @@ fn features() -> wasmparser::WasmFeatures { f.insert(wasmparser::WasmFeatures::CM_MAP); f.insert(wasmparser::WasmFeatures::CM_IMPLEMENTS); f.insert(wasmparser::WasmFeatures::CM_THREADING); + // ISSUE #95 TRIPWIRE — do not enable `CM_VALUES`. + // + // Trusted wasmtime-environ 47.0.3's component frontend has two + // `unimplemented!()` panics that a `CM_VALUES`-accepted component can + // reach: a component `start` section (translate.rs:1338) and a + // component-level value import/export (translate.rs:1499). With the + // feature off (the wasmparser 0.252 default excludes it, and nothing + // above turns it on), `wasmparser::Validator` rejects both shapes during + // validation — a `TranslateError { phase: Validation, .. }` envelope, + // never reaching the translator body that panics. That is exercised and + // pinned by `tests/cm_values_tripwire.rs`. + // + // Turning `CM_VALUES` on would convert that validation-phase rejection + // into a genuine panic. On the native (test) build that unwinds and is + // merely an ugly failure; on the wasm32-unknown-unknown C-ABI build this + // crate ships (`just shim`; `Cargo.toml`'s release profile pins + // `panic = "abort"` for that target — see the note on `catch_unwind` + // below) it is a hard trap with **no JSON envelope at all**, violating + // this crate's "never panics on invalid input" claim (see the doc + // comment on the C-ABI entry point). If `CM_VALUES` is ever enabled here, + // the translate.rs call sites above need a real plan-format mapping (or + // an explicit `phase: Unsupported` pre-check) before the flag flips. f } + /// Feature names recorded in `plan.producer.features`. Must describe /// `features()` — part of the artifact-cache key. fn feature_names() -> Vec { diff --git a/crates/translator-shim/tests/cm_values_tripwire.rs b/crates/translator-shim/tests/cm_values_tripwire.rs new file mode 100644 index 0000000..1368379 --- /dev/null +++ b/crates/translator-shim/tests/cm_values_tripwire.rs @@ -0,0 +1,62 @@ +//! ISSUE #95 tripwire: pins that a component using a `start` section, or a +//! component-level `value` import, is rejected in the VALIDATION phase +//! today — never reaching the `unimplemented!()` panics trusted +//! wasmtime-environ 47.0.3's `translate.rs` has for both shapes +//! (`:1338` for `start`, `:1499` for values). See the `CM_VALUES` comment +//! on `features()` in `src/lib.rs`. +//! +//! Both shapes are gated by wasmparser's `cm_values` feature +//! (`wasmparser::WasmFeatures::CM_VALUES`, see +//! `validator/component.rs::ComponentState::add_start`'s +//! `require_feature::cm_values` call), which `features()` never enables — +//! `wasmparser::Validator` rejects them before `Translator::translate` ever +//! sees them. If a future change to `features()` turns `CM_VALUES` on, +//! these tests start failing (the panic aborts the *test process*, which +//! `cargo test` reports as a hard crash rather than a clean assertion +//! failure) — that failure mode is itself the tripwire. + +use translator_shim::{translate, Phase}; + +/// A component with a top-level `start` function. Not decodable to a +/// meaningful plan under `CM_VALUES` off; must be a validation-phase +/// rejection ("component model `value`s" feature-gate error), not a panic. +#[test] +fn start_section_is_a_validation_rejection() { + let wat = r#" + (component + (core module $m + (func (export "f")) + ) + (core instance $i (instantiate $m)) + (func $f (canon lift (core func $i "f"))) + (start $f) + ) + "#; + let bytes = wat::parse_str(wat).expect("start-section component should parse as WAT"); + let err = translate(&bytes).expect_err("start section must be rejected, not accepted"); + assert_eq!( + err.phase, + Phase::Validation, + "start section must be a VALIDATION verdict (assert_invalid-equivalent), \ + not Unsupported/Internal — got {err:?}", + ); +} + +/// A component-level `value` import. Same feature gate, same expected +/// verdict. +#[test] +fn value_import_is_a_validation_rejection() { + let wat = r#" + (component + (import "v" (value string)) + ) + "#; + let bytes = wat::parse_str(wat).expect("value-import component should parse as WAT"); + let err = translate(&bytes).expect_err("value import must be rejected, not accepted"); + assert_eq!( + err.phase, + Phase::Validation, + "value import must be a VALIDATION verdict (assert_invalid-equivalent), \ + not Unsupported/Internal — got {err:?}", + ); +} diff --git a/docs/architecture.md b/docs/architecture.md index 0612f74..cd79293 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -343,6 +343,20 @@ pump stands down whenever an export-call driver is live (the invariant and its benignity argument are documented at the site in `runtime/src/exec/boundary.ts`). +Named divergence (2026-08-10, [#92](https://github.com/lann/deltic/issues/92)): +**the async form of `subtask.cancel` is not atomic under jspi.** The +reference built-in returns `[BLOCKED]` with no suspension; deltic parks the +caller on a determinacy wait so the BLOCKED/resolved answer matches the +reference's synchronous-delivery outcomes across the engine's mandatory +microtask hop (jspi pin (j), pinned by `cancellable.wast`). While parked, +other ready threads of the store may run, so sibling-task effects can become +observable across the single built-in call — a reordering *within* the +reference's own `Store.tick` freedom, taken one built-in early; every +interleaved sibling was already at a block point. Rationale and mechanics at +the site (`runtime/src/intrinsics/async_builtins.ts`, the determinacy park +in `createSubtaskCancel`); regression pinned across seeds by +`runtime/tests/cancel_bracket_race_test.ts`. + ## 7. Canonical ABI decisions Authority: [CanonicalABI.md] and its executable reference @@ -379,13 +393,33 @@ decide deliberately and document here. is a core function `[rep] -> []`, invoked as a normal **non-async** cross-component call — *"the destructor may not block. However, the destructor may spawn a cooperative thread that does."* Reentrance is checked - (`may_enter_from`) with the same-instance exemption. Host policy: + (`may_enter_from`) with the same-instance exemption, and a trapping dtor + poisons the **implementing** instance (the reference's `Store.lift` bracket, + reconstructed at `runtime/src/cabi/handles.ts` `callDtorGated` — + implemented at [#85](https://github.com/lann/deltic/issues/85); the + same-instance exemption falls out of `entering_set`, not a special case). + Host policy: - CM-level blocking in a dtor → deterministic trap (falls out of general sync-task rules). - Host-import latency is invisible to CM semantics; a dtor calling a `Suspending` host import is legal but needs a suspension-legal stack: JS-initiated drops (`using`, FinalizationRegistry) enter via a `promising` - trampoline; guest-initiated drops stay on pure-wasm paths (§5). + trampoline (`ResourceTypeInfo.dtorHost`, wired by the executor in jspi + mode for suspension-capable dtors — a non-suspendable dtor keeps the + exact synchronous path, avoiding the promising microtask hop's + one-turn entered window; the async entry bracket is held until the + activation settles, tracked in `pendingHostCalls`). **Known + limitation** (#85 scope note): a + *guest*-initiated drop reaches the dtor through a JS trampoline frame, + not the §5 pure-wasm funcref path — a Suspending import under it is a + deterministic JSPI frame-rule trap, not a supported suspension. The + pure-wasm dispatch path is future machinery; until then §5's + "guest-initiated dtor calls route through generated wasm" is aspiration, + not description. + - Host-held own handles carry lend tracking mirroring `num_lends` + ([#86](https://github.com/lann/deltic/issues/86)): drop/GC-backstop defer + while lent; a backstop dtor trap poisons the implementing instance and + lands on the host-failure channel (never `catch {}`-swallowed). - Upstream spec findings related to drops and backpressure (vestigial `$async?` on `resource.drop`; dead `canon_backpressure_set` in definitions.py) are tracked in diff --git a/justfile b/justfile index 28f0eb0..a7053e0 100644 --- a/justfile +++ b/justfile @@ -169,8 +169,10 @@ browsers: # Translate all eight targets, then execute the suites. # polymorph-tls conformance under deltic (issue #18). +# (--allow-env: tools/smoke-c0/common.ts reads POLYMORPH_ROOT at module +# scope since the wosh rename; the leg tasks always had it via deno task.) smoke-tls: shim - deno run --allow-read tools/smoke-tls/run.ts --exec + deno run --allow-read --allow-env tools/smoke-tls/run.ts --exec # The C0 smoke legs (tools/smoke-c0/REPORT.md). smoke-c0: shim diff --git a/runtime/src/cabi/bulk_lists.ts b/runtime/src/cabi/bulk_lists.ts index 1efb198..ab1a4cf 100644 --- a/runtime/src/cabi/bulk_lists.ts +++ b/runtime/src/cabi/bulk_lists.ts @@ -5,10 +5,25 @@ // loop body with one typed-array view per list and a tight per-element pass // that preserves the interpreted path's EXACT observable semantics: // -// * integers: the same `assert_` texts as `storeInt` (`"int store"`, -// `"64-bit store requires bigint"`), the same wrap-on-overflow (a -// TypedArray element write coerces exactly like the matching DataView -// setter), the same number/bigint host shapes on lift; +// * integers: the same `assert_` type-shape texts as `storeInt` (`"int +// store"`, `"64-bit store requires bigint"`) but, unlike the scalar path +// in memory.ts (`storeInt`'s range `assert_`s, issue #96), NOT the same +// range check: this bulk path wraps out-of-range values instead of +// raising the host-precondition error (`OverflowError` per +// definitions.py:1568-1569 `int.to_bytes`) that `storeInt` raises. That +// is a deliberate scalar/bulk posture split, not an oversight: +// - the whole point of this file (see the perf numbers above) is an +// allocation-free, branch-minimal per-element loop; an added range +// check is itself a per-element cost, defeating the purpose; +// - values reaching this path from a descriptor-driven lower already +// went through the descriptor layer's own type conversions for the +// cases that matter in practice (see contracts/descriptor-ir.md); +// the wrap here is a defense-in-depth gap only for a raw/buggy +// embedder value, which the scalar path (used for non-bulk-eligible +// kinds, and reachable directly from embedder code) still catches. +// A TypedArray element write coerces exactly like the matching DataView +// setter (wraps mod 2^width), so this is pinned as intentional behavior +// (see bulk_list_test.ts), not merely undocumented; // * floats: the deterministic profile's NaN canonicalization on BOTH // directions (float.ts `decodeI32AsFloat` / `encodeFloatAsI32`): every // lifted NaN becomes the JS canonical NaN, every stored `number` NaN diff --git a/runtime/src/cabi/handles.ts b/runtime/src/cabi/handles.ts index 3d73cf2..ac0f463 100644 --- a/runtime/src/cabi/handles.ts +++ b/runtime/src/cabi/handles.ts @@ -6,11 +6,16 @@ // simplified here (pending the task machinery): // - canon_resource_* take the instance explicitly instead of reading // current_instance() from the running thread; -// - canon_resource_drop invokes the dtor as a direct call, where the -// reference routes it through store.lift/store.lower to get reentrance -// gating (may_enter checks) — deferred with the scheduler. +// - canon_resource_drop routes the dtor through `callDtorGated` below, +// which reconstructs the reference's store.lift/store.lower bracket +// (may_enter gating + trap poisoning) around the destructor call (#85). -import { assert_, trapIf } from "./trap.ts"; +import { assert_, Trap, trapIf } from "./trap.ts"; +import { + NeedsJspi, + notifyInstancePoisoned, + PendingCapability, +} from "../task/scheduler.ts"; import type { ComponentInstanceLike, LiftLowerContext, @@ -153,6 +158,168 @@ export function canonResourceNew( return inst.handles.add(h); } +/** + * The reentrance-gating half of `ComponentInstance` that a dtor call needs. + * `ResourceTypeInfo.impl` is typed as the deliberately-minimal `InstanceLike` + * (cabi must not depend on task/), so the gate is reached structurally; the + * concrete implementor is `task/mod.ts` `ComponentInstanceState`. + */ +interface ReentranceGate { + mayEnterFrom(caller: unknown): boolean; + enterFrom(caller: unknown): void; + leaveTo(caller: unknown): void; + handles: Iterable; + store?: { + pendingHostCalls: Set>; + hostFailure: unknown; + }; +} + +function asGate(x: unknown): ReentranceGate | null { + if (x === null || typeof x !== "object") return null; + const g = x as Partial; + return typeof g.mayEnterFrom === "function" && + typeof g.enterFrom === "function" && typeof g.leaveTo === "function" + ? (x as ReentranceGate) + : null; +} + +function isThenable(v: unknown): v is PromiseLike { + return typeof v === "object" && v !== null && + typeof (v as { then?: unknown }).then === "function"; +} + +/** + * Invoke a resource destructor through the reference's entry bracket. + * + * definitions.py `canon_resource_drop` (line 2318) does not call `rt.dtor` + * directly. It builds the dtor into a function instance and calls it through + * `Store.lift` / `Store.lower` (lines 2326-2333): + * + * ```python + * dtor = rt.dtor or (lambda rep: []) + * callee = inst.store.lift(dtor, ft, opts, rt.impl) + * caller = inst.store.lower(callee, ft, opts, inst) + * caller([h.rep]) + * ``` + * + * so the dtor inherits `Store.lift`'s gate verbatim (lines 579-584): + * `trap_if(not inst.may_enter_from(caller))`, `enter_from(caller)`, the call, + * then `leave_to(caller)` — which a trap skips, leaving the *implementing* + * instance permanently unenterable (poisoned). + * + * Two consequences that are easy to get wrong, both taken from the reference + * rather than from intuition: + * + * - the bracket runs even when `rt.dtor is None` (the `or (lambda rep: [])` + * above), so a dtor-less resource whose impl instance is mid-execution is + * still a trap. `may_enter_from`/`enter_from` walk `entering_set(caller)` + * (line 230), which is empty when the caller *is* the implementing + * instance — that, not a special case, is the same-instance exemption: + * a component dropping a handle to its own resource never traps. + * - poisoning applies to `rt.impl`, not to the dropping instance. The + * dropper's own bracket (its `Store.lift` frame) is broken by the same + * propagating trap at its own level; here only the callee is retired. + * + * Capability signals (`NeedsJspi`, `PendingCapability`) are not traps — see + * `isCapabilitySignal` in exec/boundary.ts — so they release the gate. + * + * `allowAsync` covers the host-initiated drop path (embedder/resources.ts): + * a dtor reached through a `promising` entry settles on a later turn, so the + * bracket is closed by the settle instead of synchronously. A *guest*- + * initiated drop must complete synchronously (the reference lifts the dtor + * with `async_ = False`), so a thenable there is a trap. + */ +export function callDtorGated( + rt: ResourceTypeInfo, + rep: number, + caller: unknown, + allowAsync = false, +): void { + const impl = asGate(rt.impl); + // A JS-initiated drop prefers the `promising`-wrapped entry when the + // executor wired one (#85: a dtor may legally reach a `Suspending` import + // on this path, so it needs a suspension-legal stack). Guest-initiated + // drops always take the raw synchronous dtor — see ResourceTypeInfo. + const dtorFn = allowAsync ? (rt.dtorHost ?? rt.dtor) : rt.dtor; + // No gate available: an imported (host-implemented) resource has + // `impl === null` by construction (executor.ts `bindImportedResources`), + // and there is no component instance to gate entry into. Test doubles that + // supply a bare `{handles, mayLeave}` instance land here too. + if (impl === null) { + const r = dtorFn?.(rep) as unknown; + trapIf( + !allowAsync && isThenable(r), + "resource destructor did not complete synchronously", + ); + return; + } + // definitions.py `entering_set` (line 230): `self_and_ancestors() - + // caller.self_and_ancestors()`. The caller is only meaningful when it is a + // real component instance; a host-initiated drop passes null, which is the + // reference's `caller = None` (Store.invoke). + const callerInst = asGate(caller) === null ? null : caller; + + trapIf(!impl.mayEnterFrom(callerInst), "cannot enter component instance"); + impl.enterFrom(callerInst); + + const poison = (e: unknown): void => { + if (e instanceof NeedsJspi || e instanceof PendingCapability) { + // Not a trap: the reference reaches `leave_to` on every execution these + // stand in for, so the instance stays enterable. + impl.leaveTo(callerInst); + return; + } + // `leave_to` is NOT reached (the gate stays taken, permanently), and the + // poisoned instance's live stream/future ends are retired (#66) through + // the same seam fact_calls.ts uses for its bracket-break sites. + notifyInstancePoisoned(impl, e); + }; + + let out: unknown; + try { + out = dtorFn?.(rep) as unknown; + } catch (e) { + poison(e); + throw e; + } + if (isThenable(out)) { + if (!allowAsync) { + // A guest-initiated drop is lifted with `async_ = False`: the dtor must + // resolve before `canon_resource_drop` returns. Reaching here means the + // dtor's activation escaped, which is a trap that poisons the impl. + const e = new Trap( + "resource destructor did not complete synchronously", + ); + poison(e); + throw e; + } + // Host-initiated async dtor: the entry bracket stays held until the + // destructor's activation actually finishes, which is what `Store.lift` + // does for a callee that blocks. Registered in `pendingHostCalls` so the + // driver counts it as externally-wakeable work and teardown can see it. + const store = impl.store; + const promise = Promise.resolve(out).then( + () => { + store?.pendingHostCalls.delete(promise); + impl.leaveTo(callerInst); + }, + (e) => { + store?.pendingHostCalls.delete(promise); + // The failure cannot propagate out of this microtask; the store's + // host-failure channel is where the driving call picks it up. + if (store !== undefined && store.hostFailure === undefined) { + store.hostFailure = e; + } + poison(e); + }, + ); + store?.pendingHostCalls.add(promise); + return; + } + impl.leaveTo(callerInst); +} + export function canonResourceDrop( inst: ComponentInstanceLike, rt: ResourceTypeInfo, @@ -166,9 +333,14 @@ export function canonResourceDrop( trapIf(rh.numLends !== 0, "handle still lent out"); if (rh.own) { assert_(rh.borrowScope === null); - // Reference: dtor invoked through store.lift/store.lower so that - // may_enter gating applies (cross-instance call). Deferred; direct call. - if (rt.dtor) rt.dtor(rh.rep); + // definitions.py line 2325-2333: the dtor runs through the store's + // lift/lower bracket. SCOPE NOTE (#85): the call below is a JS frame + // inside the drop trampoline, so a *guest*-initiated drop whose dtor + // suspends traps under the JSPI frame rule. That is deterministic and + // loud, and routing guest-initiated dtor calls through generated wasm is + // explicitly out of scope for #85 (docs/architecture.md §5/§7 carry the + // known-limitation note). + callDtorGated(rt, rh.rep, inst); } else { assert_(rh.borrowScope !== null); rh.borrowScope!.numBorrows -= 1; diff --git a/runtime/src/cabi/memory.ts b/runtime/src/cabi/memory.ts index 5adb84a..68cb79d 100644 --- a/runtime/src/cabi/memory.ts +++ b/runtime/src/cabi/memory.ts @@ -17,6 +17,20 @@ export function ptrSize(ptrType: PtrType): 4 | 8 { return ptrType === "i32" ? 4 : 8; } +/** + * `MemInst` caches its `Uint8Array`/`DataView` at construction time and never + * re-derives them. That is only sound for a memory that cannot grow within + * this instance's lifetime: `memory.grow` (guest-triggered or host-triggered) + * detaches the backing `ArrayBuffer`, and a cached view over a detached + * buffer reads/writes garbage rather than trapping. + * + * Production call/lift/lower paths do NOT construct `MemInst` directly against + * a live, growable `WebAssembly.Memory` for this reason: the executor + * re-derives a fresh view per access via `LiveMemory` + * (`exec/boundary.ts:97-160`), which is grow-safe. `MemInst` is for contexts + * where the buffer is known fixed for the duration (tests, or a snapshot + * already taken). + */ export class MemInst { readonly bytes: Uint8Array; readonly view: DataView; @@ -108,7 +122,32 @@ export function loadPtr(mem: MemInst, ptr: number): number | bigint { return mem.ptrSize() === 4 ? loadIntU(mem, ptr, 4) : loadIntU(mem, ptr, 8); } -// definitions.py store_int(cx, v, ptr, nbytes, signed). +// definitions.py store_int(cx, v, ptr, nbytes, signed): `int.to_bytes` raises +// `OverflowError` when `v` does not fit in `nbytes` (signed/unsigned per the +// flag) — a host-precondition violation, not a guest-visible trap (the value +// never came from validated guest bytes; it is a JS number/bigint an +// embedder handed the lowering path). Ported as `assert_`/`AssertionError` +// (see cabi/trap.ts's Trap-vs-AssertionError taxonomy), not `Trap`. +// definitions.py:1568-1569 (`store_int`). +// +// This scalar path is where the check lives; the bulk (TypedArray) path in +// bulk_lists.ts intentionally wraps instead — see that file's header for why. + +function bigintFitsIn64(v: bigint, signed: boolean): boolean { + return signed + ? v >= -(2n ** 63n) && v < 2n ** 63n + : v >= 0n && v < 2n ** 64n; +} + +function numberFitsInWidth(v: number, nbytes: 1 | 2 | 4, signed: boolean): boolean { + const bits = nbytes * 8; + if (signed) { + const min = -(2 ** (bits - 1)); + const max = 2 ** (bits - 1) - 1; + return v >= min && v <= max; + } + return v >= 0 && v <= 2 ** bits - 1; +} export function storeInt( mem: MemInst, @@ -120,11 +159,16 @@ export function storeInt( assert_(ptr + nbytes <= mem.length, "store out of bounds"); if (nbytes === 8) { assert_(typeof v === "bigint", "64-bit store requires bigint"); + assert_(bigintFitsIn64(v, signed), "int store: value out of range"); if (signed) mem.view.setBigInt64(ptr, v, true); else mem.view.setBigUint64(ptr, v, true); return; } assert_(typeof v === "number" && Number.isInteger(v), "int store"); + assert_( + numberFitsInWidth(v, nbytes, signed), + "int store: value out of range", + ); switch (nbytes) { case 1: if (signed) mem.view.setInt8(ptr, v); diff --git a/runtime/src/cabi/types.ts b/runtime/src/cabi/types.ts index c4a2707..d1705f9 100644 --- a/runtime/src/cabi/types.ts +++ b/runtime/src/cabi/types.ts @@ -54,11 +54,20 @@ export interface InstanceLike { /** * definitions.py `ResourceType`: identity + implementing instance + optional * destructor. Compared by object identity everywhere. + * + * `dtorHost` is the JS-initiated-drop variant of `dtor` (#85): in jspi mode + * the executor wires it as the `promising`-wrapped raw export (docs §7 — + * a host-initiated drop may legally reach a `Suspending` import, so it needs + * a suspension-legal entry), and `callDtorGated(allowAsync=true)` prefers it. + * Guest-initiated drops always use `dtor` directly: they must complete + * synchronously (reference lifts the dtor with `async_ = False`), and a + * promising wrapper would turn every such call into a thenable. */ export class ResourceTypeInfo { constructor( public impl: InstanceLike | null, public dtor: ((rep: number) => void) | null = null, + public dtorHost: ((rep: number) => unknown) | null = null, ) {} } diff --git a/runtime/src/embedder/instantiate.ts b/runtime/src/embedder/instantiate.ts index 19ec410..3df27ad 100644 --- a/runtime/src/embedder/instantiate.ts +++ b/runtime/src/embedder/instantiate.ts @@ -33,6 +33,7 @@ import { type GuestResourceSpec, HostResourceRegistry, invalidateWrapper, + lendWrapper, makeWrapper, takeRep, } from "./resources.ts"; @@ -421,7 +422,22 @@ class Facade { } return rep; } - return takeRep(v, false, `borrow<${b.name}>`); + // Host `own` wrapper lowered as `borrow` (#86): record the lend + // for the duration of this call, so a `drop()` or a GC finalization + // in the window cannot destroy a rep the guest still borrows. + // definitions.py `lift_borrow` -> `Subtask.add_lender` (line 890); + // `#lowerScope` is released where that subtask delivers its + // resolution, i.e. when the call ends. + const rep = takeRep(v, false, `borrow<${b.name}>`); + const release = lendWrapper(v as object); + if (self.#lowerScope === null) { + // No enclosing lowering scope (a raw/one-off lowering): the lend + // has no observable window, so it must not be left dangling. + release(); + } else { + self.#lowerScope.push(release); + } + return rep; }, }; } diff --git a/runtime/src/embedder/resources.ts b/runtime/src/embedder/resources.ts index b277b62..b3c1b4f 100644 --- a/runtime/src/embedder/resources.ts +++ b/runtime/src/embedder/resources.ts @@ -16,6 +16,7 @@ // | host passes borrow | wrapper stays valid | rep reused/allocated | import type { ResourceTypeInfo, ValType } from "../cabi/types.ts"; +import { callDtorGated } from "../cabi/handles.ts"; import { InvalidHandleError } from "./errors.ts"; import { camelCase, pascalCase } from "./casing.ts"; @@ -30,6 +31,29 @@ interface WrapperState { owns: boolean; rt: ResourceTypeInfo; className: string; + /** + * Host-side `ResourceHandle.num_lends` (#86). The reference models a + * host-held `own` as a table entry whose `num_lends` is bumped every time + * it is lifted as a `borrow` (definitions.py `Subtask.add_lender`, line + * 890, reached from `lift_borrow`, line 1516) and decremented when the + * borrowing call's subtask delivers its resolution (`deliver_resolve`, + * line 902). `lift_own` and `canon_resource_drop` both trap while it is + * non-zero (lines 1508 / 2325). + * + * Here the host holds bare reps rather than table entries, so the counter + * lives on the wrapper. Its lifecycle point is the *lowering scope* of the + * call the wrapper was passed into (`instantiate.ts` `#lowerParams`), which + * is released exactly when that call ends — the host-side analogue of the + * subtask's resolve delivery. + */ + lends: number; + /** + * A drop (explicit or via the GC backstop) that arrived while `lends > 0`. + * The reference would trap; the host has no frame to trap into by then, so + * the drop is deferred to the last release instead of running the dtor + * under a live guest borrow (which is the use-after-free #86 reports). + */ + pendingDrop: boolean; } /** Base of every runtime-built guest-resource class. */ @@ -51,16 +75,73 @@ export class GuestResource { * Backstop for leaked handles (docs/architecture.md §7). A wrapper that becomes unreachable * without `drop()` still runs the guest destructor — late, but not never. */ -const leaked = new FinalizationRegistry((s) => { - if (s.valid && s.owns) { - s.valid = false; - try { - s.rt.dtor?.(s.rep); - } catch { - // A destructor that traps during GC has nowhere to report to. - } +const runBackstop = (s: WrapperState): void => { + // Idempotence: `valid` is the single guard. A wrapper that was dropped, + // transferred, or invalidated already cleared it (and unregistered), so the + // backstop can neither double-run a dtor nor resurrect a dead rep. + if (!s.valid || !s.owns) return; + s.valid = false; + if (s.lends > 0) { + // A live guest borrow of this rep is outstanding (#86). Running the dtor + // now is exactly the use-after-free the reference forbids + // (definitions.py line 2325, `trap_if(h.num_lends != 0)`); the last + // `releaseLend` runs it instead. The closure held by the lowering scope + // keeps `s` alive, so the deferred drop is not lost with the wrapper. + s.pendingDrop = true; + return; } -}); + runHostDrop(s); +}; + +const leaked = new FinalizationRegistry(runBackstop); + +/** + * Simulate the GC backstop firing for `w` (the FinalizationRegistry callback, + * verbatim). Test seam: real GC finalization is unschedulable, and #86 is + * precisely about what the backstop does in a window a test must control. + * + * @internal + */ +export function simulateFinalizationForTest(w: object): void { + const s = wrapperState(w); + if (s !== undefined) runBackstop(s); +} + +/** + * Run a host-initiated drop of a guest `own` handle. + * + * The host holds a rep, never a table index, so there is nothing to remove + * from a handle table: the observable remainder of definitions.py + * `canon_resource_drop` for an owning handle is the gated dtor call + * (`callDtorGated`, cabi/handles.ts), with `caller = None` — a host-initiated + * call, `Store.invoke`'s `caller = None`. + * + * Never throws: the two callers are `drop()`/`[Symbol.dispose]()` — where a + * trap *is* reportable, so it propagates — and the FinalizationRegistry + * callback, where a throw would be swallowed by the engine with no + * diagnostic. `runHostDrop` is the latter's form: a trapping dtor poisons the + * implementing instance (which `callDtorGated` does) and is additionally + * recorded on the store's host-failure channel, so the next driven call + * surfaces it instead of silently continuing on a half-destroyed instance + * (#86, second defect: the former `catch {}`). + */ +function runHostDrop(s: WrapperState): void { + try { + callDtorGated(s.rt, s.rep, null, true); + } catch (e) { + recordHostFailure(s.rt, e); + } +} + +/** Park a failure that has no frame to propagate into on the store. */ +function recordHostFailure(rt: ResourceTypeInfo, e: unknown): void { + const store = (rt.impl as unknown as { + store?: { hostFailure: unknown }; + } | null)?.store; + if (store !== undefined && store.hostFailure === undefined) { + store.hostFailure = e; + } +} export function initWrapper( w: GuestResource, @@ -95,11 +176,59 @@ function dropWrapper(w: GuestResource): void { s.valid = false; leaked.unregister(w); if (!s.owns) return; // a borrow was never ours to drop - // Host-initiated drop of a guest handle. The host holds a rep, never a - // table index, so there is nothing to remove from a handle table: the - // observable half of definitions.py `canon_resource_drop` (line 2325) for an - // owning handle is exactly `rt.dtor(rep)`. - s.rt.dtor?.(s.rep); + if (s.lends > 0) { + // Lent out to an in-flight guest call (#86): defer rather than destroy a + // rep the guest still holds a `borrow` of. `drop(): void` stays + // non-blocking either way — the deferred dtor runs from `releaseLend`. + s.pendingDrop = true; + return; + } + // The dtor is entered through `callDtorGated`, which is also where a dtor + // that returns a Promise (a `promising`-entered dtor calling a `Suspending` + // import, docs/architecture.md §7) is tracked: the entry bracket is held + // until it settles and the promise is registered in the store's + // `pendingHostCalls`, so `drop()` itself never blocks. + // + // The `promising` entry itself is wired by exec/executor.ts (the `resource` + // initializer sets `ResourceTypeInfo.dtorHost` from the raw wasm export in + // jspi mode); `callDtorGated(allowAsync=true)` prefers it. In non-JSPI mode + // — or when the dtor resolved to a non-wasm callable, which `promising` + // rejects — this is a direct call, where a `Suspending` import was already + // unreachable from a dtor. + callDtorGated(s.rt, s.rep, null, true); +} + +/** + * Record that a host-held `own` wrapper was lowered as `borrow` into a + * guest call, and return the (idempotent) release for the end of that call. + * + * definitions.py: `lift_borrow` -> `Subtask.add_lender` (line 890) on the way + * in, `Subtask.deliver_resolve` (line 902) on the way out. + */ +export function lendWrapper(w: object): () => void { + const s = wrapperState(w); + if (s === undefined) return () => {}; + s.lends += 1; + let released = false; + return () => { + if (released) return; + released = true; + releaseLend(s); + }; +} + +function releaseLend(s: WrapperState): void { + s.lends -= 1; + if (s.lends > 0 || !s.pendingDrop) return; + s.pendingDrop = false; + // The drop that arrived while the handle was lent. `valid` is already + // false (both deferral sites clear it first), so nothing can race this. + runHostDrop(s); +} + +/** Host-side `num_lends` — diagnostics and white-box tests. */ +export function wrapperLends(w: object): number { + return wrapperState(w)?.lends ?? 0; } /** Invalidate a wrapper without dropping (used to end a borrow's lifetime). */ @@ -119,6 +248,14 @@ export function takeRep(w: unknown, own: boolean, what: string): number { } const s = requireLive(w, what); if (own) { + // definitions.py `lift_own` (line 1508): `trap_if(h.num_lends != 0)`. A + // handle currently lent to an in-flight call cannot be transferred away. + if (s.lends > 0) { + throw new InvalidHandleError( + `${what}: this ${s.className} handle is still lent out as a borrow ` + + `to an in-flight call and cannot be transferred`, + ); + } // Transfer: the wrapper is invalidated, and must NOT run the destructor. s.valid = false; leaked.unregister(w as GuestResource); @@ -200,6 +337,8 @@ export function buildGuestResourceClass( owns: true, rt, className, + lends: 0, + pendingDrop: false, }); } }; @@ -245,6 +384,8 @@ export function makeWrapper( owns, rt, className: cls.name ?? "resource", + lends: 0, + pendingDrop: false, }); return w; } diff --git a/runtime/src/embedder/streams.ts b/runtime/src/embedder/streams.ts index fd82a5b..87de984 100644 --- a/runtime/src/embedder/streams.ts +++ b/runtime/src/embedder/streams.ts @@ -276,7 +276,19 @@ export class Stream { return vs.map((v) => codec.toHost(v as ComponentValue)) as Chunk; } - /** Cancel an in-flight `read` (R-fix review advisory 1). */ + /** + * Cancel an in-flight `read` (R-fix review advisory 1). + * + * #97, DELIBERATE AND PINNED: the cancelled `read` resolves with whatever + * had already arrived — typically the empty chunk, which this layer also + * uses as end-of-stream (`read`'s contract, and hence `readable()` and the + * async iterator, which close on it). **A cancelled read is therefore + * indistinguishable from EOS at this layer.** Kept as-is rather than given + * a distinct signal: the caller of `cancelRead()` is the same code that + * observes the read's result, so it already knows which happened, and only + * that caller can reach the state. See exec/host_streams.ts + * `HostReadableEnd.cancelRead` for the mechanism. + */ cancelRead(): void { this.#host?.readable.cancelRead(); } @@ -435,6 +447,7 @@ export class Future implements PromiseLike { #hostP: Promise>; #codec: ElemCodec; #consumed = false; + #dropped = false; #settled: Promise | null = null; private constructor( @@ -543,14 +556,34 @@ export class Future implements PromiseLike { else void this.#hostP.then((h) => h.cancel()); } + /** + * Release this future handle. Total and idempotent (#90): it never throws, + * and calling it twice — or after `Symbol.dispose` — is a no-op. + * + * Dropping a future the host never wrote to, once the guest already holds + * its readable end, is **abandonment**: the guest's reader can never be + * satisfied, so it is armed with a trap at its rendezvous point rather than + * being handed a value-less completion (exec/host_streams.ts + * `HostFuture.drop`, task/streams.ts `abandonSharedFuture`; the spec keeps + * that state unreachable by trapping the early writable drop, + * definitions.py:1183-1184). Write-then-drop is the normal path and is + * unaffected; a future no guest ever saw is plain cleanup. + */ drop(): void { + if (this.#dropped) return; + this.#dropped = true; if (this.#host !== null) this.#host.drop(); - else void this.#hostP.then((h) => h.drop()); + // A deferred future whose host end never materialized has nothing to + // release; swallow that rejection rather than let `drop()` produce an + // unhandled one. + else void this.#hostP.then((h) => h.drop(), () => {}); } /** @internal — see `Stream.dropForTeardown` (#66). */ dropForTeardown(): void { + if (this.#dropped) return; if (this.#host !== null) { + this.#dropped = true; dropSharedForTeardown(this.#host.value as never); } else { // A deferred future (still in flight) cannot be an import argument; @@ -728,15 +761,18 @@ export function lowerFutureSource( // store's host-failure channel instead, exactly as for streams, so the // in-flight call fails with a site-named error. // - // And then we do NOT drop: a host future's write end dropping while the - // guest's readable end is parked trips an internal invariant in the - // future built-ins ("a readable future end cannot observe DROPPED", - // intrinsics/stream_builtins.ts) — a runtime-core matter outside this - // layer. Leaving the guest parked is harmless because the failure is - // already recorded: the driving loop of the call raises it before the - // call can complete. Only when there is NO store to report to (the - // future was never lowered) do we fall back to dropping, so nothing can - // hang forever. + // And then we do NOT drop -- for ATTRIBUTION, not for safety. Dropping + // here is now well-defined (#90: an unwritten, lowered future's drop + // abandons it and the guest reader traps at its rendezvous point, + // exec/host_streams.ts `HostFuture.drop`); the stale version of this + // comment claimed it would trip an internal invariant, which was true + // before the abandonment mechanism existed and is not true now. + // Reporting instead of dropping is still the better outcome: the + // store-level failure names the producer and the site, so the in-flight + // call fails with the real cause rather than with a generic + // "the writable end went away" trap. Only when there is NO store to + // report to (the future was never lowered) do we fall back to dropping, + // so nothing can hang forever. const reported = reportProducerFailure( { value: host.value } as unknown as HostStream, codec.where ?? "future producer", diff --git a/runtime/src/exec/boundary.ts b/runtime/src/exec/boundary.ts index 8172a0b..17a66ba 100644 --- a/runtime/src/exec/boundary.ts +++ b/runtime/src/exec/boundary.ts @@ -1600,7 +1600,18 @@ export function createLoweredImport(input: { // guest code in an unattributed chunk — the issue-#24 class the // attribution sentinels exist to prevent. let outcome: { value: unknown } | { error: unknown } | undefined; - const promise = Promise.resolve(raw).then( + // The async arm runs `onResolve` — result lowering, including possible + // realloc re-entry into the guest — in this bare promise continuation, + // where the sync arm above defers all CABI work to `produce` (the + // issue-#24 attribution note). The asymmetry is deliberate (#93): here + // no wasm frame is suspended mid-call — the guest returned BLOCKED and + // is between activations, which is exactly when the reference's + // `on_resolve` runs (the callee's turn), so there is no activation for + // the sentinels to attribute this chunk to. Lowering failures are host + // failures, not guest traps: they land on `store.hostFailure` and the + // driving loop raises them site-named (pinned by + // tests/async_lower_onresolve_failure_test.ts). + const promise = Promise.resolve(raw).then( (v) => { store.pendingHostCalls.delete(promise); outcome = { value: v }; diff --git a/runtime/src/exec/executor.ts b/runtime/src/exec/executor.ts index f8f86b9..f5b7a3c 100644 --- a/runtime/src/exec/executor.ts +++ b/runtime/src/exec/executor.ts @@ -17,6 +17,7 @@ import { assertModeConsistent, type SuspendingImport, chooseMode, + enterWasm, isSuspending, planNeedsSuspension, suspendingImport, @@ -548,6 +549,18 @@ class Executor { // suspension point; every function it exports is therefore // potentially-blocking, and everything else is not. this.sawBlockingImport = false; + // ISSUE #88: core wasm permits two imports with the same + // (module, field) pair (trusted wasmtime-environ 47.0.3 info.rs + // :438-445 gives one flat positional CoreDef per import slot, but + // WebAssembly.Module.imports(module) and the JS import object are + // both keyed by (module, field) name, not by slot). If two slots + // share a name and resolve to different values, the second object + // write silently wins and BOTH slots receive the last value — the + // JS API cannot express per-slot values for duplicate names. Detect + // this here and fail loudly rather than wire the wrong function in + // silently; identical values are safe (the API cannot distinguish + // the slots in that case, so nothing is actually lost). + const seenAt = new Map(); declared.forEach((imp, i) => { const before = this.sawBlockingImport; const value = this.importValue(init.args[i]); @@ -560,6 +573,19 @@ class Executor { `${imp.module}.${imp.name} (${JSON.stringify(init.args[i])})`, ); } + const key = `${imp.module}\0${imp.name}`; + const prior = seenAt.get(key); + if (prior !== undefined && prior.value !== value) { + throw new PlanError( + `module ${init.module}: duplicate import ` + + `${JSON.stringify(imp.module)}.${JSON.stringify(imp.name)} ` + + `at arg indices ${prior.index} and ${i} resolve to ` + + `different values (plan args[${prior.index}]=` + + `${JSON.stringify(init.args[prior.index])}, args[${i}]=` + + `${JSON.stringify(init.args[i])})`, + ); + } + seenAt.set(key, { index: i, value }); (importObject[imp.module] ??= {} as WebAssembly.ModuleImports)[imp.name] = value as WebAssembly.ImportValue; @@ -670,6 +696,36 @@ class Executor { token.dtor = dtor === null ? null : (rep: number) => { dtor(rep); }; + // #85: the JS-initiated-drop variant. In jspi mode a dtor may + // legally reach a `Suspending` import (docs §7), which needs a + // `promising` entry — only this module holds the raw export. + // Wrapped ONLY when the dtor is suspension-capable + // (`suspendableFuncs`: its core instance imports a blocking + // trampoline): `promising` settles on a later microtask even + // for a non-suspending activation (jspi pin (j)), which would + // leave the impl instance entered for a turn after every + // drop — a synchronous drop-then-call sequence would trap. A + // non-suspendable dtor cannot legally suspend, so the sync + // path is exact for it. `WebAssembly.promising` also rejects + // non-wasm callables (a dtor CoreDef can resolve to a JS + // trampoline) with a TypeError; fall back to the sync closure + // — pre-#85 behavior, where a suspension is a deterministic + // frame-rule trap. Deliberately does NOT set `wrappedEntries`: + // `finish()`'s invariant inventories the two primary wrapping + // sites; this is an auxiliary entry. + if ( + dtor !== null && this.suspensionMode === "jspi" && + this.suspendableFuncs.has(dtor as unknown as object) + ) { + try { + token.dtorHost = enterWasm( + dtor as (rep: number) => unknown, + this.suspensionMode, + ); + } catch { + token.dtorHost = null; + } + } } }); break; @@ -1018,10 +1074,24 @@ class Executor { } return this.loaded.streamElems[i]; }, - streamTableInstance: (i) => - this.componentInstance(this.loaded.streamTableInstances[i] ?? 0), - futureTableInstance: (i) => - this.componentInstance(this.loaded.futureTableInstances[i] ?? 0), + streamTableInstance: (i) => { + const instance = this.loaded.streamTableInstances[i]; + if (instance === undefined) { + throw new PlanError( + `stream table ${i} is not in the plan's streamTables (plan v2)`, + ); + } + return this.componentInstance(instance); + }, + futureTableInstance: (i) => { + const instance = this.loaded.futureTableInstances[i]; + if (instance === undefined) { + throw new PlanError( + `future table ${i} is not in the plan's futureTables (plan v2)`, + ); + } + return this.componentInstance(instance); + }, futureElem: (i) => { if (i >= this.loaded.futureElems.length) { throw new PlanError( diff --git a/runtime/src/exec/host_streams.ts b/runtime/src/exec/host_streams.ts index 4a87407..e5e0d6e 100644 --- a/runtime/src/exec/host_streams.ts +++ b/runtime/src/exec/host_streams.ts @@ -62,6 +62,8 @@ import { whenStoreDriverIdle, } from "./boundary.ts"; import { + abandonSharedFuture, + BUFFER_MAX_LENGTH, type ComponentInstanceState, CopyResult, type PayloadChunk, @@ -110,7 +112,25 @@ export class HostBuffer { readonly t: ValType | null, private readonly values: PayloadChunk | null, readonly length: number, - ) {} + ) { + // definitions.py `Buffer.MAX_LENGTH` (:919) is asserted on every buffer + // the spec builds (`BufferGuestImpl.__init__`, :938); `GuestBuffer` traps + // on it. A host buffer is not guest-visible, so a violation is embedder + // misuse rather than a component fault — hence a loud typed JS error and + // not a `Trap`. Caught at construction: an over-long host offer would + // otherwise silently exceed the spec bound (#97). + if (!Number.isInteger(length) || length < 0) { + throw new RangeError( + `host buffer length must be a non-negative integer, got ${length}`, + ); + } + if (length > BUFFER_MAX_LENGTH) { + throw new RangeError( + `host buffer length ${length} exceeds the Component Model's ` + + `Buffer.MAX_LENGTH (${BUFFER_MAX_LENGTH})`, + ); + } + } remain(): number { return this.length - this.progress; @@ -443,6 +463,7 @@ export interface HostStream { function bindOnLower( shared: SharedStreamImpl | SharedFutureImpl, activity: HostActivity, + alsoOnLowered?: () => void, ): void { const holder = shared as unknown as { onLowered?: ((i: ComponentInstanceState) => void) | null; @@ -460,7 +481,10 @@ function bindOnLower( "internal: a second host wrapper was built for an already-wrapped " + "stream/future (the wrapper cache should have returned the first)", ); - holder.onLowered = (inst) => activity.bind(inst.store); + holder.onLowered = (inst) => { + alsoOnLowered?.(); + activity.bind(inst.store); + }; // A stream that came *out* of a guest was lifted, never lowered, so the hook // above will not fire; `boundStore` was recorded at lift time instead. const bound = (shared as { boundStore?: unknown }).boundStore; @@ -607,6 +631,16 @@ function mkStreamEnds( }); }, cancelRead() { + // #97, DELIBERATE AND PINNED: cancelling resolves the in-flight + // `read` promise with whatever the buffer took so far — for a read + // that had not yet rendezvoused, the empty chunk. An empty chunk is + // also this layer's end-of-stream signal (see `HostReadableEnd.read` + // and embedder/streams.ts `Stream.read`), so **a host-cancelled read + // is indistinguishable from EOS at the conventions layer**. That is + // accepted rather than papered over: the code that calls + // `cancelRead()` is the same code that observes the result, so it + // already knows which of the two happened. Nothing else can reach + // this state — a guest cannot cancel the host's read. if (!parked.read) return; parked.read = false; shared.cancel(); @@ -681,6 +715,23 @@ export interface HostFuture { readResult(): Promise<{ value: T | undefined; result: CopyResult }>; /** Cancel an in-flight `read`/`write`; see `HostWritableEnd.cancelWrite`. */ cancel(): void; + /** + * Release this future. Total and idempotent (#90): it never throws, and a + * second call is a no-op. + * + * Three cases, per the #90 ruling: + * + * * the value was already delivered (the normal write-then-drop path) — + * plain state cleanup, the spec's `WritableFutureEnd.drop` precondition + * (definitions.py:1183-1184) is satisfied; + * * never written, and the future was **lowered** into a guest (the guest + * holds the readable end, so this wrapper plays the spec's writable + * role) — *abandon*: the reader can never be satisfied, so it is armed + * with the rendezvous-point trap (task/streams.ts `abandonSharedFuture`) + * rather than being handed a DROPPED it may not observe; + * * never written and never lowered — no guest ever saw it; plain + * cleanup. + */ drop(): void; value: ComponentValue; } @@ -689,11 +740,13 @@ export interface HostFuture { export function hostFuture(element: ValType | null): HostFuture { const shared = new SharedFutureImpl(element); const activity = new HostActivity(); - bindOnLower(shared, activity); + const lowering = { lowered: false }; + bindOnLower(shared, activity, () => lowering.lowered = true); const wrapper = mkFuture( shared, activity, shared as unknown as ComponentValue, + lowering, ); futureWrappers.set(shared, wrapper as HostFuture); return wrapper; @@ -712,8 +765,9 @@ export function hostFutureFor(value: ComponentValue): HostFuture { const cached = futureWrappers.get(shared); if (cached !== undefined) return cached as HostFuture; const activity = new HostActivity(); - bindOnLower(shared, activity); - const wrapper = mkFuture(shared, activity, value); + const lowering = { lowered: false }; + bindOnLower(shared, activity, () => lowering.lowered = true); + const wrapper = mkFuture(shared, activity, value, lowering); futureWrappers.set(shared, wrapper as HostFuture); return wrapper; } @@ -722,13 +776,22 @@ function mkFuture( shared: SharedFutureImpl, activity: HostActivity, value: ComponentValue, + /** + * Flipped by `bindOnLower` the first time this future is lowered into a + * guest — i.e. the first time a guest receives its READABLE end and this + * wrapper takes on the spec's writable role. `drop()` needs it (#90). + */ + lowering: { lowered: boolean }, ): HostFuture { // Distinct rendezvous identities per end — see `hostEndInstance`. const writeInst = hostEndInstance("write"); const readInst = hostEndInstance("read"); const parked = { any: false }; + /** Set once the future's one value has actually crossed (#90). */ + let delivered = false; const settle = (result: CopyResult): void => { parked.any = false; + if (result === CopyResult.COMPLETED) delivered = true; if (result === CopyResult.DROPPED) activity.close(); else activity.notify(); }; @@ -797,7 +860,20 @@ function mkFuture( activity.pump(); }, drop() { - shared.drop(); + // #90. Never throws, idempotent: `SharedFutureImpl.drop` and + // `abandonSharedFuture` both no-op on an already-dropped future, and + // neither can raise. See the `HostFuture.drop` doc for the three cases. + if (!delivered && lowering.lowered && !shared.dropped) { + abandonSharedFuture( + shared, + new Error( + "the host dropped the writable end of this future without " + + "writing a value", + ), + ); + } else { + shared.drop(); + } activity.close(); activity.pump(); }, diff --git a/runtime/src/intrinsics/async_builtins.ts b/runtime/src/intrinsics/async_builtins.ts index 9b87d1d..3aa6e0e 100644 --- a/runtime/src/intrinsics/async_builtins.ts +++ b/runtime/src/intrinsics/async_builtins.ts @@ -476,13 +476,22 @@ export function createSubtaskCancel( // SITE 5 (lit): a sync `subtask.cancel` blocks until the callee // actually resolves (definitions.py `canon_subtask_cancel`), then // reports the resolved state through the same tail as the - // non-blocking path. + // non-blocking path. Mirrors SITE 4 (stream_builtins.ts:305-323) + // and `Waitable.waitForPendingEvent` (definitions.py:786-790, + // reached from canon_subtask_cancel :2491): `hasSyncWaiter` must + // be set for the duration so a concurrent `waitable.join` on + // this subtask traps (async_builtins.ts:362-365) instead of + // racing the SUBTASK event away from this resume (#87). + st.hasSyncWaiter = true; return blockCurrentActivation({ store: inst.store, task: currentTask(), - readyFunc: () => st.resolved(), + readyFunc: () => st.hasPendingEvent(), cancellable: false, - produce: () => finish(), + produce: () => { + st.hasSyncWaiter = false; + return finish(); + }, }) as unknown as number; } needsJspi( @@ -505,6 +514,11 @@ export function createSubtaskCancel( // genuine BLOCKED answer is still immediate. Host-import subtasks // carry no callee task: their onCancel is a no-op and their state // cannot be mid-hop, so the pre-jspi immediate answer stands. + // + // NAMED DIVERGENCE (docs/architecture.md §6, #92): this park makes + // the async built-in non-atomic — other ready threads may run while + // it waits, a reordering within the reference's Store.tick freedom + // taken one built-in early. if (mode === "jspi" && st.calleeTask !== null) { const t = st.calleeTask as { threads: { done(): boolean }[]; diff --git a/runtime/src/intrinsics/fact_calls.ts b/runtime/src/intrinsics/fact_calls.ts index e151fbb..9473bc1 100644 --- a/runtime/src/intrinsics/fact_calls.ts +++ b/runtime/src/intrinsics/fact_calls.ts @@ -507,6 +507,18 @@ function mkCalleeTask(input: { if (postReturn !== null) { assert_(inst.mayLeave, "post-return with may_leave already false"); inst.mayLeave = false; + // NO local try/finally here, deliberately (#91, verified rather than + // assumed). definitions.py `canon_lift` (lines 2170-2174) has the + // same bare bracket: a trapping post-return skips `may_leave = True` + // and, since `Store.lift`'s `leave_to` is also skipped, leaves the + // instance poisoned — restoring `may_leave` locally would contradict + // both. What this runtime additionally needs, because it supports + // post-trap re-entry, is that no *live* instance is stranded with + // `may_leave === false`; exec/boundary.ts `unwind` covers exactly + // that: at the host boundary no lift or lower is in flight, so it + // asserts that resting state for every instance outside the poisoned + // entered set. This instance is either in that set (poisoned, left + // as the trap left it) or restored there. callCore(postReturn, raw as CoreValue[]); inst.mayLeave = true; ctx.stats.postReturnsRun++; @@ -649,6 +661,15 @@ export function createSyncStartCall( // component's callee would otherwise strand its host peers. notifyInstancePoisoned(prepared.calleeInst, e); } + // The lent handles are the CALLER's, and the caller is not poisoned by + // either exit (contracts/intrinsics.md v0.2 amendment 2: this runtime + // deliberately supports post-trap re-entry on the caller side, where + // the reference kills the whole store, so the sync-call scopes it + // skipped have to be unwound explicitly). Leaving `numLends` elevated + // would make every later `lift_own`/`resource.drop` of those handles + // trap "handle still lent out" (#91). Release is idempotent, and the + // success path below is unchanged. + lenderScope.releaseLenders(); throw e; } if (ok) prepared.calleeInst.leaveTo(prepared.callerInst); @@ -694,6 +715,10 @@ export function createSyncStartCall( }, }); } + // A capability signal is expressly NON-poisoning (see above), so + // stranding the caller's lenders here is strictly worse than on the + // trap path: the caller is guaranteed to keep running (#91). + lenderScope.releaseLenders(); needsJspi( "sync-start-call whose async-lifted callee did not resolve in its " + "first activation (the caller's wasm frame must block)", @@ -704,6 +729,30 @@ export function createSyncStartCall( }; } +/** + * Release a never-delivered subtask's lenders after a trap or capability bail + * broke the `[async-start-call]` bracket (#91). + * + * The reference has no analogue because it never resumes after a trap: the + * store dies with the lent handles inside it. contracts/intrinsics.md v0.2 + * amendment 2 makes the unwind this runtime's obligation instead. + * + * The resolution state mirrors `canon_lower`'s `on_resolve(None)` branch + * (definitions.py line 2267): CANCELLED_BEFORE_STARTED if the callee never + * started, CANCELLED_BEFORE_RETURNED otherwise. + */ +function unwindSubtaskLenders(subtask: Subtask): void { + if (!subtask.resolved()) { + subtask.resolve( + subtask.state === SubtaskState.STARTING + ? SubtaskState.CANCELLED_BEFORE_STARTED + : SubtaskState.CANCELLED_BEFORE_RETURNED, + [], + ); + } + if (!subtask.resolveDelivered()) subtask.deliverResolve(); +} + /** The core-ABI shape of a returned results vector (0 / 1 / many). */ function shapeResults(out: CoreValue[] | null): CoreValue | undefined { if (out === null || out.length === 0) return undefined; @@ -825,6 +874,14 @@ export function createAsyncStartCall( // as in the sync form above. notifyInstancePoisoned(prepared.calleeInst, e); } + // The subtask never reached `report()`, so it has no handle in the + // caller's table and nothing will ever deliver its resolution — but it + // holds `num_lends` on the caller's handles. Resolve it as cancelled + // (the state the reference's `on_resolve(None)` would give a call that + // never started/returned) and deliver, which is what releases the + // lenders (definitions.py `Subtask.deliver_resolve`, line 902). See the + // sync form above for why the caller must not be left holding them. + unwindSubtaskLenders(subtask); throw e; } if (ok) prepared.calleeInst.leaveTo(prepared.callerInst); diff --git a/runtime/src/intrinsics/mod.ts b/runtime/src/intrinsics/mod.ts index 3e609f1..7a1d06c 100644 --- a/runtime/src/intrinsics/mod.ts +++ b/runtime/src/intrinsics/mod.ts @@ -445,18 +445,80 @@ function createTrampolineBody( // Sync-call task bookkeeping (intrinsics.md §A: "degenerate-case // implementation in M0: assert-and-count"). wasmtime 47 signatures: - // enter-sync-call/exit-sync-call take no wasm-visible arguments that we - // act on in M0; balance is asserted at component teardown by tests. + // enter-sync-call carries the caller/callee instance pair, which is what + // the reentrance gate below needs; balance of the bracket is asserted at + // component teardown by tests. // Signatures (wasmtime-environ 47.0.3 `fact.rs:743,754`): // async.enter-sync-call(caller_instance: i32, async: i32, // callee_instance: i32) -> () // async.exit-sync-call() -> () case "enter-sync-call": return ( - _callerInstance?: number, + callerInstance?: number, async_?: number, - _calleeInstance?: number, + calleeInstance?: number, ) => { + // Reference reentrance gate. Every guest->guest call in + // definitions.py routes through the callee's lift wrapper: + // canon_lower (line 2312) calls + // `callee(on_start, on_resolve, caller = thread.task.inst)`, + // and `callee` is `Store.lift`'s `func_inst` (lines 578-585), whose + // first act is + // `trap_if(not inst.may_enter_from(caller))` (line 581) + // with `entering_set(caller) = callee.self_and_ancestors() + // - caller.self_and_ancestors()` + // (lines 230-234). + // A sync fused adapter is an *optimization* of that path, so the gate + // belongs here (issue #99). + // + // Note on the shape of the entering set, which is what makes this + // check safe for the legal shapes: + // * caller == callee, or either an ancestor of the other -> the + // entering set is empty and this never traps. Those pairs never + // reach this trampoline anyway: FACT emits an unconditional + // `CannotEnterComponent` trap for them at compile time + // (wasmtime-environ 47.0.3 `fact/trampoline.rs:120-127`), which + // is what `test/async/trap-on-reenter.wast` cases 2 and 3 pin. + // * an *idle* sibling -> `mayEnter` is true, no trap. This is what + // `test/async/sync-barges-in.wast` needs: an async callee that is + // merely blocked has already run `leave_to` (its `canon_lift` + // returned), so a sync sibling may barge in. + // * an *entered* sibling -> trap, which is the A -> C -> A cycle. + // + // CONTRACT / reachability: a pure guest-to-guest sibling cycle is + // unreachable by construction, because component instance imports + // form a DAG (a callee must be instantiated before its caller, so it + // cannot hold an import of its caller; `wasm-tools` rejects the + // mutual-import composition outright). wasmtime relies on exactly + // that to elide the runtime check in fused adapters -- see the + // comment in `may_enter`, wasmtime 47.0.3 + // `runtime/component/concurrent.rs:1876-1886`, and + // `enter_guest_sync_call` (concurrent.rs:1723) which performs no + // reentrance check at all. The gate is kept anyway because the + // reference mandates it and no corpus test pins the permissive + // behaviour; it is cheap, and it is the honest place for the + // invariant to be asserted rather than assumed. + // + // Deliberately *not* done here: `enter_from` / `leave_to` around the + // bracket. The reference locks the callee for the duration, which + // would additionally trap host-mediated reentrance (host -> A.f -> + // C.g -> host import -> host invokes C.g). Doing it needs the trap / + // capability-signal distinction that only `exec/boundary.ts` has + // (a `NeedsJspi` bail out of a sync callee skips `exit-sync-call` + // and would poison a healthy instance), and the general form of that + // hole is the missing `ComponentInstance.parent` chain already + // recorded in `task/mod.ts` `enteringSet`. Reported, not smuggled in. + if ( + typeof callerInstance === "number" && + typeof calleeInstance === "number" + ) { + const callerInst = ctx.componentInstance(callerInstance >>> 0); + const calleeInst = ctx.componentInstance(calleeInstance >>> 0); + trapIf( + !calleeInst.mayEnterFrom(callerInst), + "cannot enter component instance", + ); + } // `async_` records whether the callee is *async-lifted*. wasmtime // stores it on the guest task it creates here // (`concurrent.rs:1723` `enter_guest_sync_call`, whose `callee_async` diff --git a/runtime/src/intrinsics/stream_builtins.ts b/runtime/src/intrinsics/stream_builtins.ts index dc2442e..1b7fe56 100644 --- a/runtime/src/intrinsics/stream_builtins.ts +++ b/runtime/src/intrinsics/stream_builtins.ts @@ -26,6 +26,7 @@ import { LiftLowerContext } from "../cabi/context.ts"; import { loadStringFromRange, storeString } from "../cabi/strings.ts"; import type { ValType } from "../cabi/types.ts"; import { + abandonReasonOf, BUFFER_MAX_LENGTH, type ComponentInstanceState, CopyEnd, @@ -36,6 +37,7 @@ import { ErrorContext, EventCode, type EventTuple, + futureAbandonTrap, GuestBuffer, needsJspi, ReadableFutureEnd, @@ -262,6 +264,29 @@ function futureCopy(input: { end.state = CopyState.COPYING; const onCopyDone = (result: CopyResult) => { + // #84/#90: an unwritten future whose writable side was torn down (a + // trap-poisoned instance's table, or the host's `drop()` door) can never + // satisfy this reader. definitions.py keeps that state unreachable + // (:1183-1184 traps the early writable drop, so :2614 may assert a + // readable end never sees DROPPED); where we bypass the trap we owe the + // reader a *trap at its rendezvous point* instead of a DROPPED answer. + // + // The pending event stays a thunk, so the trap is raised exactly where + // the reader observes it: `waitable-set.wait`'s delivery (both the + // fast-path and the JSPI `produce`, intrinsics/async_builtins.ts:290/311), + // the callback loop's `waitForEventAnd` (exec/boundary.ts:1766), and + // `finishCopy`'s `take()` below — every one of which is inside the + // reader's guest activation, so the throw propagates as that task's trap + // and poisons *its* instance, and nothing else. + const abandoned = reading && result === CopyResult.DROPPED + ? abandonReasonOf(end.shared) + : null; + if (abandoned !== null) { + end.setPendingEvent((): EventTuple => { + throw futureAbandonTrap(abandoned); + }); + return; + } assert_( result !== CopyResult.DROPPED || eventCode === EventCode.FUTURE_WRITE, "a readable future end cannot observe DROPPED", diff --git a/runtime/src/intrinsics/transcode.ts b/runtime/src/intrinsics/transcode.ts index 9ddb5b1..8fec5a2 100644 --- a/runtime/src/intrinsics/transcode.ts +++ b/runtime/src/intrinsics/transcode.ts @@ -224,6 +224,39 @@ function snapshot(bytes: Uint8Array, ptr: number, len: number): Uint8Array { return bytes.slice(ptr, ptr + len); } +/** + * O(1) defensive counterpart to wasmtime's `assert_no_overlap` + * (libcalls.rs:166-177): traps (does not merely assert) because this + * replaces a guarantee FACT's trampoline construction is supposed to + * provide — src/dst are always independently-allocated regions — so a hit + * here means that guarantee broke, which is guest-memory-corruption-class + * severity, not an internal invariant a caller controls. + * + * Applied only where a call reads and writes through the SAME backing + * `Uint8Array` while interleaving reads and writes (byte-range comparison, + * not per-element — O(1) per call). Ops that first `snapshot()` the source + * into an independent copy (transcode.ts's `snapshot`, used by every op + * above that decodes-then-writes) already break aliasing before the first + * write, so they are exempt by construction and do not call this. + */ +function trapIfOverlap( + src: Uint8Array, + srcPtr: number, + srcLen: number, + dst: Uint8Array, + dstPtr: number, + dstLen: number, +): void { + if (src.buffer !== dst.buffer) return; // different memories: cannot overlap + const srcStart = src.byteOffset + srcPtr; + const srcEnd = srcStart + srcLen; + const dstStart = dst.byteOffset + dstPtr; + const dstEnd = dstStart + dstLen; + if (srcStart < dstEnd && dstStart < srcEnd) { + trap("transcode src/dst regions overlap"); + } +} + // --------------------------------------------------------------------------- // The twelve operations // --------------------------------------------------------------------------- @@ -332,6 +365,18 @@ export function createTranscoder( case "utf16-to-latin1": return (srcPtr, srcLen, dstPtr) => { const src = from.bytes(); + const dst = to.bytes(); + // This op does not call `snapshot()` (unlike its siblings above): + // it reads the full `out` prefix before writing anything to `dst`, + // which is the same aliasing-safety property snapshot() buys + // elsewhere, just via a builder array instead of a byte copy. The + // overlap guard is still added here (O(1): a byte-range compare, not + // per-element) as the one op in this file that is safe by algorithm + // shape rather than by an explicit `snapshot()` call — cheap + // insurance against that reasoning becoming stale under a future + // edit (docs/architecture.md §7; wasmtime asserts overlap on every + // op unconditionally, libcalls.rs:166-177). + trapIfOverlap(src, srcPtr, 2 * srcLen, dst, dstPtr, srcLen); const view = new DataView(src.buffer, src.byteOffset, src.byteLength); // Note: no surrogate validation here, matching wasmtime — a surrogate // is simply > 0xFF and ends the latin1 prefix. @@ -341,7 +386,6 @@ export function createTranscoder( if (u > 0xff) break; out.push(u); } - const dst = to.bytes(); for (let i = 0; i < out.length; i++) dst[dstPtr + i] = out[i]; return [out.length, out.length]; }; @@ -396,11 +440,24 @@ export function createTranscoder( // (srcPtr, srcLen, dstPtr, dstLen, latin1BytesSoFar) -> dstUnits ------- case "utf8-to-compact-utf16": - return (srcPtr, srcLen, dstPtr, _dstLen, latin1Bytes) => { + return (srcPtr, srcLen, dstPtr, dstLen, latin1Bytes) => { const s = decodeUtf8OrTrap(snapshot(from.bytes(), srcPtr, srcLen)); const dst = to.bytes(); inflateLatin1Bytes(dst, dstPtr, latin1Bytes); const view = new DataView(dst.buffer, dst.byteOffset, dst.byteLength); + // Defensive dst-capacity guard: wasmtime's equivalent + // (`run_utf8_to_utf16`'s `.zip(dst)`, libcalls.rs:308-312) is bounded + // by Rust's `Iterator::zip` truncating to the shorter of the two — + // it can never overrun `dst`. FACT is supposed to size `dstLen` to + // always have room (a full re-encode of a string that was already + // partially latin1-encoded never needs more u16 units than + // `dstLen - latin1Bytes`), so this should be unreachable; trap + // rather than let a broken caller corrupt guest memory past `dst`'s + // bound or silently truncate. + const capacity = dstLen - latin1Bytes; + if (s.length > capacity) { + trap("utf8-to-compact-utf16: destination capacity exceeded"); + } let units = 0; for (let i = 0; i < s.length; i++) { view.setUint16( diff --git a/runtime/src/jspi/bridge.ts b/runtime/src/jspi/bridge.ts index 3814418..0fed31a 100644 --- a/runtime/src/jspi/bridge.ts +++ b/runtime/src/jspi/bridge.ts @@ -460,6 +460,12 @@ export class SuspensionPoint implements SchedulableThread { /** Settle the import's Promise; the engine resumes the wasm activation. */ resume(cancelled: Cancelled = false): void { assert_(!this.#done, "resume of an already-resumed suspension point"); + // Mirrors task/thread.ts:187-190 (definitions.py:367 `Thread.resume`): + // a cancelled resume is only legal at a cancellable block point (#93). + assert_( + this.cancellable || !cancelled, + "cancelled resume of a non-cancellable suspension point", + ); if (SP_TRACE) { console.error(`[sp] resume ${dbgId(this)} owner=${dbgId(this.owner)}\n${(new Error().stack ?? "").split("\n").slice(2, 5).join("\n")}`); } diff --git a/runtime/src/plan/format.ts b/runtime/src/plan/format.ts index 5b761f8..8633de7 100644 --- a/runtime/src/plan/format.ts +++ b/runtime/src/plan/format.ts @@ -24,11 +24,19 @@ export interface WirePlan { * Stream-table metadata (plan v2), index space == wasmtime's * `TypeStreamTableIndex`; referenced by the `streamTable` field of every * `stream.*` trampoline. `element` is the `T` of `stream`, `null` for the - * zero-width payload. Optional on the wire so a v1 plan still parses. + * zero-width payload. + * + * ISSUE #94(2): the shim never `skip_serializing_if`s this field + * (crates/translator-shim/src/plan.rs), so every v2 plan the producer + * emits carries it (`[]` when empty). Required, not optional: the loader + * only ever accepts `formatVersion === 2` (strict equality, + * `SUPPORTED_FORMAT_VERSION`), so there is no live v1-compat path that + * needs this to be absent. */ - streamTables?: WireAsyncTable[]; + streamTables: WireAsyncTable[]; /** Future-table metadata (plan v2); see `streamTables`. */ - futureTables?: WireAsyncTable[]; + futureTables: WireAsyncTable[]; + /** * Resource types the component imports, in `ResourceIndex` order: * `ResourceIndex = importedResources.length + DefinedResourceIndex` diff --git a/runtime/src/plan/loader.ts b/runtime/src/plan/loader.ts index 3621eb8..3823fcd 100644 --- a/runtime/src/plan/loader.ts +++ b/runtime/src/plan/loader.ts @@ -121,6 +121,15 @@ export function loadPlan(wire: WirePlan): LoadedPlan { "canonicalOptions", "types", "resourceTables", + // ISSUE #94(2): the shim (crates/translator-shim/src/plan.rs) has no + // `skip_serializing_if` on `stream_tables`/`future_tables` — a real + // emitted v2 plan always serializes these as arrays (`[]` when empty, + // never absent). Requiring presence here keeps the loader consistent + // with what the producer actually emits, rather than silently + // tolerating an absent field via `?? []` (which would also mask a + // genuinely malformed/truncated envelope). + "streamTables", + "futureTables", "imports", "exports", ] as const @@ -130,6 +139,22 @@ export function loadPlan(wire: WirePlan): LoadedPlan { } } + // ISSUE #94(3): deep-schema strictness. `initializers` / `trampolines` / + // `canonicalOptions` / `CoreDef`s reach `runInitializers` unchecked today; + // a malformed op object (e.g. `{"op":"instantiate-module"}` missing + // `args`) dies as a raw `TypeError` deep in the executor rather than a + // typed `PlanError` here at load time. Proportionate check: a + // discriminated-union switch per op/trampoline kind verifying required + // fields are present and primitively typed — not a full JSON-schema + // engine. + wire.initializers.forEach((init, i) => + validateInitializer(init, `initializers[${i}]`) + ); + wire.trampolines.forEach((t, i) => validateTrampoline(t, `trampolines[${i}]`)); + wire.canonicalOptions.forEach((o, i) => + validateCanonicalOptions(o, `canonicalOptions[${i}]`) + ); + const importedResources = wire.importedResources ?? []; for (const [i, ir] of importedResources.entries()) { if ( @@ -185,8 +210,8 @@ export function loadPlan(wire: WirePlan): LoadedPlan { numImportedResources: importedResources.length, streamElems: elems(wire.streamTables, "streamTables"), futureElems: elems(wire.futureTables, "futureTables"), - streamTableInstances: (wire.streamTables ?? []).map((t) => t.instance), - futureTableInstances: (wire.futureTables ?? []).map((t) => t.instance), + streamTableInstances: wire.streamTables.map((t) => t.instance), + futureTableInstances: wire.futureTables.map((t) => t.instance), }; } @@ -244,6 +269,254 @@ function base64Decode(s: string): Uint8Array { return out; } +// --- ISSUE #94(3): deep-schema validation -------------------------------- +// +// Proportionate shape-checking for the wire-format ops the executor runs +// strictly: required-field presence + primitive-type checks per +// discriminated-union arm, mirroring `format.ts`'s tagged unions. Not a +// full JSON-schema validator (no cross-field or index-bounds checks beyond +// what's already done for type/resource tables above) — just enough that +// a malformed op surfaces as a typed `PlanError` here instead of a raw +// `TypeError` mid-execution in the executor. + +function isRecord(x: unknown): x is Record { + return typeof x === "object" && x !== null && !Array.isArray(x); +} + +function expect( + cond: boolean, + where: string, + what: string, +): asserts cond { + if (!cond) throw new PlanError(`${where}: ${what}`); +} + +function expectNumber(o: Record, field: string, where: string) { + expect( + typeof o[field] === "number", + where, + `.${field} must be a number, got ${describeValue(o[field])}`, + ); +} + +function expectNumberOrNull( + o: Record, + field: string, + where: string, +) { + expect( + o[field] === null || typeof o[field] === "number", + where, + `.${field} must be a number or null, got ${describeValue(o[field])}`, + ); +} + +function expectString(o: Record, field: string, where: string) { + expect( + typeof o[field] === "string", + where, + `.${field} must be a string, got ${describeValue(o[field])}`, + ); +} + +function expectBoolean(o: Record, field: string, where: string) { + expect( + typeof o[field] === "boolean", + where, + `.${field} must be a boolean, got ${describeValue(o[field])}`, + ); +} + +function expectArray(o: Record, field: string, where: string) { + expect( + Array.isArray(o[field]), + where, + `.${field} must be an array, got ${describeValue(o[field])}`, + ); +} + +function describeValue(v: unknown): string { + if (v === undefined) return "undefined (missing)"; + if (v === null) return "null"; + if (Array.isArray(v)) return `array (length ${v.length})`; + if (typeof v === "object") return "object"; + return JSON.stringify(v); +} + +const CORE_TYPE_LANES = new Set(["i32", "i64", "f32", "f64"]); + +function validateCoreDef(def: unknown, where: string): void { + expect(isRecord(def), where, `must be an object, got ${describeValue(def)}`); + const d = def as Record; + expectString(d, "kind", where); + switch (d.kind) { + case "export": + expectNumber(d, "instance", where); + expect(isRecord(d.item), where, `.item must be an object`); + validateExportItem(d.item, `${where}.item`); + return; + case "instance-flags": + expectNumber(d, "instance", where); + return; + case "trampoline": + expectNumber(d, "index", where); + return; + case "unsafe-intrinsic": + expectString(d, "intrinsic", where); + return; + case "task-may-block": + return; + default: + throw new PlanError(`${where}: unknown CoreDef kind ${describeValue(d.kind)}`); + } +} + +function validateExportItem(item: unknown, where: string): void { + expect(isRecord(item), where, `must be an object`); + const it = item as Record; + expectString(it, "name", where); + expect( + typeof it.space === "string" && + ["func", "table", "memory", "global", "tag", "unknown"].includes( + it.space as string, + ), + where, + `.space must be one of func/table/memory/global/tag/unknown, got ` + + describeValue(it.space), + ); +} + +function validateCoreExport(exp: unknown, where: string): void { + expect(isRecord(exp), where, `must be an object`); + const e = exp as Record; + expectNumber(e, "instance", where); + expect(isRecord(e.item), where, `.item must be an object`); + validateExportItem(e.item, `${where}.item`); +} + +function validateInitializer(init: unknown, where: string): void { + expect(isRecord(init), where, `must be an object, got ${describeValue(init)}`); + const i = init as Record; + expectString(i, "op", where); + switch (i.op) { + case "instantiate-module": + expectNumber(i, "module", where); + expectNumberOrNull(i, "instance", where); + expectArray(i, "args", where); + (i.args as unknown[]).forEach((a, idx) => + validateCoreDef(a, `${where}.args[${idx}]`) + ); + return; + case "lower-import": + expectNumber(i, "index", where); + expectNumber(i, "import", where); + return; + case "extract-memory": + expectNumber(i, "index", where); + validateCoreExport(i.export, `${where}.export`); + return; + case "extract-realloc": + case "extract-callback": + case "extract-post-return": + expectNumber(i, "index", where); + validateCoreDef(i.def, `${where}.def`); + return; + case "extract-table": + expectNumber(i, "index", where); + validateCoreExport(i.export, `${where}.export`); + return; + case "resource": + expectNumber(i, "index", where); + expect( + typeof i.rep === "string" && CORE_TYPE_LANES.has(i.rep as string), + where, + `.rep must be one of i32/i64/f32/f64, got ${describeValue(i.rep)}`, + ); + expect( + i.dtor === null || isRecord(i.dtor), + where, + `.dtor must be a CoreDef object or null`, + ); + if (i.dtor !== null) validateCoreDef(i.dtor, `${where}.dtor`); + expectNumber(i, "instance", where); + return; + default: + throw new PlanError(`${where}: unknown initializer op ${describeValue(i.op)}`); + } +} + +// Trampoline kinds with precise wire shapes (format.ts's non-catch-all +// arms). Everything else falls to the `{ kind: string; index: number; +// [field: string]: unknown }` catch-all — milestone-aware unsupported +// kinds the executor rejects at instantiate time (contracts/intrinsics.md +// §B), so only `kind` (string) and `index` (number) are load-time +// invariants for those. +function validateTrampoline(t: unknown, where: string): void { + expect(isRecord(t), where, `must be an object, got ${describeValue(t)}`); + const tr = t as Record; + expectString(tr, "kind", where); + expectNumber(tr, "index", where); + switch (tr.kind) { + case "lower-import": + expectNumber(tr, "lowered", where); + expectNumber(tr, "options", where); + expectNumber(tr, "type", where); + return; + case "trap": + case "enter-sync-call": + case "exit-sync-call": + return; + case "task-return": + expectNumber(tr, "instance", where); + expectNumber(tr, "results", where); + expectNumber(tr, "options", where); + return; + case "resource-drop": + case "resource-new": + case "resource-rep": + expectNumber(tr, "instance", where); + expectNumber(tr, "resource", where); + return; + default: + // Catch-all: unknown/milestone-gated kind, only the common fields + // above are required. + return; + } +} + +function validateCanonicalOptions(o: unknown, where: string): void { + expect(isRecord(o), where, `must be an object, got ${describeValue(o)}`); + const co = o as Record; + expectNumber(co, "instance", where); + expect( + typeof co.stringEncoding === "string" && + ["utf8", "utf16", "latin1+utf16"].includes(co.stringEncoding as string), + where, + `.stringEncoding must be one of utf8/utf16/latin1+utf16, got ` + + describeValue(co.stringEncoding), + ); + expectNumberOrNull(co, "memory", where); + expectNumberOrNull(co, "realloc", where); + expectNumberOrNull(co, "postReturn", where); + expectNumberOrNull(co, "callback", where); + expectBoolean(co, "async", where); + expectBoolean(co, "cancellable", where); + expect(isRecord(co.coreType), where, `.coreType must be an object`); + const ct = co.coreType as Record; + expectArray(ct, "params", `${where}.coreType`); + expectArray(ct, "results", `${where}.coreType`); + for (const [field, lanes] of [["params", ct.params], ["results", ct.results]] as const) { + (lanes as unknown[]).forEach((lane, idx) => { + expect( + typeof lane === "string" && CORE_TYPE_LANES.has(lane), + `${where}.coreType.${field}[${idx}]`, + `must be one of i32/i64/f32/f64, got ${describeValue(lane)}`, + ); + }); + } +} + + function loadTypeDecl( t: WireTypeDecl, resourceTokens: ResourceTypeInfo[], diff --git a/runtime/src/task/streams.ts b/runtime/src/task/streams.ts index 5db35de..aa9e0e1 100644 --- a/runtime/src/task/streams.ts +++ b/runtime/src/task/streams.ts @@ -44,7 +44,7 @@ // memory that can be *partially* consumed, which is what makes partial copies // expressible. -import { assert_, trapIf } from "../cabi/trap.ts"; +import { assert_, Trap, trapIf } from "../cabi/trap.ts"; import { LiftLowerContext } from "../cabi/context.ts"; import { loadListFromValidRange } from "../cabi/load.ts"; import { storeListIntoValidRange } from "../cabi/store.ts"; @@ -369,6 +369,29 @@ export class SharedFutureImpl implements SharedBase { boundStore: unknown = null; dropped = false; + /** + * Set when the future's **writable** side went away without ever delivering + * its one value (#84 teardown of a trap-poisoned instance, #90 host + * `drop()` on a lowered-but-unwritten future). + * + * definitions.py keeps this state unreachable: `WritableFutureEnd.drop` + * traps unless the end is DONE (definitions.py:1183-1184), so a readable + * future end can never observe DROPPED (`future_copy`'s `on_copy_done` + * assertion, definitions.py:2614). Our two teardown paths deliberately + * bypass that trap — a poisoned instance cannot be asked to trap again, and + * the host `drop()` is a public API door — so the state exists here and has + * to be *total*: an unwritten future whose writer died can never satisfy + * its reader, so the reader is told at its rendezvous point, with a + * **trap**, never a DROPPED/COMPLETED answer and never a silent hang. + * + * Consumers of the flag: + * * `read` below, for a reader that has not parked yet (trap on the spot); + * * intrinsics/stream_builtins.ts `futureCopy`, for a parked guest reader + * (the pending event's thunk throws instead of producing a tuple); + * * exec/host_streams.ts leaves host readers on their existing DROPPED + * path — the conventions layer already brands that outcome. + */ + abandonReason: Error | null = null; pendingInst: unknown = null; pendingBuffer: GuestBuffer | null = null; pendingOnCopyDone: OnCopyDone | null = null; @@ -408,6 +431,13 @@ export class SharedFutureImpl implements SharedBase { } read(inst: unknown, dstBuffer: GuestBuffer, onCopyDone: OnCopyDone): void { + // #84 leg (c): the reader arrives AFTER the writable side was abandoned. + // definitions.py:1141 asserts `not self.dropped` here because the drop + // trap keeps that unreachable; for our abandoned state the honest answer + // is the same trap the parked reader gets, delivered synchronously. + if (this.dropped && this.abandonReason !== null) { + throw futureAbandonTrap(this.abandonReason); + } assert_(!this.dropped && dstBuffer.remain() === 1, "future read shape"); if (!this.pendingBuffer) { this.setPending(inst, dstBuffer, onCopyDone); @@ -541,6 +571,49 @@ export function poisonFailureOf(shared: unknown): Error | undefined { /** Instances whose async ends have already been retired (idempotence). */ const retiredInstances = new WeakSet(); +// --------------------------------------------------------------------------- +// Abandoned futures (#84, #90) +// --------------------------------------------------------------------------- + +/** + * The trap a reader of an abandoned future observes at its rendezvous point. + * + * `Trap` is the guest-visible fault vocabulary (cabi/trap.ts); the recorded + * reason rides as `cause` so the embedder/host layers can still attribute the + * original fault. (`Trap`'s constructor takes only a message, so `cause` is + * attached after construction rather than through `ErrorOptions`.) + */ +export function futureAbandonTrap(reason: Error): Trap { + const t = new Trap( + `future.read can never complete: ${reason.message}`, + ); + (t as { cause?: unknown }).cause = reason; + return t; +} + +/** The abandonment reason of a shared future, if it has one (#84/#90). */ +export function abandonReasonOf(shared: unknown): Error | null { + return shared instanceof SharedFutureImpl ? shared.abandonReason : null; +} + +/** + * Mark a future's writable side as gone-without-a-value and settle the + * rendezvous (#90's host `drop()` door; the poisoning walk below routes + * through `dropSharedForTeardown` instead, which adds the dead-guest + * discipline). + * + * Never throws, and idempotent: a second call on an already-dropped future is + * a no-op, so `drop()`/`Symbol.dispose` at the layers above are total. + */ +export function abandonSharedFuture( + shared: SharedFutureImpl, + reason: Error, +): void { + if (shared.dropped) return; + shared.abandonReason ??= reason; + dropSharedForTeardown(shared); +} + /** The structural slice of `ComponentInstanceState` the walk needs. */ interface PoisonedInstanceLike { readonly index?: number; @@ -557,8 +630,34 @@ interface PoisonedInstanceLike { * Notifying it would queue a phantom event into the corpse's waitables, and * a later driving loop servicing it would resume machinery whose instance * can no longer be entered (`tick` asserts enterability). Host sentinels - * carry no `mayEnter` key and healthy guest peers park only outside the - * bracket (`mayEnter === true`), so both are always notified. + * carry no `mayEnter` key, so they are always notified. + * + * #84 AUDIT (the "healthy guest peers park only with `mayEnter === true`" + * claim this test used to rest on). Verified for the *parking* mechanism: + * every park — the callback ABI's waitable-set wait, and equally a + * sync-lowered/JSPI peer blocked inside `finishCopy`'s SITE 4 via + * `blockCurrentActivation` — yields the thread out of the scheduler's + * enter/leave bracket, and the bracket's `leaveTo` runs on the way out + * (task/scheduler.ts `Store.tick` :905-917, task/thread.ts + * `Thread.resumeWith` :157-179, whose resume-side `assert_(mayEnterFrom(null))` + * would fire otherwise). So a JSPI-blocked peer parks with `mayEnter === true`: + * blocking inside the wasm frame does NOT hold the enter bracket. + * + * NOT verified — a genuine counterexample to the *converse*: `mayEnter === + * false` does not imply "poisoned". An instance that is merely mid-call is + * also non-enterable, and a caller instance stays non-enterable for the whole + * duration of a cross-component (FACT) call into the instance that traps + * (`ComponentInstanceState.enterFrom` clears `mayEnter` on the callee's + * entering set only, task/mod.ts:136-142). A *different* task of that healthy + * caller, parked on an end of a stream/future the trapping callee also held, + * is therefore classified dead here and retired silently — i.e. stranded, + * the outcome #66 exists to prevent. The two states are not distinguishable + * at this seam (the walk may be invoked over several instances in turn, so + * "already retired" is not a reliable proxy either). Reported with #84 rather + * than fixed here: narrowing the test would risk re-opening review B2 (a + * DROPPED event queued into a corpse's waitables), which is a + * scheduler-adjacent decision outside this track's territory. + * // CONTRACT: conservative reading — behavior deliberately unchanged. * * Used by the poisoning walk below and by the trapping-import abandonment * path (embedder/instantiate.ts `releaseAsyncArgs`). Idempotent. @@ -596,6 +695,24 @@ export function dropSharedForTeardown( * cross-component catches (intrinsics/fact_calls.ts, callee side) — with the * trap as `cause`. Idempotent per instance. The parked-side notification * discipline lives in `dropSharedForTeardown` above. + * + * Two refinements over the original #66 walk, both from #84: + * + * 1. FUTURES ARE NOT STREAMS. A `stream`'s reader may legitimately observe + * DROPPED (that is end-of-stream), but a `future`'s reader may not + * (definitions.py:2614) — the reference keeps the state unreachable by + * trapping an early writable-end drop (definitions.py:1183-1184), which + * a poisoned instance can no longer be made to do. So an unwritten + * writable future end in this table marks its shared object *abandoned* + * (first pass below) and its reader traps instead. A writable end that + * already reached `CopyState.DONE` delivered its value; nothing is owed. + * + * 2. ONE END'S FAILURE MUST NOT STRAND THE REST. The notification of a + * retired end runs arbitrary peer callbacks (host settlers, event + * thunks); a throw used to abort the loop mid-table, leaving the + * remaining ends live and their peers hanging — exactly the outcome this + * walk exists to prevent. The walk now always completes and rethrows the + * first failure afterwards. */ export function retireInstanceAsyncEnds( inst: PoisonedInstanceLike, @@ -603,13 +720,20 @@ export function retireInstanceAsyncEnds( ): void { if (retiredInstances.has(inst)) return; retiredInstances.add(inst); - for (const e of inst.handles) { - if (!(e instanceof CopyEnd)) continue; + const where = inst.index !== undefined + ? `component instance ${inst.index}` + : "a component instance"; + // Snapshot: the notifications below can run peer code that mutates tables. + const ends: CopyEnd[] = []; + for (const e of inst.handles) if (e instanceof CopyEnd) ends.push(e); + + // Pass 1: record the failure, and mark abandoned every future this table + // owes a value on. Done before ANY notification, so the reader-side trap + // decision cannot depend on the order the handle table happens to yield + // the two ends of one future in. + for (const e of ends) { const shared = e.shared as SharedStreamImpl | SharedFutureImpl; if (poisonFailures.get(shared) === undefined) { - const where = inst.index !== undefined - ? `component instance ${inst.index}` - : "a component instance"; poisonFailures.set( shared, new Error( @@ -619,8 +743,28 @@ export function retireInstanceAsyncEnds( ), ); } - dropSharedForTeardown(shared); + if ( + e instanceof WritableFutureEnd && shared instanceof SharedFutureImpl && + e.state !== CopyState.DONE && !shared.dropped + ) { + shared.abandonReason ??= poisonFailures.get(shared)!; + } + } + + // Pass 2: retire. Collect failures rather than abandoning the walk. + let first: unknown; + let failed = false; + for (const e of ends) { + try { + dropSharedForTeardown(e.shared as SharedStreamImpl | SharedFutureImpl); + } catch (err) { + if (!failed) { + failed = true; + first = err; + } + } } + if (failed) throw first; } // `Store.tick`'s bracket-break site reaches the walk through this seam (its diff --git a/runtime/tests/async_builtins_test.ts b/runtime/tests/async_builtins_test.ts index 20c7bf5..fca3de9 100644 --- a/runtime/tests/async_builtins_test.ts +++ b/runtime/tests/async_builtins_test.ts @@ -10,6 +10,8 @@ import { assertEq } from "./support/asserts.ts"; import { Trap } from "../src/cabi/mod.ts"; import { BLOCKED, + createSubtaskCancel, + createWaitableJoin, createWaitableSetPoll, createWaitableSetWait, } from "../src/intrinsics/async_builtins.ts"; @@ -33,6 +35,8 @@ import { popCurrentThread, pushCurrentThread, Store, + Subtask, + SubtaskState, Task, type TaskOptions, Thread, @@ -385,3 +389,128 @@ Deno.test("stream.cancel-write supersedes an undelivered COMPLETED", () => { popCurrentThread(thread); } }); + +// --------------------------------------------------------------------------- +// #87: SITE 5's park must set `hasSyncWaiter` and wait on `hasPendingEvent`, +// mirroring SITE 4 (stream_builtins.ts:305-323) and definitions.py +// `Waitable.wait_for_pending_event` (:786-790, reached from +// `canon_subtask_cancel` :2491). +// --------------------------------------------------------------------------- + +/** A subtask fixture with a manually-driven callee (no lowered import). */ +function mkSubtaskFixture(): { + store: Store; + inst: ComponentInstanceState; + thread: Thread; + subtask: Subtask; + subtaski: number; + asGuest(fn: () => T): T; +} { + const store = new Store(); + const inst = new ComponentInstanceState(0, store); + const subtask = new Subtask(); + // A callee that has not resolved by the time `onCancel` returns: the + // shape SITE 5 parks for. `onCancel` itself is a no-op — resolution + // happens later, driven by the test. + subtask.onCancel = () => {}; + const subtaski = inst.handles.add(subtask); + const ft: FuncType = { params: [], results: [], async: true }; + const opts: TaskOptions = { + async_: true, + callback: true, + stringEncoding: "utf8", + memory: null, + }; + const task = new Task(ft, opts, inst, () => [], () => {}); + const thread = new Thread(task, (function* () {})()); + return { + store, + inst, + thread, + subtask, + subtaski, + asGuest(fn: () => T): T { + pushCurrentThread(thread); + try { + return fn(); + } finally { + popCurrentThread(thread); + } + }, + }; +} + +/** Resolve `subtask` and arm its SUBTASK event, as the callee eventually does. */ +function resolveSubtask(subtask: Subtask, subtaski: number): void { + subtask.resolve(SubtaskState.CANCELLED_BEFORE_RETURNED, []); + subtask.setSubtaskPendingEvent(subtaski); +} + +Deno.test( + "subtask.cancel (SITE 5, jspi): the sync park sets hasSyncWaiter, so a " + + "concurrent waitable.join on the same subtask traps", + async () => { + const f = mkSubtaskFixture(); + const cancel = createSubtaskCancel({ async: false }, f.inst, "jspi"); + const join = createWaitableJoin(f.inst); + + // Kick off the sync cancel: `onCancel` does not resolve the subtask, so + // this parks. `blockCurrentActivation` returns a Promise, but the park + // itself — including setting `hasSyncWaiter` — happens synchronously + // before that Promise is handed back (mirrors SITE 4's fixture-timing + // assumption). + const pending = f.asGuest(() => cancel(f.subtaski)) as unknown as Promise< + number + >; + assertEq(f.subtask.hasSyncWaiter, true); + + // A concurrent `waitable.join` on the same subtask must trap — this is + // the reference's `canon_waitable_join` (definitions.py:2463) admitting + // the trap the repo's own join-time check exists for + // (async_builtins.ts:362-365), which SITE 5 used to make unreachable. + const wset = new WaitableSet(); + const seti = f.inst.handles.add(wset); + assertTraps( + () => f.asGuest(() => join(f.subtaski, seti)), + "synchronous waiter", + ); + + // Unwind cleanly: resolve the callee so the park settles, and drain the + // store so the pending promise's `.then` isn't left dangling. + resolveSubtask(f.subtask, f.subtaski); + f.store.tick(); + const rc = await pending; + assertEq(rc, SubtaskState.CANCELLED_BEFORE_RETURNED); + assertEq(f.subtask.hasSyncWaiter, false); + }, +); + +Deno.test( + "subtask.cancel (SITE 5, jspi): a plain sync cancel still resolves " + + "correctly through the park", + async () => { + const f = mkSubtaskFixture(); + const cancel = createSubtaskCancel({ async: false }, f.inst, "jspi"); + + const pending = f.asGuest(() => cancel(f.subtaski)) as unknown as Promise< + number + >; + assertEq(f.subtask.resolved(), false); + assertEq(f.subtask.hasSyncWaiter, true); + + // The callee resolves later; the park's `readyFunc` (`hasPendingEvent`) + // only fires once the event is actually armed — not merely once + // `resolved()` is true — so drive both steps to pin the ordering SITE 5 + // now depends on. + f.subtask.resolve(SubtaskState.CANCELLED_BEFORE_RETURNED, []); + // Not yet armed: the park must not be ready on `resolved()` alone. + assertEq(f.store.tick(), false); + f.subtask.setSubtaskPendingEvent(f.subtaski); + assertEq(f.store.tick(), true); + + const rc = await pending; + assertEq(rc, SubtaskState.CANCELLED_BEFORE_RETURNED); + assertEq(f.subtask.hasSyncWaiter, false); + assertEq(f.subtask.resolveDelivered(), true); + }, +); diff --git a/runtime/tests/async_lower_onresolve_failure_test.ts b/runtime/tests/async_lower_onresolve_failure_test.ts new file mode 100644 index 0000000..ac80956 --- /dev/null +++ b/runtime/tests/async_lower_onresolve_failure_test.ts @@ -0,0 +1,127 @@ +// #93 (test half): pinning how a trap raised while lowering an async-lower +// host import's RESULT surfaces to the embedder. +// +// exec/boundary.ts:1645-1658 (the async, non-suspendable arm of +// `createLoweredImport`) runs `onResolve`'s result lowering in a bare +// `.then()` continuation, not under `blockCurrentActivation`'s `produce` +// (unlike the sync/suspendable arm just above it, :1595-1601, which defers +// all CABI work to `produce` for exactly the issue-#24 attribution reason). +// A throw there is caught locally and parked on `store.hostFailure` +// (:1652-1655) — the same channel a rejected host Promise uses — rather than +// propagating as an ordinary exception or a subtask-machinery trap. +// +// This test does not judge whether that routing is correct (issue #93 asks +// for a comment at the site, which is the orchestrator's call on +// boundary.ts); it pins the current, observed behaviour so a future change +// is a deliberate diff, not a silent one. + +import { assertEq } from "./support/asserts.ts"; +import { + createLoweredImport, + newStats, + type ResolvedOptions, +} from "../src/exec/boundary.ts"; +import { + ComponentInstanceState, + pushCurrentThread, + popCurrentThread, + Store, + Task, + type TaskOptions, + Thread, +} from "../src/task/mod.ts"; +import type { FuncType } from "../src/cabi/types.ts"; + +/** `func() -> string`, async-typed — a result type that must allocate to lower. */ +const FT: FuncType = { + params: [], + results: [{ kind: "string" }], + async: true, +}; + +const TASK_OPTS: TaskOptions = { + async_: true, + callback: true, + stringEncoding: "utf8", + memory: null, +}; + +Deno.test( + "async lower: a RESULT-lowering trap at settle time (no realloc for a " + + "string result) lands in store.hostFailure, not a subtask trap", + async () => { + const store = new Store(); + const inst = new ComponentInstanceState(0, store); + const memory = new WebAssembly.Memory({ initial: 1 }); + const view = { + addrType: "i32" as const, + get bytes() { + return new Uint8Array(memory.buffer); + }, + get view() { + return new DataView(memory.buffer); + }, + get length() { + return memory.buffer.byteLength; + }, + ptrType: () => "i32" as const, + ptrSize: () => 4 as const, + }; + const opts: ResolvedOptions = { + stringEncoding: "utf8", + // deno-lint-ignore no-explicit-any + memory: view as any, + // No realloc: lowering a `string` result needs to allocate the guest + // buffer, and `cabi/context.ts:99` traps + // ("realloc required but not provided") when asked to without one. + realloc: null, + postReturn: null, + callback: null, + async: true, + cancellable: false, + // Async lower: results travel via the trailing retptr lane, so the + // core signature is unchanged by the result type (definitions.py + // lines 2250-2256: `max_flat_results = 0` when async). + coreType: { params: ["i32"], results: ["i32"] }, + instance: inst, + }; + const call = createLoweredImport({ + name: "host-fn-string-result", + ft: FT, + opts, + hostFn: () => Promise.resolve("hello"), + stats: newStats(), + mode: "plain", + suspendable: false, + }) as (...args: number[]) => unknown; + + const task = new Task(FT, TASK_OPTS, inst, () => [], () => {}); + const thread = new Thread(task, (function* () {})()); + pushCurrentThread(thread); + let packed: unknown; + try { + // retptr = 64: somewhere harmless in the first page. + packed = call(64); + } finally { + popCurrentThread(thread); + } + // The call itself returns a STARTED subtask handle — the trap has not + // happened yet, because the host Promise has not settled. + assertEq(typeof packed, "number"); + assertEq(store.hostFailure, undefined); + + await new Promise((r) => setTimeout(r, 0)); + + // The settle-time lowering trap ("realloc required but not provided") + // was caught by the bare `.then()` continuation and parked here, exactly + // as a rejected host Promise would be (:1652-1655) — not raised as an + // uncaught exception, and not delivered to the guest through the + // subtask's SUBTASK event. + assertEq(store.hostFailure !== undefined, true); + const msg = String((store.hostFailure as { message?: string })?.message ?? store.hostFailure); + assertEq(msg.includes("realloc required but not provided"), true); + // Consumed: the subtask never resolved, and the driving loop is the one + // responsible for rethrowing `store.hostFailure` — pinning that plumbing + // (rather than this local test rethrowing it) is exactly the point. + }, +); diff --git a/runtime/tests/cancel_bracket_race_test.ts b/runtime/tests/cancel_bracket_race_test.ts new file mode 100644 index 0000000..38e8ffb --- /dev/null +++ b/runtime/tests/cancel_bracket_race_test.ts @@ -0,0 +1,172 @@ +// #92 (test half): a seeded-shuffle regression pinning the "cancel-bracket +// timing" window named in issue #92. +// +// `Task.requestCancellation` (task/mod.ts:369-428) brackets its delivery with +// `inst.enterFrom(caller)` / `inst.leaveTo(caller)` around +// `SuspensionPoint.resume()` — but under jspi, `resume()` only *settles* the +// suspended activation's Promise; the resumed wasm frame's own continuation +// (its remaining built-ins, `exit-sync-call`, etc.) runs on a LATER +// microtask, after `leaveTo` has already released the bracket. A concurrent +// host EXPORT call that enters the same instance in between goes through +// `exec/boundary.ts`'s `createLiftedFunction` (:1002-1011 in the review's +// line numbers), which gates only on `inst.mayEnterFrom(null)` — it does not +// consult the scheduler's `resumingThread` slot the way `Store.tick` +// (scheduler.ts:876) does. +// +// This test does not *construct* a divergence (issue #92 says none was +// found): it pins that (a) the window is real — `mayEnterFrom` reports the +// instance free immediately after `requestCancellation` returns, even though +// the cancelled activation's continuation has not run yet — and (b) driving +// a second export call through that window today does not double-resume +// anything or leave inconsistent final state, i.e. the corpus-soundness +// claim in the issue. It is included in `just sched-seeds` so any future +// schedule-order dependence here is caught. + +import { assertEq } from "./support/asserts.ts"; +import { createWaitableSetWait } from "../src/intrinsics/async_builtins.ts"; +import { createLiftedFunction, newStats, type ResolvedOptions } from "../src/exec/boundary.ts"; +import { + ComponentInstanceState, + popCurrentThread, + pushCurrentThread, + Store, + Task, + type TaskOptions, + Thread, + WaitableSet, +} from "../src/task/mod.ts"; +import type { FuncType } from "../src/cabi/types.ts"; + +const ASYNC_FT: FuncType = { params: [], results: [], async: true }; +const CALLBACK_OPTS: TaskOptions = { + async_: true, + callback: true, + stringEncoding: "utf8", + memory: null, +}; + +Deno.test( + "#92: requestCancellation's enter/leave bracket releases before the " + + "resumed jspi activation's continuation runs — a concurrent export " + + "call can enter in between without an observed double-resume", + async () => { + const store = new Store(); + const inst = new ComponentInstanceState(0, store); + const wset = new WaitableSet(); + const seti = inst.handles.add(wset); + const memory = new WebAssembly.Memory({ initial: 1 }); + const view = { + addrType: "i32" as const, + get bytes() { + return new Uint8Array(memory.buffer); + }, + get view() { + return new DataView(memory.buffer); + }, + get length() { + return memory.buffer.byteLength; + }, + ptrType: () => "i32" as const, + ptrSize: () => 4 as const, + }; + const opts: ResolvedOptions = { + stringEncoding: "utf8", + // deno-lint-ignore no-explicit-any + memory: view as any, + realloc: null, + postReturn: null, + callback: null, + async: true, + cancellable: true, // the caller's `cancellable` canonical option. + coreType: { params: ["i32", "i32"], results: ["i32"] }, + instance: inst, + }; + const ctx = { + componentInstance: () => inst, + options: () => opts, + resultTypes: () => [], + }; + const task = new Task(ASYNC_FT, CALLBACK_OPTS, inst, () => [], () => {}); + task.state = "started"; + const thread = new Thread(task, (function* () {})()); + + // Park task A at a cancellable `waitable-set.wait` (jspi mode) — no + // event pending, so this is SITE 2's genuine suspension. + const wait = createWaitableSetWait({ options: 0 }, ctx, inst, "jspi"); + pushCurrentThread(thread); + let parked: unknown; + try { + parked = wait(seti, 0); + } finally { + popCurrentThread(thread); + } + assertEq(typeof parked, "object"); // a Promise, per blockCurrentActivation. + assertEq(store.waiting.length, 1); + + // Sanity: the instance is correctly gated WHILE parked. + assertEq(inst.mayEnterFrom(null), true); // parked tasks release the gate (#43). + + // Deliver the cancellation exactly as `Task.requestCancellation` does: + // finds the parked SuspensionPoint as a candidate (it is registered with + // `task === task` and `cancellable === true`), brackets `enter/leave` + // around its (synchronous) `resume`. + task.requestCancellation(null); + + // THE WINDOW: `requestCancellation` has already called `leaveTo`, so the + // instance looks fully free — even though the resumed activation's own + // continuation (the `.then()` the engine attached to the settled + // Promise) has not run yet. This is exactly the gap #92 names. + assertEq(inst.mayEnterFrom(null), true); + assertEq(task.state, "cancel-delivered"); + + // Drive a concurrent EXPORT call into the SAME instance through the + // real host-entry path (`createLiftedFunction`), which gates only on + // `mayEnterFrom` — not on the scheduler's `resumingThread` claim that + // `Store.tick` respects (scheduler.ts:876). If this traps or corrupts + // state, the divergence is no longer merely theoretical. + const syncFt: FuncType = { params: [], results: [], async: false }; + const exportOpts: ResolvedOptions = { + stringEncoding: "utf8", + memory: null, + realloc: null, + postReturn: null, + callback: null, + async: false, + cancellable: false, + coreType: { params: [], results: [] }, + instance: inst, + }; + const lifted = createLiftedFunction({ + name: "concurrent-export", + ft: syncFt, + opts: exportOpts, + core: () => undefined, + stats: newStats(), + }); + let raised: unknown; + try { + lifted(); + } catch (e) { + raised = e; + } + // Pin the CURRENT observed behaviour: the concurrent entry is admitted + // (no "reentrance forbidden" trap), matching the issue's finding that + // `mayEnterFrom` does not consult `resumingThread`. + assertEq(raised, undefined); + + // Let the cancelled activation's own continuation actually run (the + // microtask the engine scheduled when `resume()` settled its Promise), + // and confirm no double-resume assertion fired and the final state is + // consistent: the SuspensionPoint is done, the WaitableSet was never + // touched by the concurrent export (it did not join or wait), and no + // exception escaped this far. + let sawRejection: unknown; + await (parked as Promise).catch((e) => { + sawRejection = e; + }); + // A cancelled `waitable-set.wait` resolves with TASK_CANCELLED — not a + // rejection — so nothing should have been caught here. + assertEq(sawRejection, undefined); + assertEq(store.waiting.length, 0); + }, +); diff --git a/runtime/tests/digest_test.ts b/runtime/tests/digest_test.ts index 0ba40a3..6d20d4c 100644 --- a/runtime/tests/digest_test.ts +++ b/runtime/tests/digest_test.ts @@ -52,6 +52,8 @@ function syntheticPlan(overrides: Partial): WirePlan { canonicalOptions: [], types: [], resourceTables: [], + streamTables: [], + futureTables: [], imports: [], exports: [], worldDigest: "", diff --git a/runtime/tests/enter_sync_call_reentrance_test.ts b/runtime/tests/enter_sync_call_reentrance_test.ts new file mode 100644 index 0000000..e953299 --- /dev/null +++ b/runtime/tests/enter_sync_call_reentrance_test.ts @@ -0,0 +1,109 @@ +// The reentrance gate on the sync fused-adapter bracket (issue #99). +// +// Reference chain, component-model @ 73b7ad5 +// `design/mvp/canonical-abi/definitions.py`: +// * `canon_lower` line 2312 invokes the callee `FuncInst` with +// `caller = thread.task.inst`; +// * that `FuncInst` is `Store.lift`'s `func_inst`, lines 578-585, whose +// first statement is `trap_if(not inst.may_enter_from(caller))` (581); +// * `may_enter_from` (214) tests every instance in +// `entering_set(caller) = callee.self_and_ancestors() +// - caller.self_and_ancestors()` (230-234). +// +// So a *sibling* callee that is currently entered traps, an idle sibling does +// not, and same-instance / ancestor pairs have an empty entering set and never +// trap here (FACT traps those statically instead -- +// wasmtime-environ 47.0.3 `fact/trampoline.rs:120-127`). +// +// These tests drive the `enter-sync-call` trampoline directly, because the +// trapping shape is not constructible as a component: mutual sibling imports +// are rejected by validation (instance imports form a DAG). See the +// adjudication note on the trampoline itself. + +import { assertEq } from "./support/asserts.ts"; +import { + createTrampoline, + type SyncCallScope, + type TrampolineContext, +} from "../src/intrinsics/mod.ts"; +import { newStats } from "../src/exec/boundary.ts"; +import { ComponentInstanceState, Store } from "../src/task/mod.ts"; + +function fixture() { + const store = new Store(); + const insts = new Map(); + const syncCallStack: SyncCallScope[] = []; + const ctx = { + componentInstance: (i: number) => { + let s = insts.get(i); + if (s === undefined) { + s = new ComponentInstanceState(i, store); + insts.set(i, s); + } + return s; + }, + syncCallStack, + factStartScopes: [], + stats: newStats(), + trapState: { pending: undefined }, + } as unknown as TrampolineContext; + const enter = createTrampoline({ kind: "enter-sync-call", index: 0 } as never, ctx); + const exit = createTrampoline({ kind: "exit-sync-call", index: 0 } as never, ctx); + const inst = (i: number) => (ctx as TrampolineContext).componentInstance(i); + return { ctx, enter, exit, inst, syncCallStack }; +} + +/** `A` = instance 0, `C` = instance 1; sync (`async_ = 0`) throughout. */ +const A = 0; +const C = 1; + +Deno.test("enter-sync-call: idle sibling callee is enterable", () => { + const { enter, exit, inst, syncCallStack } = fixture(); + inst(A).mayEnter = false; // the host entered A (boundary `enterFrom(null)`) + enter(A, 0, C); + assertEq(syncCallStack.length, 1, "bracket opened"); + exit(); + assertEq(syncCallStack.length, 0, "bracket closed"); +}); + +Deno.test("enter-sync-call: sibling cycle A -> C -> A traps", () => { + const { enter, inst } = fixture(); + // Host entered A; A is mid-call into C, so C is entered too. The cycle is + // C calling back into A. + inst(A).mayEnter = false; + inst(C).mayEnter = false; + let msg = ""; + try { + enter(C, 0, A); + } catch (e) { + msg = String((e as Error).message ?? e); + } + assertEq( + msg.includes("cannot enter component instance"), + true, + `expected the reentrance trap, got: ${msg || ""}`, + ); +}); + +Deno.test("enter-sync-call: an acyclic sibling chain A -> B -> C never traps", () => { + const { enter, exit, inst } = fixture(); + const B = 2; + inst(A).mayEnter = false; + enter(A, 0, B); + enter(B, 0, C); + exit(); + exit(); + // Nothing above mutates `mayEnter`; the point is that the gate stays quiet + // for the shape `test/linking/unit.wast` (the sibling relift chain) uses. + assertEq(inst(B).mayEnter, true); + assertEq(inst(C).mayEnter, true); +}); + +Deno.test("enter-sync-call: same-instance pair has an empty entering set", () => { + const { enter, inst } = fixture(); + // definitions.py `entering_set`: `{A} - {A}` is empty, so `may_enter_from` + // is vacuously true even with `may_enter == False`. FACT never emits this + // pair (trampoline.rs:120-127) but the gate must agree with the reference. + inst(A).mayEnter = false; + enter(A, 0, A); +}); diff --git a/runtime/tests/executor_duplicate_import_test.ts b/runtime/tests/executor_duplicate_import_test.ts new file mode 100644 index 0000000..2966715 --- /dev/null +++ b/runtime/tests/executor_duplicate_import_test.ts @@ -0,0 +1,111 @@ +// ISSUE #88: two core-wasm imports may legally share one `(module, field)` +// pair (core wasm permits it; wasmtime-environ 47.0.3 info.rs:438-445 gives +// one flat positional `CoreDef` per import *slot*, independent of naming). +// The JS `WebAssembly` API has no way to give two same-named import slots +// different values — building `importObject[module][field] = value` for +// each declared import in turn just makes the second write win. Before the +// #88 fix, a plan supplying two *different* `CoreDef`s for a duplicate name +// silently wired the wrong function into one of the slots; instantiation +// still succeeded because the JS API only checks total counts/types, not +// per-slot identity. +// +// Approach: a hand-built core module (byte-for-byte, following the pattern +// in boundary_trap_test.ts — this repo's test suite has no wat2wasm/ +// component-shaping helper) with two `(env, g)` mutable-i32-global imports, +// driven through the full `instantiateComponent` entrypoint with a +// synthetic single-module plan. This exercises the real import-object +// builder in exec/executor.ts, not a re-implementation of it. + +import { assertEq } from "./support/asserts.ts"; +import { instantiateComponent } from "../src/exec/mod.ts"; +import { PlanError } from "../src/plan/mod.ts"; +import type { WirePlan } from "../src/plan/format.ts"; + +/** + * `(module (import "env" "g" (global (mut i32))) (import "env" "g" (global + * (mut i32))))` — two imports sharing one `(module, field)` name, both + * mutable i32 globals (so `ComponentInstanceState.flags`, a + * `WebAssembly.Global({value:"i32",mutable:true})`, is a legal value for + * either slot — see task/mod.ts). + */ +const DUP_IMPORT_MODULE = new Uint8Array([ + 0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00, // \0asm, version 1 + // import section: 2 entries, each "env"."g" (mut i32 global) + 0x02, 0x13, 0x02, + 0x03, 0x65, 0x6e, 0x76, 0x01, 0x67, 0x03, 0x7f, 0x01, + 0x03, 0x65, 0x6e, 0x76, 0x01, 0x67, 0x03, 0x7f, 0x01, +]); + +function planFor(args: WirePlan["initializers"][0] & { op: "instantiate-module" }): WirePlan { + return { + formatVersion: 2, + producer: { shimVersion: "test", wasmtimeEnviron: "47.0.3", features: [] }, + component: { sha256: "0".repeat(64), len: DUP_IMPORT_MODULE.length }, + modules: [{ kind: "embedded", offset: 0, len: DUP_IMPORT_MODULE.length }], + initializers: [args], + trampolines: [], + canonicalOptions: [], + types: [], + resourceTables: [], + streamTables: [], + futureTables: [], + imports: [], + exports: [], + worldDigest: "sha256:0", + }; +} + +Deno.test("executor: duplicate (module,field) core imports resolving to the SAME value instantiate fine", async () => { + const plan = planFor({ + op: "instantiate-module", + module: 0, + instance: null, + // Both slots reference `instance-flags` on the SAME component instance + // (0): reference-identical values, which the JS API cannot distinguish + // anyway — this must not throw. + args: [ + { kind: "instance-flags", instance: 0 }, + { kind: "instance-flags", instance: 0 }, + ], + }); + const component = await instantiateComponent({ + plan, + componentBytes: DUP_IMPORT_MODULE, + verifyHash: false, + }); + assertEq(component.coreInstances.length, 1, "one core instance"); +}); + +Deno.test("executor: duplicate (module,field) core imports resolving to DIFFERENT values is a typed PlanError naming module/field/indices", async () => { + const plan = planFor({ + op: "instantiate-module", + module: 0, + instance: null, + // Two DIFFERENT component instances -> two DIFFERENT `flags` Global + // objects wired to the same "env"."g" import name: exactly the shape + // #88 says must fail loudly rather than silently collapse to the last + // write. + args: [ + { kind: "instance-flags", instance: 0 }, + { kind: "instance-flags", instance: 1 }, + ], + }); + let caught: unknown; + try { + await instantiateComponent({ + plan, + componentBytes: DUP_IMPORT_MODULE, + verifyHash: false, + }); + } catch (e) { + caught = e; + } + if (!(caught instanceof PlanError)) { + throw new Error(`expected a PlanError, got ${caught}`); + } + const msg = String(caught); + assertEq(msg.includes("env"), true, `message names the module: ${msg}`); + assertEq(msg.includes("g"), true, `message names the field: ${msg}`); + assertEq(msg.includes("0") && msg.includes("1"), true, + `message names the conflicting arg indices: ${msg}`); +}); diff --git a/runtime/tests/plan_loader_test.ts b/runtime/tests/plan_loader_test.ts index eb2c844..24e7eb8 100644 --- a/runtime/tests/plan_loader_test.ts +++ b/runtime/tests/plan_loader_test.ts @@ -37,6 +37,8 @@ function minimalPlan(overrides: Partial = {}): WirePlan { canonicalOptions: [], types: [], resourceTables: [], + streamTables: [], + futureTables: [], imports: [], exports: [], worldDigest: "sha256:0", @@ -251,3 +253,150 @@ Deno.test("loader: importedResources back-references are range-checked", () => { "not a valid index into plan.imports", ); }); + +// ISSUE #94(2): streamTables/futureTables are required-array in a v2 plan +// (the shim never omits them; see plan/format.ts's field doc). A plan that +// omits them entirely (a stale v0.1-shaped document masquerading as v2, or +// a truncated envelope) must fail loudly at load time, not be silently +// read as "no stream/future tables". +Deno.test("loader: streamTables/futureTables are required for formatVersion 2", () => { + const wire = minimalPlan() as unknown as Record; + delete wire.streamTables; + assertPlanError( + () => loadPlan(wire as unknown as WirePlan), + "plan.streamTables missing or not an array", + ); + const wire2 = minimalPlan() as unknown as Record; + delete wire2.futureTables; + assertPlanError( + () => loadPlan(wire2 as unknown as WirePlan), + "plan.futureTables missing or not an array", + ); +}); + +// ISSUE #94(3): deep-schema strictness for initializers/trampolines/ +// canonicalOptions — malformed ops must surface as typed PlanErrors at +// load time, not as raw TypeErrors deep in the executor. +Deno.test("loader: malformed instantiate-module initializer (missing args) is a typed PlanError", () => { + assertPlanError( + () => + loadPlan(minimalPlan({ + initializers: [ + { op: "instantiate-module", module: 0, instance: null } as never, + ], + })), + "args must be an array", + ); +}); + +Deno.test("loader: malformed CoreDef (unknown kind) inside instantiate-module args is a typed PlanError", () => { + assertPlanError( + () => + loadPlan(minimalPlan({ + initializers: [ + { + op: "instantiate-module", + module: 0, + instance: null, + args: [{ kind: "not-a-real-kind" } as never], + }, + ], + })), + "unknown CoreDef kind", + ); +}); + +Deno.test("loader: unknown initializer op is a typed PlanError", () => { + assertPlanError( + () => + loadPlan(minimalPlan({ + initializers: [{ op: "not-a-real-op" } as never], + })), + "unknown initializer op", + ); +}); + +Deno.test("loader: malformed trampoline (wrong-typed field) is a typed PlanError", () => { + assertPlanError( + () => + loadPlan(minimalPlan({ + trampolines: [ + { kind: "task-return", index: 0, instance: "not-a-number" } as never, + ], + })), + ".instance must be a number", + ); +}); + +Deno.test("loader: unrecognized trampoline kind still requires kind/index (milestone-gated catch-all)", () => { + // Not every trampoline kind has a precise wire shape (format.ts's + // catch-all `{ kind: string; index: number; ... }` for milestone-gated + // kinds rejected at instantiate time). The loader only enforces the two + // invariants that are always true. + loadPlan(minimalPlan({ + trampolines: [{ kind: "some-future-milestone-kind", index: 0 } as never], + })); + assertPlanError( + () => + loadPlan(minimalPlan({ + trampolines: [{ kind: "some-future-milestone-kind" } as never], + })), + ".index must be a number", + ); +}); + +Deno.test("loader: malformed canonicalOptions (bad stringEncoding) is a typed PlanError", () => { + assertPlanError( + () => + loadPlan(minimalPlan({ + canonicalOptions: [ + { + instance: 0, + stringEncoding: "utf-9000", + memory: null, + realloc: null, + postReturn: null, + callback: null, + async: false, + cancellable: false, + coreType: { params: [], results: [] }, + } as never, + ], + })), + ".stringEncoding must be one of", + ); +}); + +Deno.test("loader: valid plans with well-formed initializers/trampolines/canonicalOptions load unaffected", () => { + const wire = minimalPlan({ + initializers: [ + { + op: "instantiate-module", + module: 0, + instance: null, + args: [ + { kind: "task-may-block" }, + { kind: "unsafe-intrinsic", intrinsic: "context.get-0" }, + ], + }, + ], + trampolines: [ + { kind: "trap", index: 0 }, + { kind: "resource-drop", index: 1, instance: 0, resource: 0 }, + ], + canonicalOptions: [ + { + instance: 0, + stringEncoding: "utf8", + memory: null, + realloc: null, + postReturn: null, + callback: null, + async: false, + cancellable: false, + coreType: { params: ["i32"], results: [] }, + }, + ], + }); + loadPlan(wire); // must not throw +}); diff --git a/runtime/tests/resource_lender_unwind_test.ts b/runtime/tests/resource_lender_unwind_test.ts new file mode 100644 index 0000000..89b7ab0 --- /dev/null +++ b/runtime/tests/resource_lender_unwind_test.ts @@ -0,0 +1,190 @@ +// FACT start-call unwind: lender scopes must not leak on the non-success +// exits (issue #91). +// +// Authority: contracts/intrinsics.md v0.2 amendment 2 — on a trap escaping a +// FACT bracket the host unwinds sync-call scopes (releasing lenders) because +// this runtime deliberately supports post-trap re-entry on the caller side, +// where definitions.py kills the whole store instead. The lent handles belong +// to the CALLER, which neither a callee trap nor a capability bail poisons, +// so leaving `num_lends` elevated would make every later `lift_own` / +// `resource.drop` of those handles trap "handle still lent out" +// (definitions.py lines 1508 / 2325). + +import { + createAsyncStartCall, + createPrepareCall, + createSyncStartCall, + type PreparedCall, +} from "../src/intrinsics/fact_calls.ts"; +import type { FactStartScope } from "../src/intrinsics/mod.ts"; +import { newStats } from "../src/exec/boundary.ts"; +import { ComponentInstanceState, Store } from "../src/task/mod.ts"; +import { NeedsJspi } from "../src/task/scheduler.ts"; +import { + canonResourceDrop, + canonResourceNew, + ResourceHandle, + ResourceTypeInfo, +} from "../src/cabi/mod.ts"; +import type { CoreValue, ValType } from "../src/cabi/types.ts"; +import { assertEq } from "./support/asserts.ts"; + +function assert(cond: boolean, msg: string): asserts cond { + if (!cond) throw new Error(`assertion failed: ${msg}`); +} + +const PREPARE_ASYNC_NO_RESULT = 0xffff_ffff; + +interface Harness { + caller: ComponentInstanceState; + callee: ComponentInstanceState; + handle: ResourceHandle; + rt: ResourceTypeInfo; + handleIndex: number; + /** Run one prepare + start-call; returns whatever escaped, or null. */ + run(kind: "sync" | "async", calleeBody: () => CoreValue): unknown; +} + +function mkHarness(postReturn: (() => void) | null = null): Harness { + const store = new Store(); + const caller = new ComponentInstanceState(0, store); + const callee = new ComponentInstanceState(1, store); + // The resource is implemented by the CALLER, so dropping it later is the + // same-instance (ungated) path — this test is about `num_lends`, not #85. + const rt = new ResourceTypeInfo(caller, () => {}); + const handleIndex = canonResourceNew(caller, rt, 77); + const handle = caller.handles.get(handleIndex) as ResourceHandle; + + const factStartScopes: FactStartScope[] = []; + const prepared: { current: PreparedCall | null } = { current: null }; + const ctx = { + componentInstance: (i: number) => (i === 0 ? caller : callee), + resultTypes: () => [] as ValType[], + callback: (_i: number) => postReturn, + memoryToken: () => null, + stats: newStats(), + prepared, + factStartScopes, + suspensionMode: "plain" as const, + }; + + return { + caller, + callee, + handle, + rt, + handleIndex, + run(kind, calleeBody) { + // `[async-start]`: this is where a `transfer-borrow` intrinsic lends one + // of the caller's handles to the call (intrinsics/mod.ts + // `FactStartScope`), so the stub does exactly that. + const start = () => { + const scope = factStartScopes[factStartScopes.length - 1]; + assert(scope !== undefined, "a start scope is live"); + scope.lenders.addLender(handle); + return undefined as unknown as CoreValue; + }; + const return_ = () => undefined as unknown as CoreValue; + + // deno-lint-ignore no-explicit-any + const prep = createPrepareCall({ memory: null }, ctx as any); + const startCall = kind === "sync" + // deno-lint-ignore no-explicit-any + ? createSyncStartCall({ callback: null }, ctx as any) + : createAsyncStartCall( + { callback: null, postReturn: postReturn === null ? null : 0 }, + // deno-lint-ignore no-explicit-any + ctx as any, + ); + + prep( + start, + return_, + 0, // caller_instance + 1, // callee_instance + 0, + 0, + 0, + PREPARE_ASYNC_NO_RESULT, + ); + try { + if (kind === "sync") startCall(calleeBody, 0); + else startCall(calleeBody, 0, 0, 0); + return null; + } catch (e) { + return e; + } + }, + }; +} + +Deno.test("#91: sync-start-call releases the caller's lenders when the callee traps", () => { + const h = mkHarness(); + const boom = new Error("callee trap"); + const escaped = h.run("sync", () => { + throw boom; + }); + assertEq(escaped === boom, true); + // The callee is poisoned; the caller is not, and its handle is usable again. + assertEq(h.callee.mayEnter, false); + assertEq(h.handle.numLends, 0); + canonResourceDrop(h.caller, h.rt, h.handleIndex); // no "still lent out" trap +}); + +Deno.test("#91: sync-start-call releases lenders on a capability bail", () => { + const h = mkHarness(); + // `NeedsJspi` stands for a blocking operation this runtime cannot perform + // yet (here: raised by the callee; the sibling site is the intrinsic's own + // bail when an async-lifted callee has not resolved by the end of its first + // activation, which needs a real parking callee to reach). It is expressly + // a NON-poisoning capability signal, so the caller is guaranteed to keep + // running and stranded lenders would be permanent — strictly worse than on + // the trap path. + const boom = new NeedsJspi("callee needs to block"); + const escaped = h.run("sync", () => { + throw boom; + }); + assert(escaped instanceof NeedsJspi, `expected NeedsJspi, got ${escaped}`); + assertEq(h.callee.mayEnter, true); // not poisoned, as the class demands + assertEq(h.handle.numLends, 0); + canonResourceDrop(h.caller, h.rt, h.handleIndex); +}); + +Deno.test("#91: async-start-call releases the subtask's lenders when the callee traps", () => { + const h = mkHarness(); + const boom = new Error("callee trap"); + const escaped = h.run("async", () => { + throw boom; + }); + assertEq(escaped === boom, true); + assertEq(h.callee.mayEnter, false); + // The subtask never reached `report()`, so nothing else would ever deliver + // its resolution and release these. + assertEq(h.handle.numLends, 0); + canonResourceDrop(h.caller, h.rt, h.handleIndex); +}); + +Deno.test("#91: a trapping post-return leaves may_leave as the reference does", () => { + // definitions.py `canon_lift` (lines 2170-2174): `may_leave = False`, the + // post-return call, `may_leave = True`. A trap in between skips the + // restore, and `Store.lift`'s `leave_to` too — the instance is poisoned and + // its flags are left exactly as the trap left them. Verified rather than + // "fixed": restoring `may_leave` here locally would contradict both. The + // obligation this runtime adds — that no *live* instance is stranded with + // `may_leave === false` — is discharged at the host boundary by + // exec/boundary.ts `unwind`, which asserts the resting state for every + // instance outside the poisoned entered set. + const boom = new Error("post-return trap"); + const h = mkHarness(() => { + throw boom; + }); + // flags = 0 selects the sync-ABI callee, whose lift runs the post-return. + const escaped = h.run("async", () => undefined as unknown as CoreValue); + assertEq(escaped === boom, true); + assertEq(h.callee.mayEnter, false); // poisoned: never enterable again + assertEq(h.callee.mayLeave, false); // left as the trap left it + // The caller — the instance that survives and may be re-entered — is sane. + assertEq(h.caller.mayEnter, true); + assertEq(h.caller.mayLeave, true); + assertEq(h.handle.numLends, 0); +}); diff --git a/runtime/tests/resource_lifetime_test.ts b/runtime/tests/resource_lifetime_test.ts new file mode 100644 index 0000000..9b3e0f9 --- /dev/null +++ b/runtime/tests/resource_lifetime_test.ts @@ -0,0 +1,328 @@ +// Resource destructor gating and host-side lend tracking (issues #85, #86). +// +// Authority: definitions.py `canon_resource_drop` (line 2318) and the +// `Store.lift` entry gate it routes the dtor through (lines 579-584), plus +// the lend bookkeeping of `Subtask.add_lender` / `deliver_resolve` +// (lines 890, 902) and the `num_lends` traps in `lift_own` / +// `canon_resource_drop` (lines 1508, 2325). + +import { + callDtorGated, + canonResourceDrop, + canonResourceNew, + ResourceTypeInfo, +} from "../src/cabi/mod.ts"; +import { ComponentInstanceState, Store } from "../src/task/mod.ts"; +import { setOnInstancePoisoned } from "../src/task/scheduler.ts"; +// Side-effecting import: registers `retireInstanceAsyncEnds` as the poisoning +// hook (#66). Without it the seam is null and the poison walk is a no-op. +import { retireInstanceAsyncEnds } from "../src/task/streams.ts"; +import { + GuestResource, + lendWrapper, + makeWrapper, + simulateFinalizationForTest, + takeRep, + wrapperLends, +} from "../src/embedder/resources.ts"; +import { assertEq, assertTrap } from "./support/asserts.ts"; + +function assert(cond: boolean, msg: string): asserts cond { + if (!cond) throw new Error(`assertion failed: ${msg}`); +} + +/** Install a spy on the poisoning seam for one test, then restore the real one. */ +function withPoisonSpy( + run: (seen: { inst: unknown; cause: unknown }[]) => T, +): T { + const seen: { inst: unknown; cause: unknown }[] = []; + setOnInstancePoisoned((inst, cause) => { + seen.push({ inst, cause }); + retireInstanceAsyncEnds(inst, cause); + }); + const restore = () => setOnInstancePoisoned(retireInstanceAsyncEnds); + let out: T; + try { + out = run(seen); + } catch (e) { + restore(); + throw e; + } + // The seam must stay installed across an async body's microtasks. + if (typeof (out as { then?: unknown })?.then === "function") { + return (out as unknown as Promise).then( + (v) => { + restore(); + return v; + }, + (e) => { + restore(); + throw e; + }, + ) as unknown as T; + } + restore(); + return out; +} + +function mkPair(): { + store: Store; + caller: ComponentInstanceState; + impl: ComponentInstanceState; +} { + const store = new Store(); + return { + store, + caller: new ComponentInstanceState(0, store), + impl: new ComponentInstanceState(1, store), + }; +} + +// --------------------------------------------------------------------------- +// #85 — dtor gating +// --------------------------------------------------------------------------- + +Deno.test("#85: dropping a cross-instance own traps while the impl is entered", () => { + const { caller, impl } = mkPair(); + let ran = 0; + const rt = new ResourceTypeInfo(impl, () => { + ran += 1; + }); + const h = canonResourceNew(caller, rt, 42); + + // The implementing instance is mid-execution (someone is inside it). + impl.enterFrom(null); + assertTrap( + () => canonResourceDrop(caller, rt, h), + "cannot enter component instance", + ); + assertEq(ran, 0); + // The gate was refused, not taken: the impl is still merely *entered*, not + // poisoned, and leaving it restores enterability. + impl.leaveTo(null); + assertEq(impl.mayEnter, true); +}); + +Deno.test("#85: a dtor-less resource is gated too (the reference's `or lambda`)", () => { + const { caller, impl } = mkPair(); + const rt = new ResourceTypeInfo(impl, null); + const h = canonResourceNew(caller, rt, 7); + impl.enterFrom(null); + assertTrap( + () => canonResourceDrop(caller, rt, h), + "cannot enter component instance", + ); +}); + +Deno.test("#85: same-instance drop is exempt even while the instance is entered", () => { + const { caller } = mkPair(); + let ran = 0; + // impl === the dropping instance: `entering_set(caller)` is empty. + const rt = new ResourceTypeInfo(caller, () => { + ran += 1; + }); + const h = canonResourceNew(caller, rt, 5); + caller.enterFrom(null); // the guest is of course running while it drops + canonResourceDrop(caller, rt, h); + assertEq(ran, 1); + assertEq(caller.mayEnter, false); // unchanged by the drop +}); + +Deno.test("#85: a cross-instance drop takes and releases the gate", () => { + const { caller, impl } = mkPair(); + const seenInside: boolean[] = []; + const rt = new ResourceTypeInfo(impl, () => { + seenInside.push(impl.mayEnter); + }); + const h = canonResourceNew(caller, rt, 1); + canonResourceDrop(caller, rt, h); + assertEq(seenInside, [false]); // entered for the duration + assertEq(impl.mayEnter, true); // and left afterwards +}); + +Deno.test("#85: a trapping dtor poisons the impl instance and retires its ends", () => { + withPoisonSpy((seen) => { + const { caller, impl } = mkPair(); + const boom = new Error("dtor trap"); + const rt = new ResourceTypeInfo(impl, () => { + throw boom; + }); + const h = canonResourceNew(caller, rt, 3); + let caught: unknown; + try { + canonResourceDrop(caller, rt, h); + } catch (e) { + caught = e; + } + assertEq(caught === boom, true); + // `leave_to` is not reached: the impl is unenterable forever. + assertEq(impl.mayEnter, false); + assertEq(seen.length, 1); + assertEq(seen[0].inst === impl, true); + assertEq(seen[0].cause === boom, true); + // ... and the dropping instance is untouched by *this* bracket. + assertEq(caller.mayEnter, true); + }); +}); + +Deno.test("#85: a guest-initiated dtor that does not finish synchronously traps", () => { + withPoisonSpy((seen) => { + const { caller, impl } = mkPair(); + const rt = new ResourceTypeInfo( + impl, + (() => Promise.resolve()) as unknown as (rep: number) => void, + ); + const h = canonResourceNew(caller, rt, 9); + assertTrap( + () => canonResourceDrop(caller, rt, h), + "did not complete synchronously", + ); + assertEq(impl.mayEnter, false); + assertEq(seen.length, 1); + }); +}); + +Deno.test("#85: a host-initiated async dtor holds the gate until it settles", async () => { + const { store, impl } = mkPair(); + let resolveDtor: () => void = () => {}; + const rt = new ResourceTypeInfo( + impl, + (() => new Promise((r) => (resolveDtor = r))) as unknown as ( + rep: number, + ) => void, + ); + callDtorGated(rt, 11, null, true); + assertEq(impl.mayEnter, false); + assertEq(store.pendingHostCalls.size, 1); + resolveDtor(); + await Promise.all([...store.pendingHostCalls]); + await Promise.resolve(); + assertEq(impl.mayEnter, true); + assertEq(store.pendingHostCalls.size, 0); + assertEq(store.hostFailure, undefined); +}); + +Deno.test("#85: a rejected host-initiated dtor poisons and lands on hostFailure", async () => { + await withPoisonSpy(async (seen) => { + const { store, impl } = mkPair(); + const boom = new Error("async dtor trap"); + const rt = new ResourceTypeInfo( + impl, + (() => Promise.reject(boom)) as unknown as (rep: number) => void, + ); + callDtorGated(rt, 12, null, true); + const pending = [...store.pendingHostCalls]; + await Promise.all(pending); + await Promise.resolve(); + assertEq(store.hostFailure === boom, true); + assertEq(impl.mayEnter, false); + assertEq(seen.length, 1); + store.hostFailure = undefined; + }); +}); + +// --------------------------------------------------------------------------- +// #86 — host lend tracking +// --------------------------------------------------------------------------- + +class Res extends GuestResource {} + +function mkWrapper(rt: ResourceTypeInfo, rep = 100) { + return makeWrapper(Res, rep, rt, true); +} + +Deno.test("#86: drop() while lent is deferred until the last release", () => { + const { impl } = mkPair(); + const dropped: number[] = []; + const rt = new ResourceTypeInfo(impl, (rep) => { + dropped.push(rep); + }); + const w = mkWrapper(rt, 21); + + const release = lendWrapper(w); // host `own` lowered as `borrow` + assertEq(wrapperLends(w), 1); + w.drop(); + assertEq(dropped, []); // NOT destroyed under a live guest borrow + release(); + assertEq(dropped, [21]); + assertEq(wrapperLends(w), 0); + // Idempotent: a second release (and a second drop) change nothing. + release(); + w.drop(); + assertEq(dropped, [21]); +}); + +Deno.test("#86: two overlapping lends both have to be released", () => { + const { impl } = mkPair(); + const dropped: number[] = []; + const rt = new ResourceTypeInfo(impl, (rep) => dropped.push(rep)); + const w = mkWrapper(rt, 22); + const r1 = lendWrapper(w); + const r2 = lendWrapper(w); + assertEq(wrapperLends(w), 2); + w.drop(); + r1(); + assertEq(dropped, []); + r2(); + assertEq(dropped, [22]); +}); + +Deno.test("#86: the GC backstop under a live borrow defers instead of destroying", () => { + const { impl } = mkPair(); + const dropped: number[] = []; + const rt = new ResourceTypeInfo(impl, (rep) => dropped.push(rep)); + const w = mkWrapper(rt, 23); + const release = lendWrapper(w); + + simulateFinalizationForTest(w); // the finalizer callback, verbatim + assertEq(dropped, []); // the repro of #86: no use-after-free + release(); + assertEq(dropped, [23]); + // Not resurrected, not double-run. + simulateFinalizationForTest(w); + assertEq(dropped, [23]); +}); + +Deno.test("#86: the backstop is idempotent against an explicit drop", () => { + const { impl } = mkPair(); + const dropped: number[] = []; + const rt = new ResourceTypeInfo(impl, (rep) => dropped.push(rep)); + const w = mkWrapper(rt, 24); + w.drop(); + simulateFinalizationForTest(w); + assertEq(dropped, [24]); +}); + +Deno.test("#86: a trapping backstop dtor poisons the impl and records the failure", () => { + withPoisonSpy((seen) => { + const { store, impl } = mkPair(); + const boom = new Error("backstop dtor trap"); + const rt = new ResourceTypeInfo(impl, () => { + throw boom; + }); + const w = mkWrapper(rt, 25); + // Never throws out of the finalizer callback... + simulateFinalizationForTest(w); + // ... but is no longer swallowed either (the former `catch {}`). + assertEq(store.hostFailure === boom, true); + assertEq(impl.mayEnter, false); + assertEq(seen.length, 1); + store.hostFailure = undefined; + }); +}); + +Deno.test("#86: transferring a lent handle as own is refused (lift_own)", () => { + const { impl } = mkPair(); + const rt = new ResourceTypeInfo(impl, () => {}); + const w = mkWrapper(rt, 26); + const release = lendWrapper(w); + let msg = ""; + try { + takeRep(w, true, "own"); + } catch (e) { + msg = (e as Error).message; + } + assert(msg.includes("still lent out"), `expected a lend refusal, got ${msg}`); + release(); + assertEq(takeRep(w, true, "own"), 26); +}); diff --git a/runtime/tests/store_int_range_test.ts b/runtime/tests/store_int_range_test.ts new file mode 100644 index 0000000..b32ffca --- /dev/null +++ b/runtime/tests/store_int_range_test.ts @@ -0,0 +1,109 @@ +// Host-side integer range asserts on the scalar store path (issue #96): +// definitions.py `store_int` -> `int.to_bytes(v, nbytes, 'little', signed=…)` +// raises `OverflowError` when `v` does not fit; `storeInt` (cabi/memory.ts) +// now raises the port's equivalent host-precondition error (`AssertionError`, +// NOT `Trap` — cabi/trap.ts's taxonomy: only `Trap` models a guest-visible +// canonical-ABI fault) instead of silently wrapping. This is the fix for a +// buggy embedder value (e.g. `{x: 300}` into `record{x: u8}`) corrupting data +// instead of failing loudly. +// +// The BULK path (bulk_lists.ts, `tryStoreNumericList`) intentionally keeps +// wrap-on-overflow for throughput (see that file's header) — pinned +// separately in bulk_list_test.ts and store_list_test.ts. This file only +// covers the scalar `storeInt` path. + +import { AssertionError, MemInst, storeInt } from "../src/cabi/mod.ts"; +import { assertEq } from "./support/asserts.ts"; + +function freshMem(size = 64): MemInst { + return new MemInst(new Uint8Array(size), "i32"); +} + +function assertRangeError(fn: () => void, msg: string): void { + let err: unknown; + try { + fn(); + } catch (e) { + err = e; + } + assertEq(err instanceof AssertionError, true, `${msg}: expected AssertionError, got ${err}`); + assertEq( + String((err as Error)?.message ?? "").includes("out of range"), + true, + `${msg}: expected an "out of range" message, got: ${err}`, + ); +} + +Deno.test("storeInt: in-range values are unchanged for every width", () => { + const mem = freshMem(); + // [nbytes, signed, value, expected byte(s) readback via loadable width] + storeInt(mem, 0, 0, 1, false); + assertEq(mem.bytes[0], 0, "u8 min"); + storeInt(mem, 255, 0, 1, false); + assertEq(mem.bytes[0], 255, "u8 max"); + storeInt(mem, -128, 0, 1, true); + assertEq(mem.view.getInt8(0), -128, "s8 min"); + storeInt(mem, 127, 0, 1, true); + assertEq(mem.view.getInt8(0), 127, "s8 max"); + + storeInt(mem, 0, 8, 2, false); + assertEq(mem.view.getUint16(8, true), 0, "u16 min"); + storeInt(mem, 0xffff, 8, 2, false); + assertEq(mem.view.getUint16(8, true), 0xffff, "u16 max"); + storeInt(mem, -32768, 8, 2, true); + assertEq(mem.view.getInt16(8, true), -32768, "s16 min"); + storeInt(mem, 32767, 8, 2, true); + assertEq(mem.view.getInt16(8, true), 32767, "s16 max"); + + storeInt(mem, 0, 16, 4, false); + assertEq(mem.view.getUint32(16, true), 0, "u32 min"); + storeInt(mem, 0xffffffff, 16, 4, false); + assertEq(mem.view.getUint32(16, true), 0xffffffff, "u32 max"); + storeInt(mem, -2147483648, 16, 4, true); + assertEq(mem.view.getInt32(16, true), -2147483648, "s32 min"); + storeInt(mem, 2147483647, 16, 4, true); + assertEq(mem.view.getInt32(16, true), 2147483647, "s32 max"); + + storeInt(mem, 0n, 24, 8, false); + assertEq(mem.view.getBigUint64(24, true), 0n, "u64 min"); + storeInt(mem, 0xffffffffffffffffn, 24, 8, false); + assertEq(mem.view.getBigUint64(24, true), 0xffffffffffffffffn, "u64 max"); + storeInt(mem, -(1n << 63n), 24, 8, true); + assertEq(mem.view.getBigInt64(24, true), -(1n << 63n), "s64 min"); + storeInt(mem, (1n << 63n) - 1n, 24, 8, true); + assertEq(mem.view.getBigInt64(24, true), (1n << 63n) - 1n, "s64 max"); +}); + +Deno.test("storeInt: out-of-range u8 raises the host-precondition error, not a silent wrap", () => { + const mem = freshMem(); + assertRangeError(() => storeInt(mem, 300, 0, 1, false), "u8 over max"); + assertRangeError(() => storeInt(mem, -1, 0, 1, false), "u8 under min"); + assertEq(mem.bytes[0], 0, "memory untouched by a rejected store"); +}); + +Deno.test("storeInt: out-of-range s16 raises the host-precondition error", () => { + const mem = freshMem(); + assertRangeError(() => storeInt(mem, 32768, 0, 2, true), "s16 over max"); + assertRangeError(() => storeInt(mem, -32769, 0, 2, true), "s16 under min"); +}); + +Deno.test("storeInt: out-of-range u32 raises the host-precondition error", () => { + const mem = freshMem(); + assertRangeError(() => storeInt(mem, 0x1_0000_0000, 0, 4, false), "u32 over max"); + assertRangeError(() => storeInt(mem, -1, 0, 4, false), "u32 under min"); +}); + +Deno.test("storeInt: out-of-range s64 (bigint) raises the host-precondition error", () => { + const mem = freshMem(); + assertRangeError(() => storeInt(mem, 1n << 63n, 0, 8, true), "s64 over max"); + assertRangeError(() => storeInt(mem, -(1n << 63n) - 1n, 0, 8, true), "s64 under min"); +}); + +Deno.test("storeInt: out-of-range u64 (bigint) raises the host-precondition error", () => { + const mem = freshMem(); + assertRangeError( + () => storeInt(mem, 1n << 64n, 0, 8, false), + "u64 over max", + ); + assertRangeError(() => storeInt(mem, -1n, 0, 8, false), "u64 under min"); +}); diff --git a/runtime/tests/streams_teardown_test.ts b/runtime/tests/streams_teardown_test.ts new file mode 100644 index 0000000..4f8f635 --- /dev/null +++ b/runtime/tests/streams_teardown_test.ts @@ -0,0 +1,528 @@ +// #84 / #90 / #97: what a *future*'s reader observes when the writable side +// goes away without ever delivering a value. +// +// Background. `SharedStreamImpl` and `SharedFutureImpl` share a rendezvous +// shape, but they do NOT share this outcome: a stream reader may observe +// `CopyResult.DROPPED` (that is end-of-stream), a future reader may not +// (definitions.py:2614 asserts it). The reference keeps the state unreachable +// by trapping an early writable-future drop (definitions.py:1183-1184) — a +// guarantee a *trap-poisoned* instance can no longer be asked to honour, and +// one the host's public `drop()` door bypasses too. Both are handled by the +// same mechanism: the future is marked abandoned (task/streams.ts +// `abandonSharedFuture` / the retirement walk) and its reader gets a `Trap` at +// its rendezvous point — never DROPPED, never COMPLETED, never a hang, never +// an internal `AssertionError`. +// +// Reader timings covered here: +// (a) parked async reader (callback ABI) — the trap arrives through +// waitable-set delivery; +// (b) parked sync/JSPI reader — the trap arrives as the suspension point's +// rejection (`produce` throws); +// (c) reader that has not parked yet — `future.read` traps on the spot. +// +// Harness note: these are direct built-in tests (the `async_builtins_test.ts` +// pattern) rather than guest-fixture tests. The states under test need two +// component instances, one of which is poisoned mid-park with a *specific* +// end-ownership split; the existing `examples/guests/stream-pass` fixture +// cannot express the read-after-teardown and multi-end-walk shapes at all, +// and the host-side machinery reaches every one of them exactly. + +import { assertEq } from "./support/asserts.ts"; +import { AssertionError, Trap } from "../src/cabi/mod.ts"; +import { + createFutureRead, + createStreamRead, +} from "../src/intrinsics/stream_builtins.ts"; +import { + BLOCKED, + createWaitableSetWait, +} from "../src/intrinsics/async_builtins.ts"; +import type { ResolvedOptions } from "../src/exec/boundary.ts"; +import { HostBuffer, hostFuture, hostStream } from "../src/exec/mod.ts"; +import { + BUFFER_MAX_LENGTH, + ComponentInstanceState, + CopyResult, + CopyState, + popCurrentThread, + pushCurrentThread, + ReadableFutureEnd, + ReadableStreamEnd, + retireInstanceAsyncEnds, + SharedFutureImpl, + SharedStreamImpl, + Store, + Task, + type TaskOptions, + Thread, + WaitableSet, + WritableFutureEnd, + WritableStreamEnd, +} from "../src/task/mod.ts"; +import type { FuncType } from "../src/cabi/types.ts"; + +function assert(cond: boolean, msg: string): asserts cond { + if (!cond) throw new Error(`assertion failed: ${msg}`); +} + +const ASYNC_FT: FuncType = { params: [], results: [], async: true }; +const CALLBACK_OPTS: TaskOptions = { + async_: true, + callback: true, + stringEncoding: "utf8", + memory: null, +}; + +/** A live `MemInst` view over a real WebAssembly.Memory. */ +function mkMemory() { + const memory = new WebAssembly.Memory({ initial: 1 }); + return { + memory, + view: { + addrType: "i32" as const, + get bytes() { + return new Uint8Array(memory.buffer); + }, + get view() { + return new DataView(memory.buffer); + }, + get length() { + return memory.buffer.byteLength; + }, + ptrType: () => "i32" as const, + ptrSize: () => 4 as const, + }, + }; +} + +/** + * Two instances of one store sharing one *unwritten* future: + * + * * `writer` (instance 0) holds the `WritableFutureEnd` — this is the one + * the tests poison; + * * `reader` (instance 1) holds the `ReadableFutureEnd` and stays healthy. + * + * The element type is the zero-width payload (`future` with no value type), + * which keeps the buffers memory-free without changing any of the rendezvous + * logic under test. + */ +function mkFutureSplit(mode: "plain" | "jspi" = "plain") { + const store = new Store(); + const writer = new ComponentInstanceState(0, store); + const reader = new ComponentInstanceState(1, store); + const { memory, view } = mkMemory(); + const shared = new SharedFutureImpl(null); + const wi = writer.handles.add(new WritableFutureEnd(shared)); + const readEnd = new ReadableFutureEnd(shared); + const ri = reader.handles.add(readEnd); + const opts: ResolvedOptions = { + stringEncoding: "utf8", + // deno-lint-ignore no-explicit-any + memory: view as any, + realloc: null, + postReturn: null, + callback: null, + async: true, + cancellable: false, + coreType: { params: ["i32", "i32"], results: ["i32"] }, + instance: reader, + }; + const ctx = { + componentInstance: () => reader, + options: () => opts, + streamElem: () => null, + futureElem: () => null, + resultTypes: () => [], + suspensionMode: mode, + }; + const task = new Task(ASYNC_FT, CALLBACK_OPTS, reader, () => [], () => {}); + const thread = new Thread(task, (function* () {})()); + return { + store, + writer, + reader, + shared, + readEnd, + ri, + wi, + memory, + ctx, + task, + thread, + read: createFutureRead({ futureTable: 0, options: 0 }, ctx, reader), + wait: createWaitableSetWait({ options: 0 }, ctx, reader), + /** Model the broken enter/leave bracket the walk is always called under. */ + poison(cause: unknown) { + writer.mayEnter = false; + retireInstanceAsyncEnds(writer, cause); + }, + run(fn: () => T): T { + pushCurrentThread(thread); + try { + return fn(); + } finally { + popCurrentThread(thread); + } + }, + }; +} + +function caughtSync(fn: () => unknown): unknown { + try { + fn(); + } catch (e) { + return e; + } + return undefined; +} + +async function caughtAsync(p: PromiseLike): Promise { + try { + await p; + } catch (e) { + return e; + } + return undefined; +} + +/** Every leg makes the same three claims about the observed failure. */ +function assertAbandonTrap(e: unknown, causeIncludes: string): void { + assert( + !(e instanceof AssertionError), + `must not be an internal AssertionError: ${e}`, + ); + assert(e instanceof Trap, `expected a Trap, got ${Deno.inspect(e)}`); + assert( + String(e.message).includes("can never complete"), + `names the outcome: ${e.message}`, + ); + const cause = (e as { cause?: unknown }).cause; + assert(cause instanceof Error, `carries the recorded cause: ${cause}`); + assert( + String(cause.message).includes(causeIncludes), + `cause names the fault (${causeIncludes}): ${cause.message}`, + ); +} + +// --------------------------------------------------------------------------- +// #84 (a): parked async reader, delivery through the waitable set +// --------------------------------------------------------------------------- + +Deno.test("#84(a): a parked async future reader traps when the writer's instance is poisoned", () => { + const f = mkFutureSplit(); + const wset = new WaitableSet(); + const seti = f.reader.handles.add(wset); + f.readEnd.join(wset); + + // The reader parks: async `future.read` with nobody on the other side. + assertEq(f.run(() => f.read(f.ri, 0)), BLOCKED); + assertEq(f.readEnd.state, CopyState.COPYING); + // Healthy: the reader's instance is enterable while it is parked. This is + // the property `dropSharedForTeardown`'s notify/silently-retire test rests + // on (see its #84 AUDIT note). + assertEq(f.reader.mayEnter, true); + + const boom = new Trap("unreachable"); + f.poison(boom); + + // An event landed (so the reader is not stranded)... + assertEq(f.readEnd.hasPendingEvent(), true); + // ...and taking it through waitable-set delivery traps the reader task. + const e = caughtSync(() => f.run(() => f.wait(seti, 64))); + assertAbandonTrap(e, "trapped while it held an end"); + assertEq((e as { cause?: { cause?: unknown } }).cause?.cause, boom); + // The thunk is consumed exactly once — no phantom event is left behind. + assertEq(f.readEnd.hasPendingEvent(), false); +}); + +Deno.test("#84(a'): a stream reader keeps the spec-shaped DROPPED outcome", () => { + // The contrast case: DROPPED *is* end-of-stream for a stream, so the walk + // must not turn it into a trap. + const store = new Store(); + const writer = new ComponentInstanceState(0, store); + const reader = new ComponentInstanceState(1, store); + const { view } = mkMemory(); + const shared = new SharedStreamImpl(null); + writer.handles.add(new WritableStreamEnd(shared)); + const readEnd = new ReadableStreamEnd(shared); + const ri = reader.handles.add(readEnd); + const opts: ResolvedOptions = { + stringEncoding: "utf8", + // deno-lint-ignore no-explicit-any + memory: view as any, + realloc: null, + postReturn: null, + callback: null, + async: true, + cancellable: false, + coreType: { params: ["i32", "i32"], results: ["i32"] }, + instance: reader, + }; + const ctx = { + componentInstance: () => reader, + options: () => opts, + streamElem: () => null, + futureElem: () => null, + resultTypes: () => [], + suspensionMode: "plain" as const, + }; + const read = createStreamRead({ streamTable: 0, options: 0 }, ctx, reader); + const task = new Task(ASYNC_FT, CALLBACK_OPTS, reader, () => [], () => {}); + const thread = new Thread(task, (function* () {})()); + pushCurrentThread(thread); + try { + assertEq(read(ri, 0, 4), BLOCKED); + } finally { + popCurrentThread(thread); + } + writer.mayEnter = false; + retireInstanceAsyncEnds(writer, new Trap("unreachable")); + assertEq(readEnd.hasPendingEvent(), true); + const [, , payload] = readEnd.getPendingEvent(); + // definitions.py packs `result | (progress << 4)`: DROPPED with 0 progress. + assertEq(payload & 0xf, CopyResult.DROPPED); +}); + +// --------------------------------------------------------------------------- +// #84 (b): parked sync/JSPI reader — the suspension point's `produce` throws +// --------------------------------------------------------------------------- + +Deno.test("#84(b): a JSPI-blocked future reader's suspension rejects with the trap", async () => { + const f = mkFutureSplit("jspi"); + // Synchronous (`async: false`) copy: `finishCopy` SITE 4 blocks the wasm + // frame via `blockCurrentActivation`, which hands back a Promise. + const syncOpts = { ...f.ctx.options(), async: false }; + const syncCtx = { ...f.ctx, options: () => syncOpts }; + const read = createFutureRead({ futureTable: 0, options: 0 }, syncCtx, f.reader); + const parked = f.run(() => read(f.ri, 0)) as unknown as Promise; + assertEq(f.readEnd.state, CopyState.COPYING); + assertEq(f.readEnd.hasSyncWaiter, true); + // A JSPI-blocked peer parks OUTSIDE the enter bracket, exactly like a + // callback-ABI one: blocking inside the wasm frame does not hold `mayEnter` + // (#84 audit item; see `dropSharedForTeardown`). + assertEq(f.reader.mayEnter, true); + + f.poison(new Trap("unreachable")); + // The scheduler resumes the suspension point; `produce` throws, which the + // bridge turns into the promise's rejection (the engine's trap path). + assertEq(f.store.tick(), true); + assertAbandonTrap(await caughtAsync(parked), "trapped while it held an end"); +}); + +// --------------------------------------------------------------------------- +// #84 (c): the reader had not parked yet +// --------------------------------------------------------------------------- + +Deno.test("#84(c): future.read after the teardown traps instead of asserting", () => { + const f = mkFutureSplit(); + f.poison(new Trap("unreachable")); + const e = caughtSync(() => f.run(() => f.read(f.ri, 0))); + assertAbandonTrap(e, "trapped while it held an end"); +}); + +Deno.test("#84(c'): a spec-dropped future (the writer delivered its value) is untouched", () => { + // Distinguishing teardown-dropped from spec-dropped: a future whose value + // WAS delivered carries no abandonment reason, so nothing about the normal + // path changes. definitions.py:1183-1184 makes the writable end's drop legal + // only in this state. + const f = mkFutureSplit(); + const buf = new HostBuffer(null, [null], 1); + let result: CopyResult | null = null; + // A host writer parks its one value; the guest reader takes it. + f.shared.write({ host: "w" }, buf as never, (r) => result = r); + assertEq(f.run(() => f.read(f.ri, 0)), CopyResult.COMPLETED); + assertEq(result, CopyResult.COMPLETED); + assertEq(f.shared.abandonReason, null); + // The end is DONE, so the walk owes the reader nothing. + const end = f.writer.handles.get(f.wi) as WritableFutureEnd; + end.state = CopyState.DONE; + f.poison(new Trap("unreachable")); + assertEq(f.shared.abandonReason, null); +}); + +// --------------------------------------------------------------------------- +// #84: the walk completes even when one end's notification throws +// --------------------------------------------------------------------------- + +Deno.test("#84: one end's failing notification does not strand the remaining ends", () => { + const store = new Store(); + const inst = new ComponentInstanceState(0, store); + const peer = { mayEnter: true }; + + // End 1: a stream whose parked peer's settler throws (a host callback, an + // event thunk — anything the notification runs). + const bad = new SharedStreamImpl(null); + const boom = new Error("peer settler exploded"); + bad.setPending(peer, new HostBuffer(null, null, 4) as never, () => {}, () => { + throw boom; + }); + inst.handles.add(new WritableStreamEnd(bad)); + + // End 2 and 3: ordinary ends that must still be retired. + const okStream = new SharedStreamImpl(null); + let okResult: CopyResult | null = null; + okStream.setPending( + peer, + new HostBuffer(null, null, 4) as never, + () => {}, + (r) => okResult = r, + ); + inst.handles.add(new ReadableStreamEnd(okStream)); + const okFuture = new SharedFutureImpl(null); + inst.handles.add(new WritableFutureEnd(okFuture)); + + inst.mayEnter = false; + const raised = caughtSync(() => retireInstanceAsyncEnds(inst, new Trap("x"))); + + // The first failure is rethrown... + assertEq(raised, boom); + // ...and every other end was still retired. + assertEq(okResult, CopyResult.DROPPED); + assertEq(okStream.dropped, true); + assertEq(okFuture.dropped, true); + assert( + okFuture.abandonReason instanceof Error, + "the unwritten future is marked abandoned", + ); + assertEq(bad.dropped, true); +}); + +// --------------------------------------------------------------------------- +// #90: host drop-before-write on a lowered future +// --------------------------------------------------------------------------- + +/** Lower a host future into `inst`, the way `lower_future` does. */ +function lowerInto( + host: { value: unknown }, + inst: ComponentInstanceState, +): number { + const shared = host.value as SharedFutureImpl; + (shared as { boundStore?: unknown }).boundStore ??= inst.store; + (shared as { onLowered?: ((i: unknown) => void) | null }).onLowered?.(inst); + return inst.handles.add(new ReadableFutureEnd(shared)); +} + +Deno.test("#90: dropping a lowered, never-written host future traps its parked reader", () => { + const f = mkFutureSplit(); + // Re-do the split with a *host* writable side: the host future's shared + // object is the one the guest reads. + const store = new Store(); + const reader = new ComponentInstanceState(1, store); + const host = hostFuture(null); + const ri = lowerInto(host, reader); + const readEnd = reader.handles.get(ri) as ReadableFutureEnd; + const opts = { ...f.ctx.options(), instance: reader }; + const ctx = { ...f.ctx, componentInstance: () => reader, options: () => opts }; + const read = createFutureRead({ futureTable: 0, options: 0 }, ctx, reader); + const task = new Task(ASYNC_FT, CALLBACK_OPTS, reader, () => [], () => {}); + const thread = new Thread(task, (function* () {})()); + const run = (fn: () => T): T => { + pushCurrentThread(thread); + try { + return fn(); + } finally { + popCurrentThread(thread); + } + }; + const wset = new WaitableSet(); + const seti = reader.handles.add(wset); + readEnd.join(wset); + assertEq(run(() => read(ri, 0)), BLOCKED); + + // The public door: no throw, and the parked reader is armed. + host.drop(); + host.drop(); // idempotent + const wait = createWaitableSetWait({ options: 0 }, ctx, reader); + const e = caughtSync(() => run(() => wait(seti, 64))); + assertAbandonTrap(e, "without writing a value"); +}); + +Deno.test("#90: dropping a lowered, never-written host future with no reader parked", () => { + const store = new Store(); + const reader = new ComponentInstanceState(1, store); + const host = hostFuture(null); + lowerInto(host, reader); + host.drop(); + const shared = host.value as unknown as SharedFutureImpl; + assertEq(shared.dropped, true); + assert(shared.abandonReason instanceof Error, "marked abandoned"); + // ...and the reader that shows up later traps rather than tripping the + // `future read shape` assert (task/streams.ts). + const e = caughtSync(() => + shared.read({}, new HostBuffer(null, null, 1) as never, () => {}) + ); + assertAbandonTrap(e, "without writing a value"); +}); + +Deno.test("#90: an unlowered future's drop is plain cleanup, and dispose never throws", () => { + const host = hostFuture(null); + host.drop(); + const shared = host.value as unknown as SharedFutureImpl; + assertEq(shared.dropped, true); + assertEq(shared.abandonReason, null); + host.drop(); +}); + +Deno.test("#90: write-then-drop is unchanged", async () => { + const store = new Store(); + const reader = new ComponentInstanceState(1, store); + const host = hostFuture(null); + lowerInto(host, reader); + const w = host.write(null); + // The guest reader takes the value. + const shared = host.value as unknown as SharedFutureImpl; + let taken: CopyResult | null = null; + shared.read({ guest: 1 }, new HostBuffer(null, null, 1) as never, (r) => + taken = r); + await w; + assertEq(taken, CopyResult.COMPLETED); + host.drop(); + assertEq(shared.abandonReason, null); +}); + +// --------------------------------------------------------------------------- +// #97: the host buffer bound +// --------------------------------------------------------------------------- + +Deno.test("#97: HostBuffer enforces Buffer.MAX_LENGTH", () => { + assertEq(BUFFER_MAX_LENGTH, 2 ** 28 - 1); + // At the bound: allowed (definitions.py:938 asserts `<= MAX_LENGTH`). + const ok = new HostBuffer(null, null, BUFFER_MAX_LENGTH); + assertEq(ok.remain(), BUFFER_MAX_LENGTH); + for (const bad of [BUFFER_MAX_LENGTH + 1, 2 ** 31, Number.MAX_SAFE_INTEGER]) { + const e = caughtSync(() => new HostBuffer(null, null, bad)); + assert(e instanceof RangeError, `loud typed error for ${bad}: ${e}`); + assert( + String((e as Error).message).includes("MAX_LENGTH"), + `names the bound: ${(e as Error).message}`, + ); + } + // Nonsense lengths are rejected at the same door. + assert( + caughtSync(() => new HostBuffer(null, null, -1)) instanceof RangeError, + "negative length rejected", + ); +}); + +// --------------------------------------------------------------------------- +// #97: a host-cancelled read is indistinguishable from EOS — pinned +// --------------------------------------------------------------------------- + +Deno.test("#97: cancelRead resolves the read exactly like end-of-stream does", async () => { + // DELIBERATE (see the doc comments at exec/host_streams.ts + // `HostReadableEnd.cancelRead` and embedder/streams.ts `Stream.cancelRead`): + // both outcomes hand back the empty chunk, which the conventions layer + // reads as a clean end. This test exists so the equivalence cannot be + // changed silently — the canceller is the observer, so the ambiguity is + // resolvable by the only code that can see it. + const cancelled = hostStream(null); + const cancelledRead = cancelled.readable.read(4); + cancelled.readable.cancelRead(); + assertEq(await cancelledRead, []); + + const ended = hostStream(null); + const endedRead = ended.readable.read(4); + // The writable end going away is genuine end-of-stream. + ended.writable.drop(); + assertEq(await endedRead, []); +}); diff --git a/runtime/tests/tls_smoke_pins_test.ts b/runtime/tests/tls_smoke_pins_test.ts index 237b022..d774047 100644 --- a/runtime/tests/tls_smoke_pins_test.ts +++ b/runtime/tests/tls_smoke_pins_test.ts @@ -48,6 +48,8 @@ function minimalPlan(overrides: Partial = {}): WirePlan { canonicalOptions: [], types: [], resourceTables: [], + streamTables: [], + futureTables: [], imports: [], exports: [], worldDigest: "sha256:0", diff --git a/runtime/tests/transcode_test.ts b/runtime/tests/transcode_test.ts index e81be24..3892e92 100644 --- a/runtime/tests/transcode_test.ts +++ b/runtime/tests/transcode_test.ts @@ -244,3 +244,69 @@ Deno.test("transcode: memories are re-read after growth", () => { fn(0, 3, 64); assertEq(read(m, 64, 3), [1, 2, 3]); }); + +// --------------------------------------------------------------------------- +// Defensive guards (issue #96): unreachable under FACT's real guarantees — +// exercised here by calling the intrinsic directly with an invariant FACT +// itself would never violate (a too-small dst capacity / overlapping +// regions), which is exactly what a synthetic unit-level test needs to +// reach without going through a full FACT-generated call. +// --------------------------------------------------------------------------- + +Deno.test("transcode: utf8-to-compact-utf16 traps when dst capacity is exceeded", () => { + const m = mem(); + // "abc" is 3 latin1-ineligible... actually any 3-char ASCII string needs 3 + // u16 units; advertise a dst capacity of only 2 units total (dstLen=2, + // latin1BytesSoFar=0) so `capacity (2) < s.length (3)`. + const rest = new TextEncoder().encode("abc"); + write(m, 0, [...rest]); + assertTraps( + () => call("utf8-to-compact-utf16", m, m, 0, rest.length, 64, 2, 0), + "destination capacity exceeded", + ); +}); + +Deno.test("transcode: utf8-to-compact-utf16 does not trap when dst capacity exactly fits", () => { + const m = mem(); + const rest = new TextEncoder().encode("abc"); + write(m, 0, [...rest]); + // latin1BytesSoFar=1 (1 unit already written) + 3 more units == dstLen 4. + write(m, 64, [0x7a]); + const units = call( + "utf8-to-compact-utf16", + m, + m, + 0, + rest.length, + 64, + 4, + 1, + ) as number; + assertEq(units, 4); +}); + +Deno.test("transcode: utf16-to-latin1 traps on overlapping src/dst in the same memory", () => { + const m = mem(); + write(m, 0, u16le("ab\u0100c")); + // dst at byte 2 overlaps the src u16 range [0, 8) in the same memory. + assertTraps( + () => call("utf16-to-latin1", m, m, 0, 4, 2), + "overlap", + ); +}); + +Deno.test("transcode: utf16-to-latin1 does not trap on non-overlapping regions", () => { + const m = mem(); + write(m, 0, u16le("ab\u0100c")); + assertEq(call("utf16-to-latin1", m, m, 0, 4, 64), [2, 2]); + assertEq(read(m, 64, 2), [0x61, 0x62]); +}); + +Deno.test("transcode: utf16-to-latin1 does not trap across independent memories", () => { + const from = mem(); + const to = mem(); + write(from, 0, u16le("ab\u0100c")); + // Same offsets in independent ArrayBuffers must not be flagged as overlap. + assertEq(call("utf16-to-latin1", from, to, 0, 4, 0), [2, 2]); + assertEq(read(to, 0, 2), [0x61, 0x62]); +}); diff --git a/upstream-component-model-repo-findings.md b/upstream-component-model-repo-findings.md index 02cf6bd..b6c9f29 100644 --- a/upstream-component-model-repo-findings.md +++ b/upstream-component-model-repo-findings.md @@ -243,3 +243,56 @@ fails the guest's own assertion. Worth an upstream doc note on `test/async` (tests assume the deterministic profile) or making the guests order-tolerant. Hosts adding seeded-schedule testing should profile-scope pins for such fixtures (we did). + +--- + +## CM-5: `SharedFutureImpl.drop`'s pending-buffer assert looks internally inconsistent + +**Status:** DRAFT — candidate upstream issue against `definitions.py` +**Found:** 2026-08-10, adversarial conformance review of the stream/future +territory (deltic#84/#98) + +### Evidence + +`definitions.py:1150` (`SharedFutureImpl.drop`): + +```python +if self.pending_buffer: + assert(isinstance(self.pending_buffer, WritableBuffer)) +``` + +The assert says: if anything is parked on the shared future when an end +drops, the parked side is a *reader* (a future read parks a WritableBuffer +— the buffer the value will be written into). But by the surrounding rules +that state is unreachable from the drop paths: + +- a **writable** end may not drop before delivering its value + (`WritableFutureEnd.drop`, definitions.py:1183-1184 traps unless + `state == DONE`) — so a drop can never find the *reader* still parked via + this path with the value undelivered; +- a **readable** end that parked its read is the pending side itself; when + the reader end drops, `CopyEnd.drop` (definitions.py:1098-1101) traps on + a busy end before reaching the shared drop; +- and `definitions.py:2614` independently asserts a readable future end can + never observe DROPPED. + +The only guest-reachable pending side at shared-drop time is therefore a +*writer* (pending `ReadableBuffer`, reader dropped first — legal), which is +exactly what the assert rejects. Either the assert is inverted, or it +documents an invariant whose enforcing traps make the guarded branch dead; +in both readings it does not describe reachable states. + +### deltic disposition + +deltic's port omits the assert (`runtime/src/task/streams.ts`, +`SharedFutureImpl.drop`) — its teardown extension (#66/#84) *deliberately* +creates the writer-died-unwritten state for trap-poisoned instances and +resolves it with a reader-side trap, which the assert would spuriously kill. +No behavioral divergence on spec-reachable states. + +### Suggested upstream fix + +Either delete the assert (the neighboring traps already enforce the real +invariants) or flip it to assert the reachable shape +(`isinstance(self.pending_buffer, ReadableBuffer)`) with a comment naming +the reader-dropped-first case.