Skip to content

embedder: result-conversion failure of a host import skips releaseAsyncArgs — stream/future arguments stay parked forever #356

Description

@lannbot

result-conversion failure of a host import skips releaseAsyncArgs — stream/future arguments stay parked forever

Severity: P2 Confidence: high
Location: runtime/src/embedder/instantiate.ts:805-806 (sync return: scope.end(); return ok(out);), :798-801 (convert → ok(settlement.value) / fail(settlement.error)), :767-769 (onReject is reached only from the throw/rejection arms), :1287-1298 (releaseAsyncArgs); consumed at runtime/src/exec/boundary.ts:1888-1892 (async arm convert under try/catch → store.hostFailure) and :1921 (sync arm, throw propagates through wasm). (baseline 1a4f5e1)
Authority: contracts/embedder-api.md §"Streams and futures" — "A trapping host import drops abandoned top-level stream/future arguments; this cleanup does not traverse compound arguments. Their peers can then settle rather than waiting on abandoned arguments." §"Error model" — "An unbranded throw from a host import is a host bug and traps".
Expected: A host import that fails by returning a value the adapter cannot convert (a TypeError from fromHost, e.g. "not a number" for a u32 result) fails the call exactly as a thrown host bug does: in the sync arm it traps and poisons the instance — and in both arms the top-level Stream/Future arguments handed to that import are torn down so their peers (here the host writer's writeAll and the host future's write) settle.
Actual: Conversion failure happens after dispatch(args) returned, in ok(out) (sync) or in convert inside the boundary's settlement continuation (async). Neither path runs onReject → releaseAsyncArgs. The sync variant poisons the instance (so it is a trapping import by the runtime's own treatment) yet leaves the argument ends alive and orphaned in the host's dead closure; the async variant records store.hostFailure (surfaced once on the next unrelated entry, ping) and likewise never releases the arguments. In both cases the peer writer stays pending indefinitely; the throw controls settle immediately.
Repro: f1_conversion_failure_args.ts (fixture runtime/tests/embedder/host-settlement.wasm, export consume(stream, future, 0)):

control: sync throw   ... "pendingHostCalls":0,"poisoned":true, "writtenState":"resolved:0" ... "fwWrite":"accepted"
control: async reject ... "pendingHostCalls":0,"poisoned":false,"writtenState":"resolved:0" ... "fwWrite":"accepted"
candidate: async resolves with non-u32 ... "hostFailure":"TypeError: import 'consume': u32 expects an integer number","poisoned":false,"writtenState":"pending" ... "fwWrite":"TIMEOUT","ping2":"<none>"
candidate: sync returns non-u32        ... "call":"TypeError: import 'consume': u32 expects an integer number","poisoned":true,"writtenState":"pending" ... "fwWrite":"TIMEOUT"

Distinct from known issues: #343 is the throwing then getter at the isThenable probe (instantiate.ts:778); this is the result-conversion site (ok()/convert, :806/:800), a different throw origin that #343's proposed fix (wrap the isThenable check) does not cover, and it also has an async-arm variant that #343 lacks. #118 asks whether conversion failures should trap; here the sync arm already traps and poisons — the missing piece is the trapping-import cleanup obligation, independent of how #118 is settled.
Fix direction: route ok()/fail() conversion failures through the same onReject(e) teardown as a thrown import body — in the sync arm by wrapping ok(out) in the existing try/catch shape, in the deferred arm by calling onReject(e) inside convert's failure path (or catching around ok(settlement.value) before rethrowing). The fallible-payload variant (fail(e) throwing a TypeError while converting a ComponentException payload) sits on the same path and should be included.

Related observation from the embedder-handles track (p1_partial_lower.ts case E, embedded in the sibling issue on export-side argument orphaning): for an import whose result is a record containing a stream and a still-deferred Future, the conversion TypeError did not surface anywhere (call resolved, store.hostFailure undefined) and the source ReadableStream stayed locked — the same ok(out) site, with the failure lost entirely rather than recorded.

Running the repro

Scripts below were run from the repository root at baseline 1a4f5e1 with
the shim and fixtures built (just shim fixtures):

deno run --config runtime/deno.json --allow-read --allow-env=POLYENGINE_SCHED_SEED <script>

<REPO>/ in the scripts is the absolute path of the checkout (the review ran
them from outside the tree). Each repro was run at least twice and once under a
nonzero POLYENGINE_SCHED_SEED; output was identical across runs.

support.ts

