teardown/park residuals: poison-marker health test, settle-proof lender release

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);
+          },
         });
       }
     }

runtime/src/jspi/bridge.ts

@@ -408,6 +408,7 @@ export class SuspensionPoint<T = unknown> implements SchedulableThread {
   #settle!: (v: T) => void;
   #fail!: (e: unknown) => void;
   #done = false;
+  #finished = false;
   #store: Store;
 
   /**
@@ -436,6 +437,27 @@ export class SuspensionPoint<T = unknown> 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<T = unknown> 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<T = unknown> 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<T>(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<T> {
   // 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<T>(input: {
     input.cancellable,
     input.produce,
     owner,
+    input.onSettled,
   );
   return point.promise;
 }

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);
   }