From bc270e596818d36e7b57a5121682b98489566b00 Mon Sep 17 00:00:00 2001 From: Lann Martin Date: Sun, 6 Sep 2026 22:03:26 -0400 Subject: [PATCH] task core: port wait_until's pending-cancel wakeup; clear Thread.cancellable on resume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial correctness review of the task core against definitions.py. `Thread.waitUntil` parked on the raw `readyFunc` and checked `deliverPendingCancel` only BEFORE blocking. The reference (`Thread.wait_until` 361-373) parks on `ready_func() or (cancellable() and has_pending_cancel())` and re-runs `deliver_pending_cancel` AFTER the block. The gap bites when a cancel arrives while the task is not cancellable — a sibling activation of the instance holds the exclusive slot, so `Task.requestCancellation` parks the request as `pending-cancel`. When the slot frees, the reference wakes the parked callback task and hands it TASK_CANCELLED; ours left the WAIT arm parked until some unrelated event landed on its set (possibly never: the canceller got BLOCKED and waits for a SUBTASK event that never comes), delivered a spurious NONE on the YIELD arm, and reordered a pending event ahead of TASK_CANCELLED. The reference's `cancellable` is a live predicate (`lock_available` in the callback loop, 2167/2175); ours is a static flag per block point, so the "lock is free" conjunct is supplied by the new `Task.implicitThreadCancellable()`, which also replaces the inline `excludeImplicit` expression in `requestCancellation`. `Thread.cancellable` stayed stale-true while the thread ran, so a `requestCancellation` reaching a RUNNING implicit thread (a nested same- instance task cancelling a handle whose callee is on the stack) picked it and hit `Thread.resume`'s assertion instead of parking as pending-cancel. Cleared on resume; re-set by the next block request. This is CM#707 item 2(c) of #250 — "deliver when the callee's exclusive lock frees, even if the parked task's waitable set never fires". `async/cancel-and-exclusive-lock.wast:196` now passes; its xfail (the last of class `cm707-cancel`) is removed. Regression: wait_until_pending_cancel_test.ts (three arms), stale_cancellable_flag_test.ts. Conformance 0 failed / 0 stale; sched-seeds green. Closes #250. --- README.md | 2 +- harness/browser/expectations/chromium.ts | 4 +- harness/browser/expectations/firefox.ts | 4 +- harness/shell/expectations/bun-pinned.ts | 4 +- harness/shell/expectations/jsc-pinned.ts | 4 +- harness/shell/expectations/jsc-trunk.ts | 4 +- harness/shell/expectations/node-pinned.ts | 4 +- harness/shell/expectations/sm-nightly.ts | 4 +- harness/shell/expectations/sm-pinned.ts | 4 +- harness/src/xfail.ts | 24 -- runtime/src/task/mod.ts | 28 ++- runtime/src/task/thread.ts | 28 ++- runtime/tests/stale_cancellable_flag_test.ts | 86 ++++++++ .../tests/wait_until_pending_cancel_test.ts | 207 ++++++++++++++++++ 14 files changed, 360 insertions(+), 47 deletions(-) create mode 100644 runtime/tests/stale_cancellable_flag_test.ts create mode 100644 runtime/tests/wait_until_pending_cancel_test.ts diff --git a/README.md b/README.md index abc236b..a3b819c 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ up via JSPI where the engine provides it. Pre-1.0, but densely gated: -- **Official Component Model test suite**: 1284 passing / 0 failing commands +- **Official Component Model test suite**: 1285 passing / 0 failing commands across all directories (remaining: named xfail classes — deferred threads ([#12](https://github.com/polymorph-components/polyengine/issues/12)), cm705-sync-sched ([#249](https://github.com/polymorph-components/polyengine/issues/249)), diff --git a/harness/browser/expectations/chromium.ts b/harness/browser/expectations/chromium.ts index 8da1e20..d5bd9b5 100644 --- a/harness/browser/expectations/chromium.ts +++ b/harness/browser/expectations/chromium.ts @@ -50,9 +50,9 @@ export const chromium: LaneExpectation = { totals: { commands: 1511, executed: 1411, - passed: 1284, + passed: 1285, failed: 0, - xfail: 127, + xfail: 126, pendingRuntime: 95, pendingCapability: 0, unsupportedDirective: 5, diff --git a/harness/browser/expectations/firefox.ts b/harness/browser/expectations/firefox.ts index 4c77136..3fb19c1 100644 --- a/harness/browser/expectations/firefox.ts +++ b/harness/browser/expectations/firefox.ts @@ -54,9 +54,9 @@ export const firefox: LaneExpectation = { totals: { commands: 1511, executed: 1411, - passed: 1284, + passed: 1285, failed: 0, - xfail: 127, + xfail: 126, pendingRuntime: 95, pendingCapability: 0, unsupportedDirective: 5, diff --git a/harness/shell/expectations/bun-pinned.ts b/harness/shell/expectations/bun-pinned.ts index b7f14b8..bcaf1c5 100644 --- a/harness/shell/expectations/bun-pinned.ts +++ b/harness/shell/expectations/bun-pinned.ts @@ -54,9 +54,9 @@ export const bunPinned: ShellLaneExpectation = { totals: { commands: 1511, executed: 1411, - passed: 1284, + passed: 1285, failed: 0, - xfail: 127, + xfail: 126, pendingRuntime: 95, pendingCapability: 0, unsupportedDirective: 5, diff --git a/harness/shell/expectations/jsc-pinned.ts b/harness/shell/expectations/jsc-pinned.ts index 44c47bf..7525269 100644 --- a/harness/shell/expectations/jsc-pinned.ts +++ b/harness/shell/expectations/jsc-pinned.ts @@ -62,9 +62,9 @@ export const jscPinned: ShellLaneExpectation = { totals: { commands: 1511, executed: 1411, - passed: 1284, + passed: 1285, failed: 0, - xfail: 127, + xfail: 126, pendingRuntime: 95, pendingCapability: 0, unsupportedDirective: 5, diff --git a/harness/shell/expectations/jsc-trunk.ts b/harness/shell/expectations/jsc-trunk.ts index de3ef3a..4b39b2c 100644 --- a/harness/shell/expectations/jsc-trunk.ts +++ b/harness/shell/expectations/jsc-trunk.ts @@ -76,9 +76,9 @@ export const jscTrunk: ShellLaneExpectation = { totals: { commands: 1511, executed: 1411, - passed: 1284, + passed: 1285, failed: 0, - xfail: 127, + xfail: 126, pendingRuntime: 95, pendingCapability: 0, unsupportedDirective: 5, diff --git a/harness/shell/expectations/node-pinned.ts b/harness/shell/expectations/node-pinned.ts index 68a73a2..30d042a 100644 --- a/harness/shell/expectations/node-pinned.ts +++ b/harness/shell/expectations/node-pinned.ts @@ -51,9 +51,9 @@ export const nodePinned: ShellLaneExpectation = { totals: { commands: 1511, executed: 1411, - passed: 1284, + passed: 1285, failed: 0, - xfail: 127, + xfail: 126, pendingRuntime: 95, pendingCapability: 0, unsupportedDirective: 5, diff --git a/harness/shell/expectations/sm-nightly.ts b/harness/shell/expectations/sm-nightly.ts index b44942f..555dc48 100644 --- a/harness/shell/expectations/sm-nightly.ts +++ b/harness/shell/expectations/sm-nightly.ts @@ -62,9 +62,9 @@ export const smNightly: ShellLaneExpectation = { totals: { commands: 1511, executed: 1411, - passed: 1284, + passed: 1285, failed: 0, - xfail: 127, + xfail: 126, pendingRuntime: 95, pendingCapability: 0, unsupportedDirective: 5, diff --git a/harness/shell/expectations/sm-pinned.ts b/harness/shell/expectations/sm-pinned.ts index be7e08a..d6e0b9c 100644 --- a/harness/shell/expectations/sm-pinned.ts +++ b/harness/shell/expectations/sm-pinned.ts @@ -61,9 +61,9 @@ export const smPinned: ShellLaneExpectation = { totals: { commands: 1511, executed: 1411, - passed: 1284, + passed: 1285, failed: 0, - xfail: 127, + xfail: 126, pendingRuntime: 95, pendingCapability: 0, unsupportedDirective: 5, diff --git a/harness/src/xfail.ts b/harness/src/xfail.ts index f8da620..f43d8d1 100644 --- a/harness/src/xfail.ts +++ b/harness/src/xfail.ts @@ -353,30 +353,6 @@ export const XFAIL: XfailEntry[] = [ // shim decoder gap). // ===================================================================== // --- async/builtin-trap-poisons-instance.json: root cause: STREAMS --- - // --- async/cancel-and-exclusive-lock.json: CM#707 "always deliver - // cancellation as soon as possible" (third_party/component-model commit - // 1af0b35, pulled in by the CM#705 pin advance polyengine#173) changed - // when a pending cancellation must be delivered; polyengine's task - // scheduler still implements the pre-#707 delivery timing, so the - // cross-instance exclusive-lock scenario this file drives deadlocks - // instead of the callee observing cancellation. Classed `cm707-cancel`, - // https://github.com/polymorph-components/polyengine/issues/250. - // - // NOTE on hang risk (dispatch warning): this file does NOT wedge the - // harness. It completes crisply with our OWN "deadlock detected: event - // loop cannot make further progress" trap rather than looping forever — - // the pre-#707 runtime deadlock manifests as a clean trap, not a stall. - // No harness-visible skip mechanism was needed. --- - { - file: "async/cancel-and-exclusive-lock.json", - line: 196, - reason: - "expected return, got trap: wasm trap: deadlock detected: event " + - "loop cannot make further progress — polyengine has not implemented " + - "CM#707's immediate-cancellation-delivery timing yet, so the " + - "callee never observes the pending cancellation and the scheduler " + - "finds no ready thread; cm707-cancel, https://github.com/polymorph-components/polyengine/issues/250", - }, // --- async/cancel-stream.json: root cause: STREAMS --- // --- async/closed-stream.json: root cause: STREAMS --- // --- async/cross-abi-calls.json: root cause: FACT-ASYNC --- diff --git a/runtime/src/task/mod.ts b/runtime/src/task/mod.ts index 757b153..712025f 100644 --- a/runtime/src/task/mod.ts +++ b/runtime/src/task/mod.ts @@ -349,9 +349,7 @@ export class Task { resume(cancelled?: boolean): void; }; let candidates: Cancellable[] = this.threads.filter((t) => t.cancellable); - const excludeImplicit = this.ft.async === true && this.needsExclusive() && - this.inst.exclusiveThread !== null && - this.inst.exclusiveThread !== this.implicitThread; + const excludeImplicit = !this.implicitThreadCancellable(); if (excludeImplicit) { candidates = candidates.filter((t) => t !== this.implicitThread); } @@ -407,9 +405,31 @@ export class Task { } } + /** + * Is the implicit thread cancellable *right now*? + * + * The reference makes cancellability a live predicate — the callback loop + * passes `cancellable = lock_available` (definitions.py 2167/2175), false + * while a sibling activation of the instance holds the exclusive slot. We + * carry a static flag per block point instead, so this is where the "and + * the lock is free" conjunct lives: both `request_cancellation`'s candidate + * filter and `Thread.wait_until`'s pending-cancel wakeup disjunct consult + * it. + */ + implicitThreadCancellable(): boolean { + return !(this.ft.async === true && this.needsExclusive() && + this.inst.exclusiveThread !== null && + this.inst.exclusiveThread !== this.implicitThread); + } + + /** definitions.py `Task.has_pending_cancel` (line 533). */ + hasPendingCancel(): boolean { + return this.state === "pending-cancel"; + } + /** definitions.py `Task.deliver_pending_cancel` (line 536). */ deliverPendingCancel(cancellable: boolean): boolean { - if (cancellable && this.state === "pending-cancel") { + if (cancellable && this.hasPendingCancel()) { this.state = "cancel-delivered"; return true; } diff --git a/runtime/src/task/thread.ts b/runtime/src/task/thread.ts index e9993b0..81b520e 100644 --- a/runtime/src/task/thread.ts +++ b/runtime/src/task/thread.ts @@ -63,7 +63,14 @@ export class Thread implements SchedulableThread { /** Slot in `inst.threads`, assigned by `Task.registerThread`. */ index: number | null = null; - /** definitions.py `Thread.cancellable` — set at each block point. */ + /** + * definitions.py `Thread.cancellable` — set at each block point, cleared + * while the thread runs. The reference evaluates it as a live predicate + * (`cancellable = lock_available` in the callback loop, line 2167), so a + * thread that is not parked is never a `request_cancellation` candidate; + * clearing on resume gives the same answer for the only shape that differs + * (a running implicit thread that still holds the exclusive slot). + */ cancellable = false; #state: ThreadState = "suspended"; @@ -196,6 +203,7 @@ export class Thread implements SchedulableThread { #resumeInternal(sendValue: unknown, failure?: { error: unknown }): void { this.#state = "running"; + this.cancellable = false; pushCurrentThread(this); let step: IteratorResult; try { @@ -258,7 +266,23 @@ export class Thread implements SchedulableThread { ): Generator { assert_(this.running(), "waitUntil on a non-running thread"); if (this.task.deliverPendingCancel(cancellable)) return CANCELLED_TRUE; - const cancelled = yield { readyFunc, cancellable }; + // definitions.py `ready_or_cancelled` (line 369): a cancel that arrived + // while this task was not cancellable (parked as `pending-cancel`) makes + // the thread ready on its own — otherwise the wakeup is lost until some + // unrelated event happens to satisfy `readyFunc`. The reference's + // `cancellable()` is a live predicate; ours is the static flag AND + // `Task.implicitThreadCancellable` (the "lock is free" conjunct the + // callback loop's `lock_available` supplies there). + const readyOrCancelled = () => + readyFunc() || + (cancellable && this.task.hasPendingCancel() && + (this !== this.task.implicitThread || + this.task.implicitThreadCancellable())); + const cancelled = yield { readyFunc: readyOrCancelled, cancellable }; + // AFTER the block (line 372): converts a plain wakeup taken through the + // pending-cancel disjunct into Cancelled.TRUE, and wins over any event + // that became pending in the meantime. + if (this.task.deliverPendingCancel(cancellable)) return CANCELLED_TRUE; return cancelled; } diff --git a/runtime/tests/stale_cancellable_flag_test.ts b/runtime/tests/stale_cancellable_flag_test.ts new file mode 100644 index 0000000..eda2f2d --- /dev/null +++ b/runtime/tests/stale_cancellable_flag_test.ts @@ -0,0 +1,86 @@ +// `Thread.cancellable` is set at each block point and never cleared on resume, +// so a callback task's implicit thread still looks cancellable while its frame +// RUNS holding `inst.exclusiveThread`. `Task.requestCancellation` then picks it +// (`excludeImplicit` is false — the holder IS the implicit thread) and calls +// `Thread.resume`, which asserts "resume() on a running or finished thread". +// The reference evaluates cancellability live (`cancellable = lock_available`, +// definitions.py 2167/2175): with the slot held it is False, so +// `request_cancellation` (499-503) records PENDING_CANCEL and delivers it at +// the task's next cancellable wait. + +import { assertEq } from "./support/asserts.ts"; +import { + type BlockRequest, + type Cancelled, + ComponentInstanceState, + Store, + Task, + type TaskOptions, + Thread, +} from "../src/task/mod.ts"; +import type { FuncType } from "../src/cabi/types.ts"; + +const ASYNC_FT: FuncType = { params: [], results: [], async: true }; + +const CALLBACK_OPTS: TaskOptions = { + async_: true, + callback: true, + stringEncoding: "utf8", + memory: null, +}; + +function spawn( + task: Task, + body: (t: Thread) => Generator, +): Thread { + let thread!: Thread; + thread = new Thread( + task, + (function* (): Generator { + yield* body(thread); + })(), + ); + return thread; +} + +Deno.test("requestCancellation reaching a RUNNING implicit thread parks as pending-cancel", () => { + const store = new Store(); + const inst = new ComponentInstanceState(0, store); + const a = new Task(ASYNC_FT, CALLBACK_OPTS, inst, () => [], () => {}); + + let thrown: unknown = null; + let stateAfterRequest = ""; + + const ta = spawn(a, function* (thread) { + yield* a.enterImplicitThread(thread); // takes inst.exclusiveThread + a.start(); + // A cancellable wait that returns normally; `thread.cancellable` stays true. + yield* thread.waitUntil(() => true, true); + // Now the frame is RUNNING and holds the exclusive slot. This stands in + // for a nested task of the caller's instance running `subtask.cancel` on + // A's handle through a FACT sync start-call. + try { + a.requestCancellation(null); + } catch (e) { + thrown = e; + } + stateAfterRequest = a.state; + // Reference: the request is pending and is delivered at the next + // cancellable block point. + const cancelled = yield* thread.waitUntil(() => true, true); + if (cancelled) a.cancel(); + else a.return_([]); + a.exitImplicitThread(thread); + }); + ta.resume(); + for (let i = 0; i < 20 && store.tick(); i++); + + if (thrown !== null) { + throw new Error( + `requestCancellation on a running implicit thread threw instead of ` + + `parking the request: ${thrown}`, + ); + } + assertEq(stateAfterRequest, "pending-cancel"); + assertEq(a.state, "resolved"); +}); diff --git a/runtime/tests/wait_until_pending_cancel_test.ts b/runtime/tests/wait_until_pending_cancel_test.ts new file mode 100644 index 0000000..b2e4351 --- /dev/null +++ b/runtime/tests/wait_until_pending_cancel_test.ts @@ -0,0 +1,207 @@ +// `Thread.waitUntil` drops the reference's cancel-aware ready predicate and +// its post-block `deliver_pending_cancel` (definitions.py `Thread.wait_until` +// lines 361-373: `ready_or_cancelled = ready_func() or (cancellable() and +// task.has_pending_cancel())`, then `if deliver_pending_cancel: return +// Cancelled.TRUE` AFTER the block). Ours parks on the raw `readyFunc` and +// returns the raw cancelled flag, so a callback task whose cancel went +// `pending-cancel` behind a sibling's `exclusiveThread` is (a) never woken at +// all when the slot frees with no event pending, (b) woken with a spurious +// NONE in the YIELD arm, (c) handed the queued event before TASK_CANCELLED. +// +// The bodies below mirror exec/boundary.ts `runCallbackLoop`'s WAIT/YIELD arms +// (:2640-2700) — release the slot across the wait, retake it after — with fake +// threads, no wasm. + +import { assertEq } from "./support/asserts.ts"; +import { + type BlockRequest, + type Cancelled, + ComponentInstanceState, + EventCode, + type EventTuple, + Store, + Task, + type TaskOptions, + Thread, + Waitable, + WaitableSet, +} from "../src/task/mod.ts"; +import type { FuncType } from "../src/cabi/types.ts"; + +const ASYNC_FT: FuncType = { params: [], results: [], async: true }; + +/** Async-typed, callback ABI: `needsExclusive()` is true. */ +const CALLBACK_OPTS: TaskOptions = { + async_: true, + callback: true, + stringEncoding: "utf8", + memory: null, +}; + +function spawn( + task: Task, + body: (t: Thread) => Generator, +): Thread { + let thread!: Thread; + thread = new Thread( + task, + (function* (): Generator { + yield* body(thread); + })(), + ); + return thread; +} + +function mkTask(inst: ComponentInstanceState, opts: TaskOptions): Task { + return new Task(ASYNC_FT, opts, inst, () => [], () => {}); +} + +/** Drive the store to quiescence; bounded so a lost wakeup is not a hang. */ +function runToQuiescence(store: Store): void { + for (let i = 0; i < 50 && store.tick(); i++); +} + +/** + * Sibling B of the same instance: takes the exclusive slot, then parks + * NON-cancellably mid-activation (stands in for a jspi sync lower of an + * unresolved `suspending()` import). Opening the gate lets it return and + * release the slot. + */ +function spawnSibling( + inst: ComponentInstanceState, + gate: { open: boolean }, +): Thread { + const b = mkTask(inst, CALLBACK_OPTS); + const tb = spawn(b, function* (thread) { + yield* b.enterImplicitThread(thread); + b.start(); + yield* thread.waitUntil(() => gate.open, false); + b.return_([]); + b.exitImplicitThread(thread); + }); + tb.resume(); + return tb; +} + +Deno.test("callback WAIT: cancel pending behind a sibling's exclusive slot is delivered when the slot frees", () => { + const store = new Store(); + const inst = new ComponentInstanceState(0, store); + const wset = new WaitableSet(); // empty: no event will ever arrive + const events: EventTuple[] = []; + + const a = mkTask(inst, CALLBACK_OPTS); + const ta = spawn(a, function* (thread) { + yield* a.enterImplicitThread(thread); // takes inst.exclusiveThread + a.start(); + inst.exclusiveThread = null; // runCallbackLoop: release across the wait + const ev = yield* wset.waitForEventAnd( + thread, + () => inst.exclusiveThread === null, + true, + ); + events.push(ev); + inst.exclusiveThread = thread; // retake, as the loop does + if (ev[0] === EventCode.TASK_CANCELLED) a.cancel(); + else a.return_([]); + a.exitImplicitThread(thread); + }); + ta.resume(); + + const gate = { open: false }; + const tb = spawnSibling(inst, gate); + assertEq(inst.exclusiveThread === tb, true, "B holds the exclusive slot"); + + a.requestCancellation(null); + // Agreed by both: A is not cancellable while B holds the lock. + assertEq(a.state, "pending-cancel"); + + gate.open = true; + runToQuiescence(store); + assertEq(inst.exclusiveThread, null, "B released the slot"); + + // Reference: A is ready via `cancellable() and has_pending_cancel()`, + // resumes, and the post-block `deliver_pending_cancel` yields TASK_CANCELLED. + assertEq(events, [[EventCode.TASK_CANCELLED, 0, 0]]); + assertEq(a.state, "resolved"); +}); + +Deno.test("callback YIELD: a pending cancel released by the slot must resume as cancelled, not as NONE", () => { + const store = new Store(); + const inst = new ComponentInstanceState(0, store); + const observed: boolean[] = []; + + const a = mkTask(inst, CALLBACK_OPTS); + const ta = spawn(a, function* (thread) { + yield* a.enterImplicitThread(thread); + a.start(); + inst.exclusiveThread = null; + const cancelled = yield* thread.waitUntil( + () => inst.exclusiveThread === null, + true, + ); + observed.push(cancelled === true); + inst.exclusiveThread = thread; + if (cancelled) a.cancel(); + else a.return_([]); + a.exitImplicitThread(thread); + }); + ta.resume(); + + const gate = { open: false }; + spawnSibling(inst, gate); + a.requestCancellation(null); + assertEq(a.state, "pending-cancel"); + + gate.open = true; + runToQuiescence(store); + + // Reference: the post-block `deliver_pending_cancel` converts the + // Cancelled.FALSE resumption into Cancelled.TRUE, so the callback sees + // (TASK_CANCELLED,0,0). Ours resumes with false → a spurious (NONE,0,0). + assertEq(observed, [true]); +}); + +Deno.test("callback WAIT with an event pending: TASK_CANCELLED is delivered first and the event stays pending", () => { + const store = new Store(); + const inst = new ComponentInstanceState(0, store); + const wset = new WaitableSet(); + const events: EventTuple[] = []; + + const a = mkTask(inst, CALLBACK_OPTS); + const ta = spawn(a, function* (thread) { + yield* a.enterImplicitThread(thread); + a.start(); + inst.exclusiveThread = null; + const ev = yield* wset.waitForEventAnd( + thread, + () => inst.exclusiveThread === null, + true, + ); + events.push(ev); + inst.exclusiveThread = thread; + if (ev[0] === EventCode.TASK_CANCELLED) a.cancel(); + else a.return_([]); + a.exitImplicitThread(thread); + }); + ta.resume(); + + const gate = { open: false }; + spawnSibling(inst, gate); + a.requestCancellation(null); + assertEq(a.state, "pending-cancel"); + + // Arm the set: a waitable with a queued SUBTASK event, joined before the + // slot frees, so both disjuncts of the reference's ready predicate hold. + const w = new Waitable(); + w.join(wset); + w.setPendingEvent(() => [EventCode.SUBTASK, 7, 0]); + + gate.open = true; + runToQuiescence(store); + + // Reference: `deliver_pending_cancel` runs AFTER the block and BEFORE + // `get_pending_event`, so TASK_CANCELLED wins and the SUBTASK event is left + // on the waitable. Ours delivers the SUBTASK event first. + assertEq(events, [[EventCode.TASK_CANCELLED, 0, 0]]); + assertEq(w.hasPendingEvent(), true, "the SUBTASK event is still pending"); +});