diff --git a/runtime/src/exec/boundary.ts b/runtime/src/exec/boundary.ts index 28865fd..55e5bfc 100644 --- a/runtime/src/exec/boundary.ts +++ b/runtime/src/exec/boundary.ts @@ -1668,6 +1668,15 @@ export function createLoweredImport(input: { // and the embedder's per-declaration `suspending()` marker: the // Suspending wrap is applied per-declaration (`importValue`), so an // unmarked import physically cannot suspend, whatever the mode. + // + // A capability signal is expressly NON-poisoning (amendment 2, #91 + // scope clarification): the caller keeps running, so the borrows + // `onStart` lifted into this subtask must be discharged here or + // its lenders stay elevated forever and later `resource.drop`s + // trap "handle still lent out" on a healthy instance (found + // during the #106 closure; same class as the fact_calls.ts #91 + // sites). + subtask.unwindLenders(); needsJspi( suspendable ? `synchronous lower of import '${name}', whose host ` + @@ -1720,6 +1729,32 @@ export function createLoweredImport(input: { // precondition of the deadlock verdict) and so teardown can observe // the outstanding call, mirroring the async arm below. store.pendingHostCalls.add(promise); + // LENDER DISCHARGE ON EVERY SETTLE PATH (#106, the sibling of the + // fact_calls.ts sync-start park's #102 enumeration): + // + // * produce SUCCESS -> `onResolve` + `deliverResolve` release the + // lenders; the `onSettled` backstop below observes + // `resolveDelivered()` and is a no-op. + // * produce THROW -> exempt-by-poisoning under amendment 2 + // (contracts/intrinsics.md v0.2 §2: release is owed only on exits + // that do NOT poison the caller). Every rejection that reaches + // this park is a poisoning trap in the CALLER's own frame: + // branded `WitError`s on fallible imports were already resolved + // into err-shaped VALUES by the conventions layer + // (embedder/instantiate.ts `#wrapImportFn`'s `fail` — they take + // the success arm above), every other conventions-layer throw is + // a `Trap`, and a raw-executor rejection is a declared host bug + // that traps (empirical fact (e)). No capability signal can + // originate inside `produce`: this park only exists once jspi + + // `suspending()` were both granted. The backstop's unwind here is + // belt-and-braces bookkeeping on a poisoned instance, not an + // obligation. + // * abandon -> produce never runs, and an abandoned park + // does NOT poison the caller (pinned by + // resource_lender_park_settle_test.ts) — without the hook the + // subtask's lenders stayed elevated forever and later + // `resource.drop`s trapped "handle still lent out". The hook is + // the fix. return blockCurrentActivation({ store, task: currentTask(), @@ -1733,8 +1768,9 @@ export function createLoweredImport(input: { // which the engine turns back into a wasm trap (empirical // fact (e); `SuspensionPoint` routes a produce-throw through // exactly that path). Branded `WitError`s never reach the raw - // boundary — the bindgen adapter resolves them into err-shaped - // values one layer up. + // boundary — the conventions layer resolves them into + // err-shaped values one layer up (see the settle-path + // enumeration above). throw done.error; } onResolve(toResults(done.value)); @@ -1745,6 +1781,7 @@ export function createLoweredImport(input: { if (flatResults.length === 1) return flatResults[0]; return flatResults; }, + onSettled: () => subtask.unwindLenders(), }); } const promise = Promise.resolve(raw).then( diff --git a/runtime/src/intrinsics/async_builtins.ts b/runtime/src/intrinsics/async_builtins.ts index 981771e..eccb7b5 100644 --- a/runtime/src/intrinsics/async_builtins.ts +++ b/runtime/src/intrinsics/async_builtins.ts @@ -310,8 +310,15 @@ export function createWaitableSetWait( // The `num_waiting` bracket is real now. Skipping it was justified only // while this path could not actually yield; a genuine block CAN be // observed, because `WaitableSet.drop` traps on `num_waiting > 0` - // (line 852). Incremented before blocking and decremented in `produce`, - // which runs exactly once whether we resume normally or cancelled. + // (line 852). Incremented before blocking and decremented in + // `onSettled`, which runs exactly once on EVERY terminal transition — + // normal resume, cancelled resume, produce-throw, and `abandon` + // (#106: decrementing in `produce` missed the abandon leg, leaving + // `numWaiting` elevated forever and a later `waitable-set.drop` + // trapping spuriously). The decrement is not idempotent, so it lives + // ONLY here, not in `produce` as well; nothing can observe the still- + // elevated count between `produce` and the hook — both run + // synchronously inside the settle, before any other code. wset.numWaiting += 1; return blockCurrentActivation({ store: inst.store, @@ -319,12 +326,14 @@ export function createWaitableSetWait( readyFunc: () => wset.hasPendingEvent(), cancellable, produce: (cancelled: Cancelled) => { - wset.numWaiting -= 1; const ev: EventTuple = cancelled ? [EventCode.TASK_CANCELLED, 0, 0] : wset.getPendingEvent(); return unpackEvent(opts, inst, ptr ?? 0, ev); }, + onSettled: () => { + wset.numWaiting -= 1; + }, }) as unknown as number; } else { needsJspi( @@ -506,6 +515,13 @@ export function createSubtaskCancel( st.hasSyncWaiter = false; return finish(); }, + // #106: `abandon` never runs `produce`; without the backstop + // the flag stayed set forever and a later `waitable.join` on + // this subtask trapped spuriously. Idempotent, so the success + // path's clear-inside-`produce` ordering is untouched. + onSettled: () => { + st.hasSyncWaiter = false; + }, }) as unknown as number; } needsJspi( diff --git a/runtime/src/intrinsics/fact_calls.ts b/runtime/src/intrinsics/fact_calls.ts index bf71095..b6d6abc 100644 --- a/runtime/src/intrinsics/fact_calls.ts +++ b/runtime/src/intrinsics/fact_calls.ts @@ -782,24 +782,12 @@ 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. + * Now a thin alias of `Subtask.unwindLenders` — the same unwind serves the + * host-import parks (exec/boundary.ts, #106) — kept for the local name the + * `[async-start-call]` comments reference. */ 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(); + subtask.unwindLenders(); } /** The core-ABI shape of a returned results vector (0 / 1 / many). */ diff --git a/runtime/src/intrinsics/stream_builtins.ts b/runtime/src/intrinsics/stream_builtins.ts index b2bbb43..68653d4 100644 --- a/runtime/src/intrinsics/stream_builtins.ts +++ b/runtime/src/intrinsics/stream_builtins.ts @@ -346,6 +346,14 @@ function finishCopy( traceCopy(`${what} sync copy i=${i} RESUME -> 0x${p.toString(16)}`); return p; }, + // #106: `abandon` never runs `produce`; without the backstop the + // flag stayed set forever and a later `cancel-copy` on this end + // trapped "sync waiter" against a waiter that no longer exists. + // Idempotent, so the success path's clear-before-`take` ordering + // inside `produce` is untouched. + onSettled: () => { + end.hasSyncWaiter = false; + }, }) as unknown as number; } needsJspi( diff --git a/runtime/src/task/subtask.ts b/runtime/src/task/subtask.ts index 18bfe63..804fc5a 100644 --- a/runtime/src/task/subtask.ts +++ b/runtime/src/task/subtask.ts @@ -105,6 +105,34 @@ export class Subtask extends Waitable { return this.lenders === null; } + /** + * Release a never-delivered subtask's lenders after its call broke off a + * non-poisoning exit — trap-rethrow past the CALLEE, capability bail, or + * an abandoned park (contracts/intrinsics.md v0.2 amendment 2, #91 scope + * clarification; the park legs are #102/#106). + * + * The reference has no analogue because it never resumes after a trap: + * the store dies with the lent handles inside it. 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. + * + * Idempotent, and a no-op when the resolution was already delivered — a + * settled hook can call it unconditionally without disturbing the success + * path's own `deliverResolve`. + */ + unwindLenders(): void { + if (!this.resolved()) { + this.resolve( + this.state === SubtaskState.STARTING + ? SubtaskState.CANCELLED_BEFORE_STARTED + : SubtaskState.CANCELLED_BEFORE_RETURNED, + [], + ); + } + if (!this.resolveDelivered()) this.deliverResolve(); + } + /** definitions.py `Subtask.drop` (line 912). */ override drop(): void { trapIf( diff --git a/runtime/tests/park_state_settle_test.ts b/runtime/tests/park_state_settle_test.ts new file mode 100644 index 0000000..5a43f8f --- /dev/null +++ b/runtime/tests/park_state_settle_test.ts @@ -0,0 +1,381 @@ +// Park-scoped state must be discharged on EVERY settle path (issue #106) — +// the cleanup-only-in-produce siblings enumerated by the #102 track. +// +// Authority: contracts/intrinsics.md v0.2 amendment 2 (#91 scope +// clarification) for the lender sites; `SuspensionPoint.onSettled` +// (jspi/bridge.ts, #102) is the mechanism. The settle-path enumeration these +// tests walk is the one pinned in resource_lender_park_settle_test.ts: +// produce-success / produce-throw / abandon (a cancelled resume of a +// non-cancellable park is not a settle, and never-settled dies with the +// store). +// +// Sites covered, matching the #106 numbering: +// 1. stream sync copy park — `end.hasSyncWaiter` (SITE 4 lit) +// 2. waitable-set.wait park — `wset.numWaiting` (SITE 2 lit) +// 3. subtask.cancel sync park — `st.hasSyncWaiter` (SITE 5 lit) +// 4. sync host-import park — the subtask's LENDERS; produce-throw is +// exempt-by-poisoning (analysis at the site), abandon is not (#102 pins +// an abandoned park as non-poisoning), so the hook discharges it. +// 5. (found during closure) the `needsJspi` bail in the same lowering runs +// AFTER `onStart` lifted borrows — a capability signal is expressly +// non-poisoning and must not strand lenders. + +import { assertEq } from "./support/asserts.ts"; +import { + createSubtaskCancel, + createWaitableSetDrop, + createWaitableSetWait, +} from "../src/intrinsics/async_builtins.ts"; +import { + createStreamCancelRead, + createStreamRead, +} from "../src/intrinsics/stream_builtins.ts"; +import { + createLoweredImport, + newStats, + type ResolvedOptions, +} from "../src/exec/boundary.ts"; +import { + ComponentInstanceState, + NeedsJspi, + popCurrentThread, + pushCurrentThread, + ReadableStreamEnd, + SharedStreamImpl, + Store, + Subtask, + Task, + type TaskOptions, + Thread, + WaitableSet, +} from "../src/task/mod.ts"; +import type { SuspensionPoint } from "../src/jspi/mod.ts"; +import { + canonResourceDrop, + canonResourceNew, + ResourceHandle, + ResourceTypeInfo, +} from "../src/cabi/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, +}; + +function mkMemoryView() { + const memory = new WebAssembly.Memory({ initial: 1 }); + return { + 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, + }; +} + +/** One instance, one live thread, and the point-finder the #102 tests use. */ +function mkWorld() { + const store = new Store(); + const inst = new ComponentInstanceState(0, store); + const task = new Task(ASYNC_FT, CALLBACK_OPTS, inst, () => [], () => {}); + task.state = "started"; + const thread = new Thread(task, (function* () {})()); + return { + store, + inst, + task, + thread, + point(): SuspensionPoint | undefined { + return store.waiting.find( + (w) => typeof (w as { resume?: unknown }).resume === "function" && + typeof (w as { abandon?: unknown }).abandon === "function", + ) as SuspensionPoint | undefined; + }, + run(fn: () => T): T { + pushCurrentThread(thread); + try { + return fn(); + } finally { + popCurrentThread(thread); + } + }, + }; +} + +function opts( + inst: ComponentInstanceState, + over: Partial = {}, +): ResolvedOptions { + return { + stringEncoding: "utf8", + // deno-lint-ignore no-explicit-any + memory: mkMemoryView() as any, + realloc: null, + postReturn: null, + callback: null, + async: false, + cancellable: false, + coreType: { params: ["i32", "i32"], results: ["i32"] }, + instance: inst, + ...over, + }; +} + +const rejected = (p: Promise) => + p.then(() => "resolved", (e) => `rejected: ${(e as Error).message}`); + +// --------------------------------------------------------------------------- +// SITE 2: waitable-set.wait — `numWaiting` +// --------------------------------------------------------------------------- + +function parkOnWait(cancellable: boolean) { + const w = mkWorld(); + const wset = new WaitableSet(); + const seti = w.inst.handles.add(wset); + const o = opts(w.inst, { cancellable }); + const ctx = { componentInstance: () => w.inst, options: () => o, resultTypes: () => [] }; + const wait = createWaitableSetWait({ options: 0 }, ctx, w.inst, "jspi"); + const parked = w.run(() => wait(seti, 0)); + assert(parked instanceof Promise, "the wait parked"); + assertEq(wset.numWaiting, 1); + const point = w.point(); + assert(point !== undefined, "the suspension point is waiting"); + const drop = createWaitableSetDrop(w.inst); + return { ...w, wset, seti, parked, point, drop }; +} + +Deno.test("#106 SITE 2: abandon discharges numWaiting; waitable-set.drop stays legal", async () => { + const t = parkOnWait(false); + const settled = rejected(t.parked as Promise); + t.point.abandon(new Error("store teardown")); + assertEq(t.wset.numWaiting, 0); + // The observable the counter exists for: drop no longer traps + // "cannot drop waitable set with waiters" against a waiter that is gone. + t.drop(t.seti); + assertEq(await settled, "rejected: store teardown"); +}); + +Deno.test("#106 SITE 2: cancelled resume decrements exactly once (produce runs, hook is sole owner)", async () => { + const t = parkOnWait(true); + // Deliver cancellation exactly as Task.requestCancellation does — the + // produce(cancelled=true) leg (cancel_bracket_race_test.ts's machinery). + t.task.requestCancellation(null); + await (t.parked as Promise); + assertEq(t.wset.numWaiting, 0); // 0, not -1: the decrement is not doubled + t.drop(t.seti); +}); + +Deno.test("#106 SITE 2: produce-throw still discharges numWaiting", async () => { + const t = parkOnWait(false); + const settled = rejected(t.parked as Promise); + // resume with no pending event: `produce` calls `getPendingEvent()` on an + // empty set and throws — the produce-throw settle leg. + t.point.resume(false); + assertEq(t.wset.numWaiting, 0); + t.drop(t.seti); + assert((await settled).startsWith("rejected:"), "the park rejected"); +}); + +// --------------------------------------------------------------------------- +// SITE 4 (lit): stream sync copy — `end.hasSyncWaiter` +// --------------------------------------------------------------------------- + +Deno.test("#106 SITE 4: abandon clears hasSyncWaiter; cancel-copy and drop stay legal", async () => { + const w = mkWorld(); + const shared = new SharedStreamImpl(null); + const readEnd = new ReadableStreamEnd(shared); + const ri = w.inst.handles.add(readEnd); + const o = opts(w.inst); + const ctx = { + componentInstance: () => w.inst, + options: () => o, + streamElem: () => null, + futureElem: () => null, + resultTypes: () => [], + suspensionMode: "jspi" as const, + }; + const read = createStreamRead({ streamTable: 0, options: 0 }, ctx, w.inst); + const parked = w.run(() => read(ri, 0, 4)); + assert(parked instanceof Promise, "the sync read parked (no counterpart)"); + assertEq(readEnd.hasSyncWaiter, true); + const settled = rejected(parked as Promise); + const point = w.point(); + assert(point !== undefined, "the suspension point is waiting"); + + point.abandon(new Error("store teardown")); + + assertEq(readEnd.hasSyncWaiter, false); + // The observables the flag exists for: a concurrent cancel-copy trapped + // "sync waiter" while set (cancelCopy), and Waitable.drop asserts + // `!hasSyncWaiter`. Both must be legal again once no waiter exists. + const cancel = createStreamCancelRead({ streamTable: 0, async: false }, ctx, w.inst); + w.run(() => cancel(ri)); + assertEq(await settled, "rejected: store teardown"); +}); + +// --------------------------------------------------------------------------- +// SITE 5 (lit): subtask.cancel sync park — `st.hasSyncWaiter` +// --------------------------------------------------------------------------- + +Deno.test("#106 SITE 5: abandon clears the subtask's hasSyncWaiter", async () => { + const w = mkWorld(); + const st = new Subtask(); + st.onCancel = () => {}; // accepts and ignores, like a host import + const sti = w.inst.handles.add(st); + const cancel = createSubtaskCancel({ async: false }, w.inst, "jspi"); + const parked = w.run(() => cancel(sti)); + assert(parked instanceof Promise, "sync subtask.cancel parked (unresolved callee)"); + assertEq(st.hasSyncWaiter, true); + const settled = rejected(parked as Promise); + const point = w.point(); + assert(point !== undefined, "the suspension point is waiting"); + + point.abandon(new Error("store teardown")); + + assertEq(st.hasSyncWaiter, false); + assertEq(await settled, "rejected: store teardown"); +}); + +// --------------------------------------------------------------------------- +// Sync host-import park — subtask lenders (+ the needsJspi bail, site 5 of +// the #106 closure) +// --------------------------------------------------------------------------- + +function mkImportWorld(input: { + hostFn: (...a: unknown[]) => unknown; + mode: "plain" | "jspi"; + suspendable: boolean; +}) { + const w = mkWorld(); + const rt = new ResourceTypeInfo(w.inst, () => {}); + const handleIndex = canonResourceNew(w.inst, rt, 77); + const handle = w.inst.handles.get(handleIndex) as ResourceHandle; + const ft: FuncType = { + params: [{ kind: "borrow", rt }], + results: [], + async: false, + }; + const call = createLoweredImport({ + name: "lend-to-host", + ft, + opts: opts(w.inst, { + // Sync lower of (borrow) -> (): one flat i32 param, no results. + coreType: { params: ["i32"], results: [] }, + }), + hostFn: input.hostFn, + stats: newStats(), + mode: input.mode, + suspendable: input.suspendable, + }) as (...args: number[]) => unknown; + return { ...w, rt, handle, handleIndex, call }; +} + +Deno.test("#106 host-import park: abandon releases the subtask's lenders (caller not poisoned)", async () => { + const t = mkImportWorld({ + hostFn: () => new Promise(() => {}), + mode: "jspi", + suspendable: true, + }); + const parked = t.run(() => t.call(t.handleIndex)); + assert(parked instanceof Promise, "the sync suspending import parked"); + assertEq(t.handle.numLends, 1); // lent for the duration of the call + const settled = rejected(parked as Promise); + const point = t.point(); + assert(point !== undefined, "the suspension point is waiting"); + + point.abandon(new Error("store teardown")); + + assertEq(t.handle.numLends, 0); + // An abandoned park does NOT poison the caller (the + // resource_lender_park_settle_test.ts pin), so the handle must remain + // usable — this drop trapped "handle still lent out" before the hook. + canonResourceDrop(t.inst, t.rt, t.handleIndex); + assertEq(await settled, "rejected: store teardown"); +}); + +Deno.test("#106 host-import park: success path releases exactly once (hook observes resolveDelivered)", async () => { + let resolve!: (v: unknown) => void; + const t = mkImportWorld({ + hostFn: () => new Promise((r) => (resolve = r)), + mode: "jspi", + suspendable: true, + }); + const parked = t.run(() => t.call(t.handleIndex)); + assert(parked instanceof Promise, "the sync suspending import parked"); + assertEq(t.handle.numLends, 1); + const point = t.point(); + assert(point !== undefined, "the suspension point is waiting"); + + resolve(undefined); + await Promise.resolve(); // the recorded-outcome microtask + point.resume(false); // produce: onResolve + deliverResolve, then the hook + await parked; + + // deliverResolve ran inside produce; the onSettled backstop saw + // `resolveDelivered()` and stayed out of the way (a double release would + // throw AssertionError inside the hook and be reported, and the count + // would go negative — 0 pins both). + assertEq(t.handle.numLends, 0); + canonResourceDrop(t.inst, t.rt, t.handleIndex); +}); + +Deno.test("#106 host-import park: rejection (produce-throw) — poisoning trap, lenders unwound as belt-and-braces", async () => { + let reject!: (e: unknown) => void; + const t = mkImportWorld({ + hostFn: () => new Promise((_, r) => (reject = r)), + mode: "jspi", + suspendable: true, + }); + const parked = t.run(() => t.call(t.handleIndex)); + assert(parked instanceof Promise, "the sync suspending import parked"); + const settled = rejected(parked as Promise); + const point = t.point(); + assert(point !== undefined, "the suspension point is waiting"); + + reject(new Error("host bug")); + await Promise.resolve(); + point.resume(false); // produce throws `done.error` — the trap path + + // This settle poisons the caller (amendment 2 owes no release here — the + // analysis lives at the site); the hook unwinds anyway as harmless + // bookkeeping, which this pins so a future refactor keeps it deliberate. + assertEq(t.handle.numLends, 0); + assertEq(await settled, "rejected: host bug"); +}); + +Deno.test("#106 needsJspi bail: a capability signal after onStart must not strand lenders", () => { + // jspi mode but the import is NOT suspending()-marked: the lowering lifts + // the borrow (numLends -> 1), then bails NeedsJspi before any park exists. + const t = mkImportWorld({ + hostFn: () => new Promise(() => {}), + mode: "jspi", + suspendable: false, + }); + let threw: unknown = null; + try { + t.run(() => t.call(t.handleIndex)); + } catch (e) { + threw = e; + } + assert(threw instanceof NeedsJspi, "the sync lower bailed NeedsJspi"); + // NeedsJspi is expressly non-poisoning (amendment 2): the caller keeps + // running, so the lend must have been discharged... + assertEq(t.handle.numLends, 0); + // ...and the handle must remain fully usable. + canonResourceDrop(t.inst, t.rt, t.handleIndex); +});