From f529d3b580faeed530d7ea7aa7558ab32bbb610b Mon Sep 17 00:00:00 2001 From: Lann Martin Date: Mon, 7 Sep 2026 11:52:13 -0400 Subject: [PATCH] lift: branch on the driver's exit verdict, not a re-derived done() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An async-typed lifted export's driver exits on one of two verdicts: `done()` held (task.return seen, no wasm call or hop in flight) or idle (#292 / #310). Since #310 the lift RE-EVALUATED `driveDone()` in its `.then` continuation to decide between `finishHostEntry` and `backgroundCompletion`. That predicate is time-varying: one microtask after an EXIT-done, another driver of the same store (the settlement pump servicing a settled host call) can resume a background activation that transiently hop-parks, so `midWasmCall()`/`hopParked()` read true and the lift took `backgroundCompletion()` — which resolves on `Task.onFinished`, i.e. never for a callback-ABI task whose spawned futures are immortal. Regression from 0.6.5, whose continuation was `pending.then(finishHostEntry)` unconditionally. Observed in polyvisor: `lifecycle.boot` (spawns the engine driver, event pump and accept loop, then task.returns) never resolved on a reload once a store sync task added enough host-call traffic for the pump to win the race. Trace: `EXIT-done ... done=true` followed by `-> backgroundCompletion threads=1 resolvedSeen=true`. Fix: `drive`/`driveLoop`/`driveAsync` return their exit verdict (`DriveExit = "done" | "idle"`) and the lift branches on that. The verdict is a fact about the loop that ran; re-deriving it later asks a different question. `driveStoreAsync` discards the verdict (the pumps have no completion to route). Test: `runtime/tests/lift_done_verdict_test.ts` — a callback-ABI lift with an immortal second thread task.returns and exits done; a foreign thread hop-parks in the microtask window before the continuation. Red on main ("expected resolved, got pending"), green with the fix. --- runtime/src/exec/boundary.ts | 101 +++++++++---- runtime/tests/lift_done_verdict_test.ts | 191 ++++++++++++++++++++++++ 2 files changed, 260 insertions(+), 32 deletions(-) create mode 100644 runtime/tests/lift_done_verdict_test.ts diff --git a/runtime/src/exec/boundary.ts b/runtime/src/exec/boundary.ts index de101c0..e450fcf 100644 --- a/runtime/src/exec/boundary.ts +++ b/runtime/src/exec/boundary.ts @@ -445,7 +445,8 @@ function traceDrive(loop: string, store: Store, done: () => boolean, branch: str * paths, the destructor entry, and the pumps (which cannot reach the verdict * anyway — see `driveStoreAsync`). * - * `"exit"` returns instead, leaving `done()` false for the caller to notice. + * `"exit"` returns instead, with the verdict `"idle"` (`DriveExit`) for the + * caller to act on. * It exists for ONE caller: an **async-typed** lifted export (#292). The * reference's driving loop is guarded by `if not ft.async_`, so for such an * export `canon_lift` returns right after the first `thread.resume()` and the @@ -461,18 +462,35 @@ function traceDrive(loop: string, store: Store, done: () => boolean, branch: str type IdlePolicy = "trap" | "exit"; /** - * Pump `store` until `done()` holds. Returns `undefined` if that was achieved - * synchronously, or a Promise that settles when it has been. + * WHY a driving loop returned: `"done"` means `done()` held at the exit test, + * `"idle"` means the loop ran out of moves with `done()` still false and + * `idle: "exit"` let it return instead of trapping (only an async-typed lift + * asks for that; see `IdlePolicy`). * - * Under `idle: "exit"` it may also return with `done()` still false; see - * `IdlePolicy`. + * The verdict is a RETURN VALUE, not something the caller re-derives, because + * `done` is a predicate over shared, time-varying store state: between a + * driver's exit test and its caller's continuation (one microtask later) + * another driver of the same store can flip it. That is polyengine#310 — a + * lift that exited `done` re-tested `driveDone()` in its `.then`, the + * settlement pump had meanwhile hop-parked a background activation, and the + * lift took the never-completing background path. The exit verdict is a fact + * about the past and cannot rot; the predicate can. + */ +type DriveExit = "done" | "idle"; + +/** + * Pump `store` until `done()` holds. Returns the exit verdict directly if + * that was settled synchronously, or a Promise of it otherwise. + * + * Under `idle: "exit"` the verdict may be `"idle"`, i.e. it returned with + * `done()` still false; see `IdlePolicy` and `DriveExit`. */ function drive( store: Store, done: () => boolean, what: string, idle: IdlePolicy = "trap", -): void | Promise { +): DriveExit | Promise { try { return driveLoop(store, done, what, idle); } catch (e) { @@ -491,7 +509,7 @@ function driveLoop( done: () => boolean, what: string, idle: IdlePolicy, -): void | Promise { +): DriveExit | Promise { for (;;) { traceDrive("drive", store, done, "top"); // The synchronous drain must not run while a thread is parked on a @@ -515,7 +533,7 @@ function driveLoop( // will not fire — arm the settlement pump here for any host calls the // guest registered fire-and-forget during this drive. ensureSettlementPump(store); - return; + return "done"; } // A thread parked on a Promise (jspi) can only progress after a microtask // turn, exactly like an outstanding host call. So can an outstanding @@ -534,7 +552,7 @@ function driveLoop( // Same hand-off as the `done()` exit above: work this loop started // outlives it. ensureSettlementPump(store); - return; + return "idle"; } traceDrive("drive", store, done, "DEADLOCK-TRAP"); trapIf( @@ -606,7 +624,9 @@ export async function driveStoreAsync( done: () => boolean, what: string, ): Promise { - return await driveAsync(store, done, what); + // The exit verdict is for `drive`'s lift caller (see `DriveExit`); the + // pumps drive to quiescence and have nothing to decide on it. + await driveAsync(store, done, what); } /** @@ -1001,7 +1021,7 @@ async function driveAsync( done: () => boolean, what: string, idle: IdlePolicy = "trap", -): Promise { +): Promise { const depth = storeDriverDepth(store) + 1; driverDepth.set(store, depth); // An incumbent driver may be parked in the awaiting-race holding the @@ -1079,7 +1099,7 @@ async function driveAsync( if (store.hostFailure !== undefined) throw takeHostFailure(store); if (done()) { traceDrive("driveAsync", store, done, "EXIT-done"); - return; + return "done"; } // Only a SERVICEABLE tail is a reason to loop again: a queue holding // only tails DEFERRED on a non-enterable instance (issue #156) would @@ -1195,7 +1215,7 @@ async function driveAsync( if (store.readyCandidates().length === 0) { if (idle === "exit") { traceDrive("driveAsync", store, done, "EXIT-idle"); - return; + return "idle"; } trapIf( true, @@ -1383,7 +1403,7 @@ async function driveAsync( if (store.pendingHostCalls.size === 0) { if (idle === "exit") { traceDrive("driveAsync", store, done, "EXIT-idle"); - return; + return "idle"; } traceDrive("driveAsync", store, done, "DEADLOCK-TRAP"); trapIf( @@ -1858,16 +1878,20 @@ export function createLiftedFunction(input: { * The task outlived its driver (#292): hand the host a Promise settled by * the task itself. * + * Reached ONLY when this lift's own driver exited with the verdict + * `"idle"` (`DriveExit`) — it ran out of moves with the task unfinished. + * It is never reached after a `"done"` exit, however the store's state + * may have moved on since (polyengine#310). + * * Resolution rides `finishHostEntry` unchanged — it already holds * `completed`/`resultsToHost` — fired from `Task.onFinished`, i.e. the * moment this task's last thread unregisters. That point is safe for the - * old `done` predicate's task-scoped clauses: `resolvedSeen` is - * guaranteed (`unregisterThread`'s own `trapIf(state !== "resolved")` - * fires first otherwise), and this task's threads are gone from - * `store.awaiting`, so `midWasmCall()` is false by construction. - * `hopParked()` is deliberately NOT re-tested: a hop is the obligation of - * the driver that put it in flight (#280), never of a Promise waiting on - * another task. + * `done` predicate's task-scoped clauses: `resolvedSeen` is guaranteed + * (`unregisterThread`'s own `trapIf(state !== "resolved")` fires first + * otherwise), and this task's threads are gone from `store.awaiting`, so + * `midWasmCall()` is false by construction. `hopParked()` plays no part: + * a hop is the obligation of the driver that put it in flight (#280), + * never of a Promise waiting on another task. * * Rejection has two sources: `finishHostEntry` itself throwing, and the * instance being poisoned by a later driver that ran this task into a @@ -1906,8 +1930,7 @@ export function createLiftedFunction(input: { }; }); - let pending: void | Promise; - let driveDone: () => boolean = () => true; + let outcome: DriveExit | Promise; try { // Completion is "the task resolved AND its threads have drained", not // merely "resolved". `task.return` resolves the task, but the activation @@ -1966,7 +1989,7 @@ export function createLiftedFunction(input: { // covers this task's own suspended thread. const midWasmCall = () => task.threads.some((t) => store.awaiting.has(t)); const hopParked = () => entryHopThreads(store).length > 0; - driveDone = () => resolvedSeen && !midWasmCall() && !hopParked(); + const driveDone = () => resolvedSeen && !midWasmCall() && !hopParked(); // ASYNC-TYPED EXPORTS DO NOT TRAP ON IDLE (#292). definitions.py // `canon_lift` runs the driving loop — and with it the // empty-candidate-set `trap_if` — only `if not ft.async_` (line 2189); @@ -1984,7 +2007,7 @@ export function createLiftedFunction(input: { // intra-component future a later export call writes. // Sync-typed exports are unchanged: their loop traps on idle in every // mode, which is what the paragraphs above describe. - pending = drive( + outcome = drive( store, driveDone, `export '${name}'`, @@ -1994,19 +2017,33 @@ export function createLiftedFunction(input: { unwind(); throw e; } - if (pending === undefined) { + /** + * Branch on the driver's EXIT VERDICT, never on a fresh `driveDone()` + * (polyengine#310). + * + * `driveDone` is a predicate over store-wide state that other drivers + * mutate. Re-evaluating it here — a microtask after the driver returned, + * on the asynchronous path — reads a different instant than the one the + * driver decided on. Observed: this lift's driver exited `EXIT-done`, + * then the settlement pump (servicing a settled host call belonging to + * another task) resumed a background activation that transiently + * hop-parked, so `hopParked()` read true in the continuation and the lift + * took `backgroundCompletion()` — which waits for the task's LAST thread + * to unregister, i.e. never, for a task holding long-lived spawned + * futures. The verdict cannot rot that way: `"done"` means the driver + * saw the task finished, which stays true. + */ + const finish = (verdict: DriveExit): unknown => + verdict === "idle" ? backgroundCompletion() : finishHostEntry(); + if (!isPromiseLike(outcome)) { try { - if (idlePolicy === "exit" && !driveDone()) return backgroundCompletion(); - return finishHostEntry(); + return finish(outcome); } catch (e) { unwind(); throw e; } } - return pending.then(() => { - if (idlePolicy === "exit" && !driveDone()) return backgroundCompletion(); - return finishHostEntry(); - }, (e) => { + return outcome.then(finish, (e) => { unwind(); throw e; }); diff --git a/runtime/tests/lift_done_verdict_test.ts b/runtime/tests/lift_done_verdict_test.ts new file mode 100644 index 0000000..cab8b7d --- /dev/null +++ b/runtime/tests/lift_done_verdict_test.ts @@ -0,0 +1,191 @@ +// A lift that exited `done` resolves even if another driver hop-parks before +// its continuation runs — polyengine#310. +// +// THE REGRESSION (0.6.5 -> 0.6.6). `createLiftedFunction`'s asynchronous +// completion path used to be +// +// pending.then(() => { +// if (idlePolicy === "exit" && !driveDone()) return backgroundCompletion(); +// return finishHostEntry(); +// }) +// +// which RE-DERIVES `driveDone()` a microtask after the driver already decided. +// `driveDone` is a predicate over store-wide state (`hopParked()` looks at +// every task's threads, #280), and other drivers of the same store mutate it: +// in the traced consumer the settlement pump serviced a settled host call +// belonging to another task, the activation it resumed transiently hop-parked, +// and the lift — whose own driver had exited `EXIT-done` one microtask +// earlier — took `backgroundCompletion()`. That path waits for the task's LAST +// thread to unregister, so for an export that spawns long-lived background +// futures (the consumer's `boot`: engine driver, event pump, accept loop) the +// host's Promise never settles. +// +// THE SHAPE here, store-level in the style of `parked_driver_host_call_test.ts` +// (no checked-in example guest has the participants): +// +// * a real async-typed, callback-ABI lifted export whose core returns a +// Promise, so its thread parks on it and the drive necessarily goes +// through `driveAsync` — i.e. the export completes on the `.then` path +// where the re-derivation lived; +// * the core resolves the task (`task.return`) and spawns an IMMORTAL +// second thread into the same task (`readyFunc: () => false`), the model +// of the consumer's background futures: it keeps `task.threads` non-empty +// forever, which is what makes `backgroundCompletion()` a black hole +// rather than a detour; +// * a foreign ready thread, resumed by the very tick that follows the +// export thread's resumption, queues a microtask that puts a foreign +// HOP-park into `store.awaiting` (`entryHopThreads` = awaiting with no +// `SuspensionPoint` owner). It lands after the driver's `done()` test and +// before the lift's continuation — exactly the window the trace shows. +// +// Pre-fix the export's Promise never settles; with the verdict plumbed out of +// the driver (`DriveExit`) it resolves. + +import { assertEq } from "./support/asserts.ts"; +import { createLiftedFunction, newStats, type ResolvedOptions } from "../src/exec/mod.ts"; +import { + ComponentInstanceState, + currentThread, + Store, + Thread, +} 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}`); +} + +/** `async func()` — no params, no results, so no memory is needed. */ +const FT: FuncType = { params: [], results: [], async: true }; + +/** + * A foreign hop-parked activation: in `store.awaiting`, with nothing owning + * it in `store.waiting`. `entryHopThreads` reads exactly these two fields. + */ +function foreignHop() { + return { task: { inst: {} }, awaiting: new Promise(() => {}) }; +} + +/** + * The other driver's guest thread: ready only once the export's own thread has + * left the task, so it is ticked in the same synchronous stretch as the + * driver's `done()` test — and not before it. + */ +class ForeignThread { + resumed = 0; + readonly task = { inst: {} }; + constructor( + private readonly armed: () => boolean, + private readonly onResume: () => void, + ) {} + ready(): boolean { + return this.resumed === 0 && this.armed(); + } + waiting(): boolean { + return this.resumed === 0; + } + resume(): void { + this.resumed++; + this.onResume(); + } +} + +Deno.test({ + name: + "a lift that exited done resolves even if another driver hop-parks before its continuation runs (#310)", + fn: async () => { + const store = new Store(); + const inst = new ComponentInstanceState(0, store); + const opts: ResolvedOptions = { + stringEncoding: "utf8", + memory: null, + realloc: null, + postReturn: null, + // Callback ABI: the packed code the core returns is CallbackCode.EXIT + // (0), so the loop exits at once and the implicit thread unregisters. + callback: () => (() => 0) as never, + async: true, + cancellable: false, + coreType: { params: [], results: ["i32"] }, + instance: inst, + }; + + const hop = foreignHop(); + // Armed by the export thread's own exit: `task.threads` then holds only + // the immortal background thread. + let taskThreads: () => number = () => 2; + const foreign = new ForeignThread( + () => taskThreads() === 1, + // Queued during the tick, so it runs after the driver's `done()` test + // (same synchronous stretch) and before the lift's `.then` (queued only + // when `driveAsync`'s promise resolves, one microtask later). + () => queueMicrotask(() => store.awaiting.add(hop)), + ); + store.startWaiting(foreign); + + let background!: Thread; + const fn = createLiftedFunction({ + name: "boot", + ft: FT, + opts, + core: () => { + const thread = currentThread() as unknown as { task: never }; + const task = thread.task as unknown as { + return_(r: never[]): void; + registerThread(t: Thread): void; + threads: unknown[]; + }; + // The consumer's spawned background futures: registered into this + // task and parked on a condition nothing ever satisfies. + background = new Thread( + task as never, + (function* () { + yield { readyFunc: () => false, cancellable: false }; + })(), + ); + task.registerThread(background); + background.resume(); + // `task.return` — the export has its answer while its background + // work keeps running. + task.return_([]); + taskThreads = () => task.threads.length; + // Park the export's own activation on a Promise, which is what forces + // the drive onto the asynchronous (`.then`) completion path. + return Promise.resolve().then(() => 0); + }, + stats: newStats(), + }); + + const out = fn() as Promise; + assert( + out instanceof Promise, + "an async-typed lift whose core parks must return a Promise", + ); + + const settled = await Promise.race([ + out.then(() => "resolved"), + // A few macrotask turns: everything this test needs is microtask work, + // so anything still pending here is pending forever. + new Promise((r) => setTimeout(() => r("pending"), 0)) + .then(() => new Promise((r) => setTimeout(() => r("pending"), 0))), + ]); + assertEq(foreign.resumed, 1); + assert( + store.awaiting.has(hop), + "the test never injected the foreign hop-park, so it proves nothing", + ); + assertEq(settled, "resolved"); + assert( + background.waiting(), + "the background thread must still be parked: without it the task is " + + "over and `backgroundCompletion` would settle on its own", + ); + assertEq(store.hostFailure, undefined); + + // Leave nothing live behind: the injected hop is parked on a promise that + // never settles, and the background thread on a condition that never + // holds, so any driver that inherited them would spin. + store.awaiting.delete(hop); + assertEq(store.pendingHostCalls.size, 0); + }, +});