// Shared fixture plumbing for the import-boundary review probes.
// Run from the repo root:
//   deno run --config runtime/deno.json --allow-read --allow-env=POLYENGINE_SCHED_SEED <script>
export const REPO = "<REPO>/";
import { Translator } from "<REPO>/runtime/src/shim/mod.ts";
import {
  type EmbedderOptions,
  instantiate,
} from "<REPO>/runtime/src/embedder/mod.ts";

const shim = await Deno.readFile(
  REPO + "target/wasm32-unknown-unknown/release/translator_shim.wasm",
);
export const translator = await Translator.create(shim);

export async function fixture(
  rel: string,
  imports: Record<string, unknown>,
  opts: EmbedderOptions = {},
) {
  const componentBytes = await Deno.readFile(REPO + rel);
  const { plan, adapters } = translator.translate(componentBytes);
  return await instantiate({ plan, adapters, componentBytes }, imports, opts);
}

export const turn = () => new Promise<void>((r) => setTimeout(r, 0));
export const micro = () => Promise.resolve();

export async function caught(f: () => unknown): Promise<unknown> {
  try {
    await f();
  } catch (e) {
    return e;
  }
  return undefined;
}

export function describeErr(e: unknown): string {
  if (e === undefined) return "<none>";
  if (e instanceof Error) return `${e.name}: ${e.message.slice(0, 160)}`;
  return String(e);
}

export function guest(name: string): string {
  return `examples/guests/build/${name}.component.wasm`;
}

f1_conversion_failure_args.ts

// F1: result-conversion failure (TypeError from ok()/convert) skips releaseAsyncArgs:
// the host-held stream/future arguments are never torn down (sync arm poisons
// the instance; async arm records hostFailure) — compare the throw controls.
// P4: async import result conversion failure (TypeError, not a throw) —
// cleanup of stream/future args, hostFailure routing, instance state.
import { caught, describeErr, fixture, turn } from "./support.ts";
import { Future, Stream } from "<REPO>/runtime/src/embedder/streams.ts";
import { hostFuture } from "<REPO>/runtime/src/exec/host_streams.ts";
import { hasRealHostCall, isInstancePoisoned } from "<REPO>/runtime/src/task/mod.ts";

class R {
  disposed = 0;
  [Symbol.dispose]() {
    this.disposed++;
  }
}
const FIX = "runtime/tests/embedder/host-settlement.wasm";

async function run(label: string, consume: (...a: unknown[]) => unknown, jspi = false) {
  const c = await fixture(FIX, {
    r: R,
    make: () => new R(),
    makeSync: () => new R(),
    producers: { sources: () => { throw new Error("unused"); } },
    consume,
  }, { jspi });
  const { stream, writer } = Stream.create<number>();
  const fw = hostFuture<number>({ kind: "u32" });
  const future = Future.fromHostFuture(fw, {
    element: { kind: "u32" },
    toHost: (v: unknown) => v as number,
    fromHost: (v: number) => v,
  });
  const written = writer.writeAll(new Uint8Array([1, 2]));
  let writtenState = "pending";
  written.then((n) => writtenState = "resolved:" + n, (e) => writtenState = "rejected:" + describeErr(e));

  const call = await caught(() => c.exports.consume(stream, future, 0));
  await turn();
  await turn();
  const store = c.handle.componentInstances[0].store;
  const before = {
    call: describeErr(call),
    hostFailure: describeErr(store.hostFailure),
    pendingHostCalls: store.pendingHostCalls.size,
    realHostCall: hasRealHostCall(store),
    poisoned: isInstancePoisoned(c.handle.componentInstances[0]),
    writtenState,
  };
  // Next embedder call surfaces the parked failure?
  const ping = await caught(() => c.exports.ping());
  await turn();
  const fwWrite = await Promise.race([
    fw.write(7).then(() => "accepted", (e) => "rejected:" + describeErr(e)),
    new Promise<string>((r) => setTimeout(() => r("TIMEOUT"), 150)),
  ]);
  const after = {
    ping: describeErr(ping),
    hostFailure: describeErr(store.hostFailure),
    poisoned: isInstancePoisoned(c.handle.componentInstances[0]),
    writtenState,
    fwWrite,
    ping2: describeErr(await caught(() => c.exports.ping())),
  };
  console.log(label, JSON.stringify({ before, after }));
}

await run("control: sync throw", () => { throw new Error("boom"); });
await run("control: async reject", () => Promise.reject(new Error("boom")));
await run("candidate: async resolves with non-u32", () => Promise.resolve("not a number"));
await run("candidate: sync returns non-u32", () => "not a number");

