diff --git a/runtime/src/intrinsics/fact_calls.ts b/runtime/src/intrinsics/fact_calls.ts index c0ddac2..bf71095 100644 --- a/runtime/src/intrinsics/fact_calls.ts +++ b/runtime/src/intrinsics/fact_calls.ts @@ -716,6 +716,42 @@ export function createSyncStartCall( // Not cancellable: a sync-lowered caller has no way to observe or // request cancellation mid-call -- the reference's wait here carries // no cancellation branch. + // LENDER RELEASE ON EVERY SETTLE PATH (#102). + // + // Enumeration of how this `SuspensionPoint` can reach a terminal + // state (jspi/bridge.ts `SuspensionPoint`), and whether `produce` + // runs on each: + // + // 1. `resume(false)` -> `produce` returns the packed result. + // RUNS. This is the success path; release stays INSIDE `produce`, + // before the results are shaped, so its ordering relative to the + // produced value is unchanged by this fix. + // 2. `resume(false)` -> `produce` throws (a trap computed at resume + // time). PARTIALLY RUNS. Release is `produce`'s first statement + // so it is already discharged here, but the `onSettled` backstop + // makes that independent of statement order. + // 3. `resume(true)` — a CANCELLED resume. Unreachable by + // construction: this park is `cancellable: false` and + // `SuspensionPoint.resume` asserts `cancellable || !cancelled` + // (#93). Note the assert fires BEFORE `#done` is set, so such a + // call leaves the point still parked and never settles it — a + // scheduler bug, not a guest-reachable exit; there is no + // non-poisoning continuation to release into. + // 4. `abandon(reason)` — store teardown / abandonment: fails the + // import's Promise WITHOUT calling `produce`. DOES NOT RUN. This + // is the #102 hole; `onSettled` covers it. + // 5. Never settled at all (the store is dropped while this point + // sits in `store.waiting`, e.g. the caller's whole host call was + // abandoned). No JS runs, so nothing can release; the lent + // handles die with the store, which is the reference's own + // outcome. Out of scope for amendment 2 (no non-poisoning exit). + // 6. Trap-poisoning of the parked instance: does not settle this + // point by itself — it reaches the guest either as (2) (a + // produce-time trap) or as (4) (teardown abandons the park), so + // it is covered by those two rows, not a third mechanism. + // + // `releaseLenders` is idempotent (#91), so the backstop is a no-op + // whenever `produce` already ran. return blockCurrentActivation({ store: prepared.callerInst.store, task: currentTask(), @@ -725,6 +761,7 @@ export function createSyncStartCall( lenderScope.releaseLenders(); return shapeResults(callerResults as CoreValue[] | null); }, + onSettled: () => lenderScope.releaseLenders(), }); } // A capability signal is expressly NON-poisoning (see above), so @@ -1011,12 +1048,34 @@ export function createAsyncStartCall( thread.done() || store.waiting.some((w) => w.task === task); if (!determinate()) { + // Same settle-path enumeration as the sync form above (#102). Here + // the lender scope is the `Subtask` itself, discharged by + // `deliverResolve`, and `report()` is what eventually delivers it — + // either eagerly (the resolved branch) or, for a live subtask, via + // the handle it hands the guest. So the backstop must fire ONLY when + // `report()` did not complete: on the success path the subtask is + // typically still live and in the caller's table, and unwinding it + // there would cancel a perfectly good call. + // + // `report()` not completing means the guest never received the + // subtask index (it either threw before `handles.add`, or after it + // with the index lost), so nothing will ever deliver this subtask's + // resolution — exactly the state `unwindSubtaskLenders` exists for + // (contracts/intrinsics.md v0.2 amendment 2). + let produced = false; return blockCurrentActivation({ store: prepared.callerInst.store, task: currentTask(), readyFunc: determinate, cancellable: false, - produce: () => report(), + produce: () => { + const r = report(); + produced = true; + return r; + }, + onSettled: () => { + if (!produced) unwindSubtaskLenders(subtask); + }, }); } } diff --git a/runtime/src/jspi/bridge.ts b/runtime/src/jspi/bridge.ts index 0fed31a..213ae62 100644 --- a/runtime/src/jspi/bridge.ts +++ b/runtime/src/jspi/bridge.ts @@ -408,6 +408,7 @@ export class SuspensionPoint implements SchedulableThread { #settle!: (v: T) => void; #fail!: (e: unknown) => void; #done = false; + #finished = false; #store: Store; /** @@ -436,6 +437,27 @@ export class SuspensionPoint implements SchedulableThread { private readonly produce: (cancelled: Cancelled) => T, // deno-lint-ignore no-explicit-any owner?: any, + /** + * `finally`-style hook: runs EXACTLY ONCE, on whichever terminal + * transition this point takes — produce-success, produce-throw, or + * `abandon` (issue #102). It is the seam a blocking built-in uses to + * discharge state it owns for the duration of the park (the FACT + * start-calls' borrow-lender scopes, contracts/intrinsics.md v0.2 + * amendment 2) without having to trust that `produce` runs. + * + * INVARIANTS this hook must respect, so bridge.ts's own contracts are + * not disturbed: + * * it must not throw (a throw here would escape `resume` *after* the + * import's Promise was settled, i.e. into whatever drained the + * scheduler); it is called inside a `try`/`catch` that reports such + * a throw rather than propagating it; + * * it must be idempotent-safe by construction anyway, because it runs + * AFTER `produce` on the success path — a built-in that already did + * its cleanup inside `produce` (to pin cleanup ordering relative to + * the produced value) sees this as a no-op backstop; + * * it must not resume/abandon this or any other suspension point. + */ + private readonly onSettled?: () => void, ) { this.#store = store; this.owner = owner ?? maybeCurrentThread() ?? task?.implicitThread ?? null; @@ -471,6 +493,18 @@ export class SuspensionPoint implements SchedulableThread { } this.#done = true; this.#store.stopWaiting(this); + try { + this.#resumeInner(cancelled); + } finally { + // Terminal state reached, by whichever of the two paths below. See + // `onSettled`: this is the backstop, not the primary cleanup site, so + // it runs after `produce` and after the settle — on the success path it + // observes cleanup `produce` already did, and changes nothing. + this.#finish(); + } + } + + #resumeInner(cancelled: Cancelled): void { let value: T; try { value = this.produce(cancelled); @@ -522,7 +556,29 @@ export class SuspensionPoint implements SchedulableThread { if (this.#done) return; this.#done = true; this.#store.stopWaiting(this); - this.#fail(reason); + try { + this.#fail(reason); + } finally { + // The settle path that never runs `produce` at all — the one issue #102 + // is about. + this.#finish(); + } + } + + /** Run `onSettled` at most once. Never throws (see the field's doc). */ + #finish(): void { + if (this.#finished) return; + this.#finished = true; + if (this.onSettled === undefined) return; + try { + this.onSettled(); + } catch (e) { + // Swallowing is the conservative reading: we are past the point where + // the guest's Promise was settled, so there is no frame left that could + // meaningfully receive this. Report loudly instead of corrupting an + // unrelated drain. + console.error(`[sp] onSettled threw for ${dbgId(this)}:`, e); + } } } @@ -542,6 +598,12 @@ export function blockCurrentActivation(input: { readyFunc: (() => boolean) | null; cancellable: boolean; produce: (cancelled: Cancelled) => T; + /** + * Optional `finally`-style hook — see `SuspensionPoint.onSettled`. Use it + * for state that must be discharged however the park ends, including the + * settle paths that never call `produce` (issue #102). + */ + onSettled?: () => void; }): Promise { // GATE LIFETIME: pristine reference semantics (definitions.py // `block_internal` line 378 does NOT touch `inst.exclusive_thread`). A @@ -576,6 +638,7 @@ export function blockCurrentActivation(input: { input.cancellable, input.produce, owner, + input.onSettled, ); return point.promise; } diff --git a/runtime/src/task/streams.ts b/runtime/src/task/streams.ts index c1c8c9f..1c19021 100644 --- a/runtime/src/task/streams.ts +++ b/runtime/src/task/streams.ts @@ -53,7 +53,7 @@ import { alignment, alignTo, elemSize } from "../cabi/layout.ts"; import { despecialize, valTypeEqual } from "../cabi/types.ts"; import type { ComponentValue, ValType } from "../cabi/types.ts"; import { Waitable } from "./waitable.ts"; -import { setOnInstancePoisoned } from "./scheduler.ts"; +import { isInstancePoisoned, setOnInstancePoisoned } from "./scheduler.ts"; /** Structural element-type equality (`null` = the zero-width payload). * Delegates to `valTypeEqual`: naive `JSON.stringify` comparison throws on @@ -625,40 +625,57 @@ interface PoisonedInstanceLike { * Drop a shared stream/future as *teardown*, without waking a doomed guest. * * Same outcome as `drop()` for host ends and healthy guest peers (a DROPPED - * notification), with one difference: a parked side belonging to an entered - * — and on every teardown path, about-to-be- or already-poisoned — guest - * instance (`mayEnter === false`) is retired silently via `resetPending`. + * notification), with one difference: a parked side belonging to a + * **poisoned** guest instance is retired silently via `resetPending`. * 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, so they are always notified. + * can no longer be entered (`tick` asserts enterability). Host sentinels are + * not instances at all, 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. + * #100: THE HEALTH TEST IS "POISONED", NOT "`mayEnter === false`". The + * original test used non-enterability as a proxy for deadness. The proxy is + * unsound in one direction, and the unsoundness stranded healthy tasks: * - * 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. + * * (sound half, #84 audit) a healthy guest peer always parks with + * `mayEnter === true`. 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). Blocking inside a + * wasm frame does NOT hold the enter bracket. + * * (unsound 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 an instance that traps + * (`ComponentInstanceState.enterFrom` clears `mayEnter` on the callee's + * entering set only, task/mod.ts). A *different*, healthy task of that + * caller, parked on an end of a stream/future the trapping callee also + * held, was classified dead here and retired silently — stranded, the + * exact outcome #66 exists to prevent. + * + * So the test consults the poison marker itself. It is per-instance and + * recorded at the single seam every bracket-break site routes through + * (`notifyInstancePoisoned`, task/scheduler.ts: exec/boundary.ts `poison`, + * `Store.tick`, `Thread.resumeWith`, the FACT cross-component catches in + * intrinsics/fact_calls.ts, and cabi/handles.ts's gated destructor call), + * and it is recorded *before* the retirement walk runs, so an instance's own + * parked ends still see it during its own walk. `retiredInstances` is + * consulted alongside it because the walk is also reachable directly (it is + * set at walk entry, so the two agree); neither ever contains the synthetic + * per-instantiation root, which every poison site skips or releases (plan v3 + * amendment 4, `releaseSyntheticRootOnPoison`). + * + * Why this does not re-open review B2 (phantom events into a corpse): the + * concern is that a DROPPED event queued onto a waitable of an instance that + * can never be entered again would be serviced by a later driving loop and + * resume machinery whose `tick` asserts enterability. "Can never be entered + * again" is precisely poisoning — a mid-call instance's `mayEnter` is + * restored by its own `leaveTo` when the call returns, and its parked task + * then resumes normally and consumes the event. The narrowed predicate + * therefore excludes exactly the population B2 is about, and admits only + * peers that will run again. * * Used by the poisoning walk below and by the trapping-import abandonment * path (embedder/instantiate.ts `releaseAsyncArgs`). Idempotent. @@ -669,9 +686,9 @@ export function dropSharedForTeardown( if (shared.dropped) return; shared.dropped = true; if (shared.pendingBuffer) { - const pi = shared.pendingInst as { mayEnter?: boolean } | null; - const parkedInDeadGuest = pi !== null && typeof pi === "object" && - typeof pi.mayEnter === "boolean" && !pi.mayEnter; + const pi = shared.pendingInst; + const parkedInDeadGuest = typeof pi === "object" && pi !== null && + (isInstancePoisoned(pi) || retiredInstances.has(pi)); if (parkedInDeadGuest) shared.resetPending(); else shared.resetAndNotifyPending(CopyResult.DROPPED); } diff --git a/runtime/tests/resource_lender_park_settle_test.ts b/runtime/tests/resource_lender_park_settle_test.ts new file mode 100644 index 0000000..9ce2f50 --- /dev/null +++ b/runtime/tests/resource_lender_park_settle_test.ts @@ -0,0 +1,306 @@ +// FACT start-call JSPI park: lender scopes must not leak on the settle paths +// that never run `produce` (issue #102). +// +// Authority: contracts/intrinsics.md v0.2 amendment 2 (scope-clarified +// 2026-08-10) — lender release on every non-poisoning exit. #91 covered the +// non-park exits of the start-call bodies (trap rethrow, capability bail, +// async-start resume-trap), see `resource_lender_unwind_test.ts`. One layer +// down, `createSyncStartCall`'s `blockCurrentActivation` park released its +// lenders only inside `produce()`, so a `SuspensionPoint` that settles +// WITHOUT producing — `abandon`, i.e. store teardown — left `numLends` +// elevated forever, and every later `lift_own` / `resource.drop` of those +// handles would trap "handle still lent out" (definitions.py 1508 / 2325). +// +// Settle-path enumeration for these parks (mirrored in the comment at the fix +// site, fact_calls.ts): +// +// 1. resume(false), produce returns -> produce RUNS (success) +// 2. resume(false), produce throws -> produce PARTIALLY runs +// 3. resume(true) -> unreachable: these parks are +// `cancellable: false` and `SuspensionPoint.resume` asserts +// `cancellable || !cancelled` (#93) BEFORE marking the point done +// 4. abandon(reason) -> produce NEVER runs <-- #102 +// 5. never settled (store dropped) -> nothing runs at all; the +// handles die with the store, as in the reference +// 6. trap-poisoning of the parked instance -> surfaces as (2) or (4) +// +// The fix ties release to the point's terminal state via `onSettled` +// (jspi/bridge.ts), keeping the release inside `produce` as well so the +// SUCCESS-path ordering — release before the packed result is shaped — is +// unchanged. + +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, withActivation } 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 { 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; +const START_FLAG_ASYNC_CALLEE = 1; + +interface Harness { + store: Store; + caller: ComponentInstanceState; + callee: ComponentInstanceState; + handle: ResourceHandle; + rt: ResourceTypeInfo; + handleIndex: number; + /** Run one prepare + start-call in jspi mode; returns what it returned. */ + run(kind: "sync" | "async", calleeBody: () => CoreValue): unknown; + /** The single parked suspension point, or `undefined`. */ + point(): SuspensionPoint | undefined; +} + +function mkHarness(): Harness { + const store = new Store(); + const caller = new ComponentInstanceState(0, store); + const callee = new ComponentInstanceState(1, store); + 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[], + resultTypesForTuple: () => null, + callback: (_i: number) => null, + memoryToken: () => null, + stats: newStats(), + prepared, + factStartScopes, + // The park under test only exists in jspi mode. Nothing here needs the + // engine's JSPI: `calleeCanBlock` is absent, so no callee is + // `promising`-wrapped, and `blockCurrentActivation` mints an ordinary + // `SuspensionPoint` whose Promise we never hand to wasm. + suspensionMode: "jspi" as const, + }; + + // `blockCurrentActivation` reads `currentTask()`, so the start-call must run + // under an ambient activation, as it always does in a real guest frame (the + // caller's). A minimal stand-in is enough: the point only reads + // `task.implicitThread` and `task.inst`. + const callerAmbient = { task: { inst: caller, implicitThread: null } }; + + return { + store, + caller, + callee, + handle, + rt, + handleIndex, + point() { + return store.waiting.find( + (w) => typeof (w as { resume?: unknown }).resume === "function" && + typeof (w as { abandon?: unknown }).abandon === "function", + ) as SuspensionPoint | undefined; + }, + run(kind, calleeBody) { + // `[async-start]` is where `transfer-borrow` lends one of the caller's + // handles to the call (intrinsics/mod.ts `FactStartScope`). + 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) + // deno-lint-ignore no-explicit-any + : createAsyncStartCall({ callback: null, postReturn: null }, ctx as any); + + prep( + start, + return_, + 0, // caller_instance + 1, // callee_instance + 0, + 0, + 0, + PREPARE_ASYNC_NO_RESULT, + ); + return withActivation(callerAmbient, () => + kind === "sync" + ? startCall(calleeBody, 0) + : startCall(calleeBody, 0, 0, START_FLAG_ASYNC_CALLEE)); + }, + }; +} + +/** + * A callee whose core call never returns: it parks on a host promise, so the + * callee's thread stays alive and unresolved and the CALLER reaches its park + * (a callee that merely returned without resolving would trap "task finished + * all threads without resolving" instead). + */ +const neverResolves = (() => new Promise(() => {})) as unknown as () => CoreValue; + +Deno.test("#102: sync-start-call park releases lenders when abandoned (no produce)", async () => { + const h = mkHarness(); + const parked = h.run("sync", neverResolves); + assert(parked instanceof Promise, "the caller's activation parked"); + // Rejection is the point of `abandon`; consume it so the test does not fail + // on an unhandled rejection. + const settled = parked.then( + () => "resolved", + (e) => `rejected: ${(e as Error).message}`, + ); + const point = h.point(); + assert(point !== undefined, "the suspension point is registered as waiting"); + assertEq(h.handle.numLends, 1); // lent for the duration of the park + + // Settle path 4: teardown abandons the park. `produce` never runs. + point.abandon(new Error("store teardown")); + + assertEq(h.handle.numLends, 0); + // The caller is NOT poisoned by an abandoned park, so the handle must stay + // usable — this is the trap amendment 2 exists to prevent. + canonResourceDrop(h.caller, h.rt, h.handleIndex); + assertEq(await settled, "rejected: store teardown"); + assertEq(h.store.waiting.includes(point), false); +}); + +Deno.test("#102: sync-start-call park success path is unchanged (release before the result)", async () => { + const h = mkHarness(); + let resolved = false; + const parked = h.run("sync", neverResolves); + assert(parked instanceof Promise, "the caller's activation parked"); + const point = h.point(); + assert(point !== undefined, "the suspension point is registered as waiting"); + assertEq(h.handle.numLends, 1); + + // Settle path 1. `produce` releases the lenders BEFORE shaping the results, + // and this pins that ordering: at the instant the value exists, the release + // has already happened. (`resume` settles synchronously; the value is + // observed on the microtask turn after.) + parked.then(() => { + resolved = true; + // Ordering pin: the released state is visible to whatever observes the + // produced value. + assertEq(h.handle.numLends, 0); + }); + point.resume(false); + assertEq(h.handle.numLends, 0); // released inside `produce`, not by a hook + await parked; + assertEq(resolved, true); + canonResourceDrop(h.caller, h.rt, h.handleIndex); +}); + +Deno.test("#102: sync-start-call park releases lenders when produce throws", async () => { + const h = mkHarness(); + const parked = h.run("sync", neverResolves); + assert(parked instanceof Promise, "the caller's activation parked"); + const settled = parked.then(() => "resolved", (e) => `rejected: ${(e as Error).message}`); + const point = h.point(); + assert(point !== undefined, "the suspension point is registered as waiting"); + + // Settle path 2: a trap computed at resume time. Simulated by making the + // produced value itself unobtainable — the shape a real produce-throw has + // (jspi/bridge.ts `resume`'s catch: it reaches the guest as a rejection). + // Deliberately throwing BEFORE the release the real `produce` performs: the + // point of the fix is that release no longer depends on `produce` getting + // that far. + (point as unknown as { produce: () => unknown }).produce = () => { + throw new Error("resume-time trap"); + }; + + point.resume(false); + assertEq(h.handle.numLends, 0); + canonResourceDrop(h.caller, h.rt, h.handleIndex); + assertEq(await settled, "rejected: resume-time trap"); +}); + +Deno.test("#102: a cancelled resume cannot reach these non-cancellable parks", async () => { + const h = mkHarness(); + const parked = h.run("sync", neverResolves); + assert(parked instanceof Promise, "the caller's activation parked"); + const settled = parked.then(() => "resolved", (e) => `rejected: ${(e as Error).message}`); + const point = h.point(); + assert(point !== undefined, "the suspension point is registered as waiting"); + + // Settle path 3: rejected by `SuspensionPoint.resume`'s assert (#93), which + // fires before the point is marked done — so this is not a settle path at + // all, and there is no non-poisoning continuation to release into. Pinned + // here so a future `cancellable: true` at this site has to revisit the + // enumeration. + let threw: unknown = null; + try { + point.resume(true); + } catch (e) { + threw = e; + } + assert(threw !== null, "a cancelled resume of a non-cancellable point traps"); + assertEq(point.waiting(), true); // still parked: NOT a terminal transition + + // Teardown then still discharges the lenders. + point.abandon(new Error("teardown after the illegal resume")); + assertEq(h.handle.numLends, 0); + assertEq(await settled, "rejected: teardown after the illegal resume"); +}); + +Deno.test("#102: async-start-call determinacy park releases subtask lenders when abandoned", async () => { + const h = mkHarness(); + // A callee whose core call parks on a host promise: the callee thread is + // neither done nor scheduler-parked, so `determinate()` is false and the + // CALLER parks on the determinacy wait (fact_calls.ts, issue #43). + const parked = h.run("async", neverResolves); + if (!(parked instanceof Promise)) { + // The determinacy wait was satisfied eagerly; nothing to test here (the + // eager path is `resource_lender_unwind_test.ts`'s territory). + return; + } + const settled = parked.then(() => "resolved", (e) => `rejected: ${(e as Error).message}`); + const point = h.point(); + assert(point !== undefined, "the caller parked on the determinacy wait"); + assertEq(h.handle.numLends, 1); + + point.abandon(new Error("store teardown")); + + // `report()` never ran, so the guest never got a subtask index and nothing + // would ever deliver this subtask's resolution: the park's backstop unwinds + // it, exactly as the trap path does (#91's `unwindSubtaskLenders`). + assertEq(h.handle.numLends, 0); + canonResourceDrop(h.caller, h.rt, h.handleIndex); + assertEq(await settled, "rejected: store teardown"); +}); + +Deno.test("#102: async-start-call determinacy park does NOT unwind a live subtask on success", async () => { + const h = mkHarness(); + const parked = h.run("async", neverResolves); + if (!(parked instanceof Promise)) return; + const point = h.point(); + assert(point !== undefined, "the caller parked on the determinacy wait"); + + // Settle path 1: `report()` completes and hands the guest a subtask index. + // The subtask is LIVE — its lenders are released by its own + // `deliverResolve` later (definitions.py `Subtask.deliver_resolve`, 904) — + // so the backstop must stay out of the way. A blanket unwind here would + // cancel a perfectly good call. + point.resume(false); + const packed = await parked; + assertEq(typeof packed, "number"); + assertEq(h.handle.numLends, 1); +}); diff --git a/runtime/tests/streams_teardown_test.ts b/runtime/tests/streams_teardown_test.ts index 4f8f635..048e058 100644 --- a/runtime/tests/streams_teardown_test.ts +++ b/runtime/tests/streams_teardown_test.ts @@ -44,6 +44,7 @@ import { ComponentInstanceState, CopyResult, CopyState, + notifyInstancePoisoned, popCurrentThread, pushCurrentThread, ReadableFutureEnd, @@ -526,3 +527,189 @@ Deno.test("#97: cancelRead resolves the read exactly like end-of-stream does", a ended.writable.drop(); assertEq(await endedRead, []); }); + +// --------------------------------------------------------------------------- +// #100: `mayEnter === false` is not "poisoned" — a mid-FACT-call CALLER's +// healthy parked task must not be silently retired +// --------------------------------------------------------------------------- +// +// The stranding shape from the issue. Instance A (caller) is mid +// cross-component (FACT) call into instance B (callee), so A is non-enterable +// for the whole duration of that call (`enterFrom` clears `mayEnter` on the +// callee's entering set; the caller's own bracket is still open). A DIFFERENT, +// perfectly healthy task of A is parked on an end of a stream/future whose +// peer end B holds. B traps; the poisoning walk runs over B's table and +// reaches A's parked side. The old health test (`mayEnter === false`) read A +// as a corpse and retired it in silence — stranded, the outcome #66 exists to +// prevent. The narrowed test (task/scheduler.ts's per-instance poison marker, +// recorded at the `notifyInstancePoisoned` seam) gives A the spec-shaped +// outcome instead: DROPPED for a stream, the #84 abandonment trap for an +// unwritten future. +// +// Both directions are pinned here: the dead-guest discipline (a parked task of +// the POISONED instance itself is still silently retired) has its own leg +// below, so narrowing the predicate cannot quietly become "always notify". + +/** Options/ctx for a guest built-in call in `inst`. */ +function mkCtx( + inst: ComponentInstanceState, + // deno-lint-ignore no-explicit-any + view: any, +) { + const opts: ResolvedOptions = { + stringEncoding: "utf8", + memory: view, + realloc: null, + postReturn: null, + callback: null, + async: true, + cancellable: false, + coreType: { params: ["i32", "i32"], results: ["i32"] }, + instance: inst, + }; + return { + componentInstance: () => inst, + options: () => opts, + streamElem: () => null, + futureElem: () => null, + resultTypes: () => [], + suspensionMode: "plain" as const, + }; +} + +/** Run `fn` as a task of `inst` (the built-ins read the current thread). */ +function inTask(inst: ComponentInstanceState, fn: () => T): T { + const task = new Task(ASYNC_FT, CALLBACK_OPTS, inst, () => [], () => {}); + const thread = new Thread(task, (function* () {})()); + pushCurrentThread(thread); + try { + return fn(); + } finally { + popCurrentThread(thread); + } +} + +/** + * Put `caller` mid-cross-component-call into `callee`: the host entered the + * caller, and the caller entered the callee (task/mod.ts `enterFrom`). + */ +function enterMidFactCall( + caller: ComponentInstanceState, + callee: ComponentInstanceState, +): void { + caller.enterFrom(null); + callee.enterFrom(caller); + assertEq(caller.mayEnter, false); // the trap the old health test fell into + assertEq(callee.mayEnter, false); +} + +Deno.test("#100: a mid-FACT-call caller's parked stream reader gets DROPPED, not silence", () => { + const store = new Store(); + const caller = new ComponentInstanceState(0, store); // A: healthy + const callee = new ComponentInstanceState(1, store); // B: traps + const { view } = mkMemory(); + const shared = new SharedStreamImpl(null); + callee.handles.add(new WritableStreamEnd(shared)); + const readEnd = new ReadableStreamEnd(shared); + const ri = caller.handles.add(readEnd); + + // A's other task parks on the read (a healthy park: `mayEnter === true`). + const read = createStreamRead( + { streamTable: 0, options: 0 }, + mkCtx(caller, view), + caller, + ); + assertEq(inTask(caller, () => read(ri, 0, 4)), BLOCKED); + assertEq(readEnd.state, CopyState.COPYING); + assertEq(caller.mayEnter, true); + + // A calls into B; B traps. The poison goes through the one seam every + // bracket-break site uses, which is what records the marker. + enterMidFactCall(caller, callee); + notifyInstancePoisoned(callee, new Trap("unreachable")); + + // A is alive: it gets end-of-stream, not silence. + assertEq(shared.dropped, true); + assertEq(readEnd.hasPendingEvent(), true); + const [, , payload] = readEnd.getPendingEvent(); + assertEq(payload & 0xf, CopyResult.DROPPED); +}); + +Deno.test("#100: a mid-FACT-call caller's parked future reader gets the abandonment trap", () => { + const store = new Store(); + const caller = new ComponentInstanceState(0, store); + const callee = new ComponentInstanceState(1, store); + const { view } = mkMemory(); + const shared = new SharedFutureImpl(null); + // B owes a value it can never deliver once it traps (#84). + callee.handles.add(new WritableFutureEnd(shared)); + const readEnd = new ReadableFutureEnd(shared); + const ri = caller.handles.add(readEnd); + const ctx = mkCtx(caller, view); + const read = createFutureRead({ futureTable: 0, options: 0 }, ctx, caller); + const wait = createWaitableSetWait({ options: 0 }, ctx, caller); + const wset = new WaitableSet(); + const seti = caller.handles.add(wset); + readEnd.join(wset); + + assertEq(inTask(caller, () => read(ri, 0)), BLOCKED); + assertEq(caller.mayEnter, true); + + enterMidFactCall(caller, callee); + const boom = new Trap("unreachable"); + notifyInstancePoisoned(callee, boom); + + assertEq(readEnd.hasPendingEvent(), true); + const e = caughtSync(() => inTask(caller, () => wait(seti, 64))); + assertAbandonTrap(e, "trapped while it held an end"); + assertEq((e as { cause?: { cause?: unknown } }).cause?.cause, boom); +}); + +Deno.test("#100: the poisoned instance's OWN parked task is still retired silently", () => { + // The other direction — the dead-guest discipline the narrowed predicate + // must preserve. A parked side of the instance being poisoned would, if + // notified, leave a phantom event in a waitable of an instance that can + // never be entered again (review B2). + const store = new Store(); + const doomed = new ComponentInstanceState(0, store); + const peerInst = new ComponentInstanceState(1, store); + const { view } = mkMemory(); + const shared = new SharedStreamImpl(null); + peerInst.handles.add(new WritableStreamEnd(shared)); + const readEnd = new ReadableStreamEnd(shared); + const ri = doomed.handles.add(readEnd); + const read = createStreamRead( + { streamTable: 0, options: 0 }, + mkCtx(doomed, view), + doomed, + ); + assertEq(inTask(doomed, () => read(ri, 0, 4)), BLOCKED); + + notifyInstancePoisoned(doomed, new Trap("unreachable")); + + assertEq(shared.dropped, true); + assertEq(readEnd.hasPendingEvent(), false); + // Silent retirement leaves the end where the trap left it (`resetPending` + // clears the rendezvous, not the end's state): the corpse's task never runs + // again, so nothing observes it. + assertEq(readEnd.state, CopyState.COPYING); +}); + +Deno.test("#100: a host peer parked on a poisoned guest's end is still notified", () => { + // Host sentinels are not component instances, so the predicate never reads + // them as poisoned (the pre-#100 test relied on the absence of a `mayEnter` + // key for the same conclusion). + const store = new Store(); + const guest = new ComponentInstanceState(0, store); + const shared = new SharedStreamImpl(null); + let result: CopyResult | null = null; + shared.setPending( + null, + new HostBuffer(null, null, 4) as never, + () => {}, + (r) => result = r, + ); + guest.handles.add(new WritableStreamEnd(shared)); + notifyInstancePoisoned(guest, new Trap("unreachable")); + assertEq(result, CopyResult.DROPPED); +});