Activity

  1. added
    bugSomething isn't working
    p2Minor bugs; desirable lower-priority features
    on Sep 12, 2026
  2. lannbot commented on Sep 12, 2026

    @lannbot
    CollaboratorAuthor

    Cross-check against the pinned Component Model reference (definitions.py @ 7c67611) and wasmtime 4675ee1. Verdict vocabulary: SPEC-BACKED = the reference mandates the expected behavior; CONTRACT-ONLY = spec silent, polyengine's own contract decides; POLICY-QUESTION = neither decides; WEAKENED = part of the claim is overstated (corrections below).

    Spec: canon_lower.on_resolve (definitions.py 2243-2255): flat_results = lower_flat_values(cx, max_flat_results, result, ft.result_type(), flat_args) — a failure lowering the host's result is a trap(), which ends the store; the guest's stream/future ends and their peers die with it. "Peer waits forever" is unreachable in the reference. The spec is silent on the polyengine-specific question (a non-trapping failure class for the async arm, store.hostFailure) — that is #118's territory.
    Contract: §"Streams and futures": "A trapping host import drops abandoned top-level stream/future arguments; this cleanup does not traverse compound arguments. Their peers can then settle rather than waiting on abandoned arguments." §"Error model": "An unbranded throw from a host import is a host bug and traps". The issue quotes these accurately. The sync arm does poison the instance on conversion failure (observed poisoned:true), so by the runtime's own treatment it is a trapping import and the drop obligation applies verbatim; the code path (ok(out) at instantiate.ts:806 and convert at :798-801) bypasses onReject (:767-769). For the async arm the contract does not decide whether a conversion failure is a trap (that is #118), but the "peers can then settle" obligation is the same either way once the call is failed.
    Wasmtime: same outcome class, different mechanism. call_sync_lower (func/host.rs:384-420): Self::lower_raw(&mut lower, ty, ret, dst) returns Err on a result-lowering failure, propagates as a trap into the wasm frame, catch_traps → set_trapped() (func.rs:1476-1479); the whole store is dead, so no peer can wait. Wasmtime has no "typed host value fails to convert" class — R: Lower is checked at compile time; the only runtime result-lowering failures are realloc/memory bounds, which are traps. No test pins the peer-settlement question because it cannot arise.
    Verdict: CONTRACT-ONLY — spec: whole-store failure; contract clause is explicit and the sync arm already satisfies its precondition ("trapping host import") while skipping the obligation.
    wasmtime: same behavior in effect (a result-lowering failure is a trap that kills the store, so peers never wait), by a coarser mechanism.
    Severity: keep P2 — a host writeAll/write pends indefinitely after a failure the runtime already classifies as poisoning; deterministic; contract-explicit.
    Notes: Keep the fix independent of #118: route both ok() and fail() conversion failures through onReject(e) regardless of whether the async arm is later made trapping. The fallible-payload variant (fail(e) throwing while converting a ComponentException payload) is correctly included — it is the same site.

  3. lannbot commented on Sep 12, 2026

    @lannbot
    CollaboratorAuthor

    Re-evaluated on current main 5616bce, Deno 2.9.5. Keep P2. Both malformed-result variants strand the peer writes; thrown/rejected import controls settle them. Unlike #354/#355's transient refusal path, the main trigger requires a malformed host return. Cleanup can be fixed independently of #118's error-classification decision.

    Route failures converting both success and error payloads through abandonment cleanup. Do not mistake a valid ComponentException return for a trapping import, and do not let cleanup replace the original conversion error.

    Correction to the appended case E: the original probe's Future materializes during the awaited second instantiation. A diagnostic replay reports successful RETURNED (2), not a swallowed conversion failure. A guaranteed-deferred follow-up surfaces TypeError normally and still leaks the stream prefix. Thus partial import-result construction needs cleanup, but the claim that this probe proves lost error reporting is unsupported. See the matching correction on #355.

  4. lannbot commented on Sep 13, 2026

    @lannbot
    CollaboratorAuthor

    Rechecked current main ae86a4a after #369, Deno 2.9.5. Still reproducible; keep P2/open.

    f1_conversion_failure_args.ts: both synchronous non-u32 return and asynchronously fulfilled non-u32 return leave the stream writer pending and future write timing out. Synchronous throw / async rejection controls still release the arguments. The synchronous malformed-result case poisons; the asynchronous case reports the TypeError through hostFailure once and leaves the instance live.

    The new lifecycle fixes invocation/thenable-observation failure cleanup, but finishHostCall calls hooks.finish without routing preparation/conversion errors through hooks.reject. Prepared-value custody owns newly acquired result values, not the top-level async arguments already given to the host import. Those arguments still require the failure cleanup this issue asks for.

    The partial-result construction subcase from #355 is fixed by #369; it does not close this import-argument abandonment defect. #118's error-classification policy remains separate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingp2Minor bugs; desirable lower-priority features

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions