Visitar URL original
fix(napi): stop unloading addons with live native code, preserve non-… · napi-rs/napi-rs@61fae8a · GitHub
Skip to content

Commit 61fae8a

Browse files
Brooooooklynclaude
andauthored
fix(napi): stop unloading addons with live native code, preserve non-Error rejections, and add the wasm teardown barrier (#3423)
* feat(napi): add Error::from_unknown_without_coercion JavaScript lets any value be thrown or used to reject a promise, but `From<Unknown> for Error` assumes an object. It calls `napi_create_reference` on the value and then `napi_coerce_to_string` on it, and both are wrong for a value that is not an object: * `napi_create_reference` rejects primitives with `napi_invalid_arg` below Node-API 10, so the value is lost and replaced by `Error { InvalidArg, "Create Error reference failed" }`. * `napi_coerce_to_string` runs `toString`/`Symbol.toPrimitive`, that is arbitrary user code, while an error is already unwinding. `from_unknown_without_coercion` does neither. The value is retained behind a private holder object, which `napi_create_reference` accepts for every value type, so converting the `Error` back with `ToNapiValue` reproduces the original value identically. `Error::reason` is filled in only from data that can be read without running JavaScript: a real `Error`'s own string `message`, or a string primitive itself. A property read that throws is swallowed and its exception cleared, so a hostile accessor cannot poison the environment. `ErrorRef` grows an `indirect` flag to record that a reference points at a holder rather than at the value; `referenced_value` unwraps it, so the indirection never escapes `Error`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): preserve the value a promise or async generator rejected with `Promise.reject('boom')` awaited from Rust produced `Error { InvalidArg, "Create Error reference failed" }`, and `asyncIterator.throw('boom')` produced the same, because both captured the value with `From<Unknown> for Error` and `napi_create_reference` refuses primitives below Node-API 10. Whatever JavaScript actually threw never reached the addon, and never made it back out to JavaScript either. Both paths now use `Error::from_unknown_without_coercion`, which retains the value regardless of its type and derives the reason without running user code. `DeferredTrace::into_rejected` needed the matching fix: it reused the retained value as the rejection whenever that value had a `message` property, but blanked out `message` and `code` otherwise instead of falling back to the captured reason. A primitive is not even an object, so the lookup is now tolerant and the fallback reports what was captured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * feat(napi): add retain_current_module_for_unload_safety Node unloads an addon when the environment that loaded it goes away and no other environment holds it. On Windows that is a `FreeLibrary` which drops the module's reference count to zero and unmaps the image. Native code still reachable from a thread that outlives the environment then points into unmapped memory: the observed symptom is a `0xC0000005` access violation raised from a Tokio waker vtable, in an addon that was loaded only inside a worker whose thread then exited. `retain_current_module_for_unload_safety` takes one extra loader reference to the image and never releases it, so the addon's code stays mapped for the rest of the process. Windows uses `GetModuleHandleExW` with `PIN | FROM_ADDRESS`, declared inline so no new Windows crate is needed; the dlopen family resolves the image with `dladdr` and bumps its reference count with `dlopen(RTLD_NOLOAD)`, which is why `libc` is now a `cfg(unix)` dependency. The reference is taken at most once per process. It is best effort by design. Targets with no portable loader-pinning primitive, and failures of the underlying call, leave the image unpinned, which is exactly the behavior addons had before this existed, so it can never make things worse than the status quo. The built-in Tokio runtime is the first caller: `RT` is process-global and its workers are never joined at environment teardown, so their task wakers keep vtable pointers into the addon after Node may have unloaded it. A runtime supplied through `create_custom_tokio_runtime` already owns threads by the time it is handed over, so that pins immediately rather than at first use. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): stop aborting the process when a ThreadsafeFunction release fails `Drop for ThreadsafeFunctionHandle` asserted that `napi_release_threadsafe_function` returned `napi_ok`. It does not during an environment teardown: Node closes the threadsafe function first and reports `napi_closing` to whichever handle drops afterwards. The assert then panics inside a destructor reached from a Node callback or from a thread being torn down, unwinds across the FFI boundary, and aborts the process. Worker exits hit this. The failure is now handled instead of asserted. Since the release did not happen, native code that can still reach the threadsafe function may outlive the environment, so the addon image is retained rather than the process being torn down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(examples): type the resolved-promise case as Promise<undefined> `describePromiseRejection` takes `Promise<()>`, which napi-derive emits as `Promise<undefined>`. `Promise.resolve()` infers `Promise<void>`, and `void` is not assignable to `undefined`, so `tsc` rejected the call. This broke two CI steps that both typecheck the examples: the freebsd `Build` step (`yarn tsc -p examples/napi/tsconfig.json`) and the linux-gnu `Unit tests` step (the same with `--noEmit --skipLibCheck`). The `Promise.reject(...)` cases were fine — those infer `Promise<never>`, which is assignable to anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): release an Error's JS reference only on its owning thread A `napi_ref` belongs to the env that created it and may only be touched from that env's JavaScript thread. `ErrorRef` relied on the per-env custom-GC handle to enforce that, which covers the common case but leaves two holes: * When no handle was captured — a pre-`napi4` build, or a reference created before module registration — `ErrorRef::drop` fell through to a direct `napi_reference_unref`/`napi_delete_reference` on whatever thread the last `Arc` happened to drop on. On native that mutates V8's `GlobalHandles` concurrently with the JS thread; the corruption surfaces much later as a SIGSEGV inside V8. * `Error::referenced_value` likewise read the reference unconditionally in that case, and never checked that the `env` it was handed is the one the reference was created for. Record the creating thread in `ErrorRef` and gate both paths on it. A read from the wrong env or thread reports the reference as absent, so the caller rebuilds a fresh error from `reason` — the same fallback already used for an off-thread custom-GC drop. A release from the wrong thread leaks instead of calling into napi; the leak is bounded, because env teardown reclaims the reference, while the call is not. `release_error_reference` also stops deleting a reference whose `napi_reference_unref` failed: `ref_count` is not meaningful after a failed call, so the delete was a guess. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): restore JavaScript error identity on wasm targets On `wasm32-wasip1` and `wasm32-wasip1-threads` a JavaScript error thrown by user code lost everything but its message on the way to Rust and back. A `TypeError` carrying a `cause` and custom properties arrived as a bare `Error { code: 'GenericFailure' }`, because two paths refused to retain the thrown value on wasm: * `retain_value_without_coercion` returned `None`, so `Error::from_unknown_without_coercion` — the promise-rejection and `AsyncGenerator.throw()` capture path — kept no reference. * `call_js_cb` forced `maybe_ref` to `None` for a ThreadsafeFunction callback that throws synchronously. Both were added because releasing the reference from a thread other than the one that created it crashes the wasi worker with `Cannot read properties of undefined (reading 'checkGCAccess')`: emnapi resolves `napi_env` per agent, and a wasi worker has no entry for the main agent's env (#2975, #3369). The carve-outs predate the per-env custom-GC routing that now sends such a release back to the owning agent, and were never revisited afterwards. They are no longer needed. An off-thread release is routed through the env's custom-GC threadsafe function — the mechanism `Buffer`, `TypedArray` and `FunctionRef` already rely on for their own `napi_ref`s on wasm, with no target carve-out — and the preceding commit makes the case where no handle was captured leak rather than call into napi. `Error::referenced_value` refuses to resolve a reference off its owning env and thread, so an `Error` that travels to a worker still rebuilds a plain error there instead of dereferencing a foreign env's handle. The value is retained behind a private holder object, which emnapi accepts even though it reports Node-API version 8, where `napi_create_reference` rejects primitives outright. Retaining the exception in `call_js_cb` also stops reporting its own failure as the callback's status. That status raises a fatal exception, so `throw 'oops'` in a ThreadsafeFunction callback — a primitive, which `napi_create_reference` refuses below Node-API 10 — killed the process over an error that had already been delivered to the Rust callback. It survived on wasm only because the carve-out hardcoded `napi_ok`. The `WASI_TEST` branches that documented the degraded behaviour are gone; those assertions now match every other target. The generated WASI loaders pick up `describePromiseRejection`, which they were missing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * feat(napi): add the napi_prepare_wasm_env_cleanup teardown barrier Every generated WASI loader — and emnapi's own `disposeNapiModule` — looks `napi_prepare_wasm_env_cleanup` up on the instantiated module and calls it, synchronously and on the main thread, as the first step of disposing the environment. That is the last moment at which the environment is still active and native code can still call into JavaScript: immediately afterwards `Env::beginTeardown` flips emnapi's `canCallIntoJs` to `false`, a settle from a task that is still running can no longer reach its threadsafe function, and the promise it owned can never settle. napi never defined the symbol. Native targets get this ordering from Node for free, because `napi_register_module_v1` registers `thread_cleanup` with `napi_add_env_cleanup_hook` and Node runs cleanup hooks before it finalizes threadsafe functions. wasm has no equivalent: the only teardown callback there is the `exports` finalizer installed with `napi_wrap`, which runs deep inside the environment teardown, long after JavaScript calls are disabled. So on wasm the teardown simply happened too late, and a real addon (rolldown) hit it as thread '<unnamed>' panicked at crates/napi/src/js_values/deferred.rs:311: Call threadsafe function in JsDeferred failed RuntimeError: unreachable Nothing reported the missing barrier, and that is the insidious part. `napi-build` links the export with `--export-if-defined`, so a binary without the symbol still links, and every loader guards the call with `typeof … === 'function'`, so a binary without the symbol still loads and runs. The link succeeded, the addon loaded, and the drain was silently absent. The export performs exactly the teardown the `exports` finalizer already performs — `shutdown_async_runtime()` — only early enough to be useful, so this adds no new teardown semantics on any target. A registered `AsyncRuntime` backend's `shutdown` contract makes it a real barrier: it returns only once every backend-owned thread, task, and blocking closure has quiesced, and tasks dropped by that shutdown reject their promises through the cancellation callback, which can still reach JavaScript from here. The built-in Tokio path keeps its existing best-effort `shutdown_background` semantics: it starts the drain here instead of after teardown, but does not join Tokio's workers. Blocking on them would be the wrong trade on wasm, where this runs on the only thread that can drain the threadsafe-function queue and, in a browser, may not block at all. `--export-if-defined` stays: `napi-build` and `napi` are versioned independently, so a new `napi-build` can still be unified with a `napi` that predates the symbol, and turning it into a hard `--export` would break that combination at link time. Detecting an absent barrier is left to a test that inspects the built artifact instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * test(examples): cover the wasm environment teardown barrier Two layers, because the barrier fails in two different silent ways. `wasi-env-cleanup-export.spec.ts` asserts the built wasm artifacts really export `napi_prepare_wasm_env_cleanup`. This is presence, not behavior, and it exists precisely because absence is undetectable at runtime: `--export-if-defined` lets a barrier-less binary link, and the loaders' `typeof` guard lets it load and run. The spec also fails when a lane that is supposed to have built a WASI artifact produced none, since silently skipping is the same failure mode. It runs in `test-node-wasi` (which runs the whole suite against the wasm32-wasip1-threads artifact) and is added to the explicit spec list of `test-custom-async-runtime-wasi`. `examples/custom-async-runtime` covers the behavior. Its threadless loader is the only one a test controls end to end, so it now exposes the prepare hook and whether the export exists at all, and `test.mjs` drives the barrier by hand: everything it asserts happens strictly before `context.destroy()`. It checks that the registered backend's `shutdown` ran during the barrier (`shutdownCalls` incremented), that the environment is still live afterwards, and that a task still in flight had its promise rejected rather than stranded. That replaces the placeholder which only checked that disposal did not trap, and whose comment described this hook as missing. That lane was never wired into CI, so the workflow now runs it. It is placed last in the job because it rebuilds `custom_async_runtime.wasm32-wasip1.wasm` in place. Before this branch's barrier commit, with the export assertion bypassed so the deeper assertions are reached: AssertionError: the barrier must shut the backend down before the environment is destroyed 2 !== 3 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): retain the addon image when a custom async runtime is registered `retain_current_module_for_unload_safety` had three call sites, and the two that pin proactively are both behind `cfg(feature = "tokio_rt")`: `create_runtime` and `create_custom_tokio_runtime`. The third only fires after a threadsafe-function release has already failed. Nothing on the `async-runtime` path pinned anything, so an addon that registers a custom `AsyncRuntime` backend and does not enable `tokio_rt` got no image pinning at all — its backend threads could outlive the environment and fault after Node unloaded the addon, which is the exact crash the function was added for. Pin at publication, inside `try_register`. A published backend is reachable for the rest of the process: `backend` is a `OnceLock` that is never cleared, and its trait object's vtable — plus every thread, waker, and destructor the backend creates in `start` — lives in the addon's image. `shutdown` at a later environment teardown would call through that vtable after the image could already have been unmapped. Publication is also the earliest safe point: it happens exactly once, and it happens before `start` can run, so nothing the backend creates predates the pin. `try_register` is the single publication point that both `register_async_runtime` and `try_register_async_runtime` route through, so there is no entry point left that can forget. Rejected registrations do not pin: they are retired through `AsyncRuntime::shutdown` and dropped without ever becoming reachable. Like the existing call sites this is `cfg(not(target_family = "wasm"))` — loader pinning has no meaning for a wasm instance, whose code and environment share one lifetime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): settle deferreds with the retained rejection value A promise may reject, and `AsyncGenerator.throw()` may be called, with any value. `Error::from_unknown_without_coercion` retains that value, but neither settle path handed it back: * with `deferred_trace`, `DeferredTrace::into_rejected` only reused the retained value when a `message` property was found on it, so a string, a number or a plain object was replaced by the synthetic trace `Error`. For `null` and `undefined` it was worse than a replacement: `napi_has_named_property` cannot convert them to an object, and the `TypeError` it left pending made the following `set_named_property` fail, so the promise never settled at all. * without `deferred_trace`, `JsError::into_value` gates reuse on `napi_is_error`, which rejects every non-`Error` value the same way. Both now go through the retained value directly — `ToNapiValue for Error` in the default path — and only fall back to a synthesized error when there is nothing retained to hand back (an error created in Rust, or converted off its owning thread). Verified on native in both feature configurations for string, number, null, undefined, boolean, bigint, symbol, plain object, array, function and `Error`: every one now settles JavaScript with the identical value. The tests assert identity rather than the resulting message, which a freshly synthesized `Error` carrying the same message would also satisfy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * docs(napi): correct what from_unknown_without_coercion guarantees The doc claimed `Error::reason` is filled "only from data that can be read without running JavaScript", and the test fixture encoded that by naming its thrown message "accessor should not run". It does run: reading `message` uses `napi_get_named_property`, an ordinary `[[Get]]`, and Node-API exposes no descriptor read (there is no `napi_get_own_property_descriptor` through Node-API 10, and `napi_get_all_property_names` reports attributes but not whether a property is data or accessor), so an accessor cannot be detected without invoking it. A probe counting invocations confirms the getter fires exactly once. State the real, narrower guarantee: no *coercion* is performed — `toString` and `Symbol.toPrimitive` are never reached, unlike `From<Unknown>`, which calls `napi_coerce_to_string` on every value — and a throwing accessor is swallowed with its exception cleared so it cannot poison the environment, but an accessor with side effects does have them and one that blocks does block. Also record that identity depends on the conversion used: `ToNapiValue` (the promise and async-generator settlement paths) hands the value back untouched, while `JsError::into_value` on the synchronous throw path still gates reuse on `napi_is_error`. The fixture now asserts the observable truth — the accessor is invoked once and its value is used, and a throwing one is contained — instead of a guarantee the implementation does not make. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): capture JS errors without ever running a getter Four defects in the non-coercing capture path, all measured. 1. `Error::reason` claimed to be built "only from data readable without running JavaScript" and was not: `message` was read with `napi_get_named_property`, an ordinary `[[Get]]`, so a `get message()` accessor ran. A fixture whose getter increments a counter reported 1 invocation. Node-API has no descriptor read, but `napi_get_global` and `napi_call_function` do exist, so the lookup now goes through `Reflect.getOwnPropertyDescriptor`, walking the prototype chain with `napi_get_prototype`. A data descriptor contributes its `value`; an accessor descriptor stops the walk and is never invoked. The counter now reports 0, on the instance and on a subclass prototype alike, and a `message` data property up the chain is still found so `new Error()` keeps reporting `''` from `Error.prototype.message`. Two caveats are documented rather than papered over: a `Proxy` still runs its `getOwnPropertyDescriptor`/`getPrototypeOf` traps, and `Reflect` is read off the global on every call and is patchable. Caching it is deliberately rejected -- it would only move *when* a patched `Reflect` is observed, and would need a per-env `napi_ref` kept alive across worker teardown. 2. `Error::cause` was hardcoded to `None`, a regression against main on the fallback path -- off-thread or a foreign env, exactly where the retained value is gone and `JsError::into_value` has to rebuild the error from `reason`/`cause`. It is now captured with the same descriptor discipline, bounded to eight links so a cyclic chain terminates. `From<Unknown>` segfaults on that input today. Promise.reject(new TypeError('m', { cause: RangeError })) main reason="TypeError: the message" cause=Some("RangeError: the cause") before reason="the message" cause=None after reason="the message" cause=Some("the cause") 3. A throw out of a ThreadsafeFunction callback did not round-trip, while a promise rejection did. That path still used a bare `napi_create_reference` (rejects every non-object below Node-API 10) and built `reason` from `napi_coerce_to_string` plus a `[[Get]]` of `stack` -- which runs user code while unwinding, and *throws* on a symbol, leaving a second exception pending in an env Node had already been told was clean. Of the 11 value types, 7 failed to round-trip and 3 left `Uncaught Node-API callback exception` warnings behind. All 11 are now identical and the warnings are gone. The cost, stated plainly: `reason` no longer carries the stack trace, because `stack` is an own accessor on every V8 error. JavaScript is unaffected -- it now receives the original exception object, stack and all. 4. `DeferredTrace::into_rejected` returned through `?` on a failing `set_named_property` and skipped `napi_delete_reference`, leaking the trace reference. The release is now unconditional, with the more informative failure still winning. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): drain queued settlements before destroying a wasm env `napi_prepare_wasm_env_cleanup` shuts the registered `AsyncRuntime` backend down while the environment can still call into JavaScript, and the cancelled tasks do reject their `JsDeferred`s from there. But `napi_call_threadsafe_function` only *appends* to the threadsafe-function queue. On wasm that queue is dispatched by the host, and @emnapi/core 2.0.0-alpha.3 dispatches a call made on the JavaScript thread from a macrotask, two coalescing `setImmediate` turns later. The generated loaders ran the barrier and `Context.destroy()` as two adjacent synchronous statements: __prepareWasmEnvCleanup() const result = __emnapiContext.destroy() `destroy()` runs the threadsafe function's cleanup hook, which drains the queue with a null env and discards it. So every promise the barrier had just cancelled was stranded — the exact outcome the barrier exists to prevent. Measured: 0 of 64 microtask checkpoints ever deliver the settlement; it takes exactly 2 macrotask turns. Close the gap with an explicit handshake rather than a guessed turn count: - `napi_wasm_env_cleanup_pending` exports a count of settlements that are in the queue but not yet dispatched, maintained by `JsDeferred` around `napi_call_threadsafe_function` / `napi_resolve_deferred`. It counts only queued settles, never merely-pending promises, so a long-running task that shutdown did not cancel cannot make a loader spin. - The loaders yield real macrotask turns until that count reads zero (bounded), then destroy. When nothing is queued the drain returns synchronously, so an idle disposal is unchanged. - The Node `process.on('exit')` fallback cannot yield, so it keeps the synchronous teardown and does not get this guarantee. The behavioral test drove the barrier by hand and awaited the rejection before `dispose()`, which is what hid this. It now also exercises the ordinary `dispose()` path end to end in its own process, and the CLI template suite asserts every emitted loader participates in the handshake. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): make the error-object APIs return error objects `Error::from_unknown_without_coercion` retains whatever JavaScript handed over — including primitives — so a rejection settles with itself. Two APIs inherited that behaviour and must not have it: * `Env::create_error` returned the retained value verbatim, so `create_error(Error::from_unknown_without_coercion(42))` handed back the number `42` from a function documented to create a JavaScript error object. Nothing threw; every later object operation on the result silently no-oped. * `ToNapiValue` for `JsError`/`JsTypeError`/`JsRangeError` delegated to `ToNapiValue for Error`, bypassing the `napi_is_error` gate that `JsError::into_value` already had. All eleven value types round-tripped through them unchanged. Both now gate reuse on `napi_is_error` through a shared `is_js_error` helper and synthesize an error otherwise. Delegating to `Error` also lost the subclass, so a `JsTypeError` with nothing reusable came back as a plain `Error`; routing through `into_value` fixes that too. The settlement paths are untouched: promise rejection, `AsyncGenerator.throw()` and a throw out of a ThreadsafeFunction callback convert through `ToNapiValue for Error` and still reproduce the value identically, in both `deferred_trace` configurations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(cli): drain queued settlements before rolling back a failed wasm init The generated WASI loaders drained the threadsafe-function queue before destroying the environment on the disposal path, but not on the initialization-failure path: the eager loaders' `__rollbackWasiInitialization` called `__destroyEmnapiContext()` directly, and the deferred loader's `__createInstance` catch called `__destroyManagedOwnedContext()` without `__prepareForDisposal()`. That was assumed safe because nothing can be in flight before instantiation finishes. It can. Registration runs with a live environment — emnapi calls `napi_register_wasm_v1` from `napiModule.init()`, after `beforeInit` — so an addon's module-init hook can start a task and *then* return an error, which is exactly how instantiation fails. The barrier cancels that task and only queues its rejection; destroying in the same turn drains the queue with a null env and discards it, stranding a promise that already escaped into JavaScript. Both paths now run the barrier, drain, and only then destroy. Both stay synchronous when nothing is queued, which covers every failure before `beforeInit`: no instance, so no barrier and nothing to drain. `process.on('exit')` still cannot yield and is still documented as stranding. `examples/custom-async-runtime` grows a wasm-only, globally gated registration-failure hook and drives both generated loaders through it; the CLI template spec asserts the shape for the flavors no test instantiates. Without the fix the behavioural test reports the promise `PENDING` and the shape assertion fails for all four loader flavors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * test(examples): assert generator concurrency relatively, not on a clock `async generators should run concurrently` asserted a fixed 300ms budget on two DelayedCounter(5, 10) drained through Promise.all. It failed the QEMU armv7 job at 362ms. That budget, not the async-generator path, is what broke. Across 18 armv7 runs the test's elapsed tracks the median of its sibling async-generator tests in the same file at a ratio of 1.01 +/- 0.03 -- including the failing run (362ms against a 360ms sibling median). Those siblings range from 127ms to 360ms run to run, a 2.8x spread, while the *synchronous* generator tests in the same file stay flat (189ms in the failing run against 184-243ms on main). The emulated target's per-yield wake-up cost, not the sleeping, dominates there and it moves with whatever else the runner is doing. Main itself has come within 12% of the cliff at 264ms. Nothing on that path changed. `Env::create_error` and the `JsError` conversions are not reached at all by a resolving generator: instrumenting `is_js_error` and `Env::create_error` counts zero calls across ten yields and two completions, while a direct call to `createErrorFromRetainedValue` hits both. Per-yield cost measured with crates/napi at 6a5e4401 versus 344128f1 is 1278.12us versus 1276.66us. So measure the serial baseline in the same process moments apart and assert on what concurrency actually buys. Two claims, neither on the clock's absolute scale: - the counters interleave -- serialised, every value of the first lands before the first value of the second; - the sleeps overlap rather than merely interleaving -- concurrency saves at least half of one counter's sleep budget. The saving is what a fixed budget only approximates. Simulating the emulated target by injecting 0-200ms of JS-thread work per yield, concurrent elapsed climbs 77ms -> 1025ms and blows through any fixed budget, while the saving stays between 70ms and 134ms against a 25ms threshold. Both assertions were confirmed to fire, each against the failure it exists for, by breaking DelayedCounter in Rust rather than editing the test: a global turnstile serialises the counters and trips the interleaving check (`aaaaabbbbb`); a FIFO ticket lock keeps them interleaved but stops their sleeps overlapping and trips the saving check (serial 153ms, concurrent 145ms, saved 8ms). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * test(examples): stop asserting timer overlap on the WASI lane `async generators should run concurrently` failed `Test node wasi target` with "serial took 133ms and concurrent took 122ms (saved 11ms)" against a 25ms threshold, while its clock-free interleaving assertion passed. That is the signature of the FIFO ticket lock used to prove the test has teeth: values alternate, sleeps do not overlap. It is not this branch. Building fb91bcd8 for wasm32-wasip1-threads and running the same measurement gives serial 153ms / concurrent 153ms, saved median 1ms over 15 runs; this branch gives serial 155ms / concurrent 151ms, saved median 5ms over 25 runs. Every tokio_runtime.rs change here is cfg(not(target_family = "wasm")). The old fixed 300ms budget passed 122ms trivially, which is all that was hiding it. The cause is below napi-rs. Tokio parks a worker on its next timer deadline through runtime::park::Inner::park_timeout, which is #[cfg(all(target_family = "wasm", not(target_feature = "atomics")))] { std::thread::sleep(dur) } // "Wasm without atomics doesn't have threads" and `target_feature = "atomics"` is never set on wasm32-wasip1-threads -- it is an *unstable* target feature, and rustc does not surface unstable target features to cfg. `rustc --print cfg --target wasm32-wasip1-threads` lists bulk-memory and mutable-globals but not atomics, and a crate compiled with an explicit -Ctarget-feature=+atomics still reports the cfg as false (rustc warns "unstable feature specified for -Ctarget-feature"). So on the one wasm target that does have threads, tokio's timer park is an uninterruptible sleep, even though std's own Condvar::wait_timeout and thread::park_timeout are fully interruptible there (measured: notify at 20ms interrupts a 200ms wait after 22.2ms). available_parallelism() is 1 under Node's WASI, so the single worker owns the time driver, and a task injected from the JavaScript thread is not polled until the sleep already in flight runs out. Measured first-poll latency for a task submitted at offset O into a holder sleeping D: D=200/O=30 -> 152, 133, 116ms; D=400/O=60 -> 314, 286, 323ms -- it tracks the holder's remaining time, not the injection. With no timer pending the same injection is polled in 0.11ms, because that path parks on the condvar instead. Nothing on napi's own path is at fault. Eight 50ms sleeps joined inside one task finish in 51ms and eight tokio::spawn'd from within the runtime in 53ms; only the cross-thread wake-up is lost. Patching that single cfg in a local tokio 1.53.1 checkout takes first-poll latency from ~165ms to 0.2ms and the same DelayedCounter measurement from serial 155 / concurrent 151 to serial 184 / concurrent 98 (saved median 91ms). Forcing worker_threads(4) in create_runtime also recovers it (saved 51-70ms), since the extra workers park on the multi-thread scheduler's own condvar, which has no wasm fallback -- but that spawns four Node workers per addon and is a runtime behavior change, not a test fix. So keep the clock-free interleaving assertion everywhere and apply the saving assertion only where the executor can really overlap timers. Both still fire, verified by breaking DelayedCounter in Rust rather than editing the test: a turnstile that serialises whole counters trips interleaving on native *and* on WASI (`aaaaabbbbb`), and a fair FIFO async mutex held across the sleep trips the saving check on native (serial 154ms, concurrent 153ms, saved 1ms). Lowering the threshold instead would ship a concurrency assertion that accepts fully serialised execution. wasm32-wasip1 (threadless) is unaffected: with the built-in tokio backend it rejects async work outright ("Built-in Tokio async tasks require a threaded WASI target"), so DelayedCounter cannot run there at all, and no lane runs this spec against it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): narrow the error-object gate to Env::create_error `7b289161` gated `Env::create_error` *and* the `ToNapiValue` impls for `JsError`/`JsTypeError`/`JsRangeError` on `napi_is_error`. The gate belongs on the first only. `Env::create_error` *constructs* an error object and is documented to return one, so a retained `42` coming back verbatim breaks its own contract and silently no-ops every object operation the caller then performs. It keeps the gate. The `ToNapiValue` impls are *conversions*, on exactly the same footing as `ToNapiValue for Error`. An addon returning `JsTypeError::from(Error::from_unknown_without_coercion(value))` is relaying a value JavaScript chose, and JavaScript must get it back as itself. Routing them through `into_value` replaced every non-`Error` with a synthesized one; building rolldown against this branch turned that into 4 native and 4 `wasm32-wasip1` failures. Restoring the pass-through is not a plain revert, because the same commit also fixed a real second bug: delegating to `<Error as ToNapiValue>` falls back to `JsError::into_value`, so a `JsTypeError` with nothing to reuse arrived in JavaScript as a plain `Error`. Both halves now hold — a retained value comes back verbatim, and a synthesized one uses `$kind`, keeping the subclass the wrapper names. `is_js_error` stays, now used by `create_error` and `JsError::into_value` (the synchronous throw path) alone. The settlement paths — promise rejection, `AsyncGenerator.throw()`, a throw out of a ThreadsafeFunction callback — are untouched and still reproduce all eleven value types identically, in both `deferred_trace` configurations. The one test that asserted `t.not(result, value)` across four APIs is re-aimed, not deleted: `Env::create_error` keeps the full ten-type synthesis matrix, the wrappers get a matching ten-type *verbatim* matrix, and a third test covers the subclass on the synthesizing path. Each of the two new tests fails against exactly one of the two prior behaviours. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): deliver the wasm cleanup barrier's settlements synchronously `napi_prepare_wasm_env_cleanup` cancels the in-flight tasks and their rejections reach `napi_call_threadsafe_function` — which only *appends* to a queue `@emnapi/core` dispatches from a macrotask, two coalescing turns later. Closing that gap needs the host to yield event-loop turns, and `6a5e4401` gave the generated loaders `napi_wasm_env_cleanup_pending` to yield against. A host that cannot yield has no such option. `process.on('exit')` cannot, and neither can a caller reaching for `emnapiContext.destroy()` directly — rolldown does exactly that, and `destroy()` runs the threadsafe function's cleanup hook, which drains the queue with a null env and discards every settle the barrier had just produced. So the barrier now delivers instead of queueing. While it runs, a `JsDeferred` settled on the thread that owns it settles its promise straight through, with the environment still fully alive: - `WasmEnvCleanupBarrier` marks the current thread for the duration of the backend shutdown, and `call_tsfn` consults it together with the deferred's owner thread. - The settle body moves into `settle_deferred`, reached identically from the threadsafe-function dispatch and from here. - The dispatch runs the callback inside a handle scope; this export is a bare wasm function JavaScript enters directly, with no napi machinery around it, so the delivery opens its own. Without it the first `napi_create_string_utf8` fails on a null scope. If the scope cannot be opened it falls back to the queue — worse, never wrong. The two mechanisms compose rather than overlap: a delivered settle never enters the threadsafe-function queue, so it never enters `PENDING_DEFERRED_SETTLES` either. After the barrier returns, `napi_wasm_env_cleanup_pending` reads 0 for everything it delivered, and the loaders' drain returns on its first check. Nothing blocks. The main thread is never parked, which is what keeps this legal in a browser, where blocking it is not allowed at all. What remains uncovered is a settle produced on *another* thread, which can only reach the promise through the queue — that is what the counter still exists for, and it is the same documented limitation `process.on('exit')` already had. Native is untouched: the whole path is `cfg(target_family = "wasm")`. The threadless WASI lane grows the case a drain can never satisfy: barrier and `Context.destroy()` in one synchronous turn, nothing awaited in between. It asserts both halves — the in-flight promise rejects, and the queue was left empty. Without the barrier guard it reports `SYNC_DISPOSE_QUEUED 1` and `SYNC_DISPOSE_OUTCOME PENDING`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): latch the wasm runtime against restart during env disposal `napi_prepare_wasm_env_cleanup` shuts the async runtime down while the environment is still fully alive, and the generated loaders then yield real event-loop turns so the threadsafe-function queue can be dispatched before `Context.destroy()`. Every one of those turns is a turn in which arbitrary JavaScript runs — including the rejection handlers of the very promises the barrier just cancelled. An addon export called from one of those handlers walked straight back into the dispatch path, where `ensure_started` found the `Idle` phase the teardown left behind and started the backend again, after its `shutdown` had already reported quiescence. Nothing in the threadsafe-function queue represents that restarted work, so `napi_wasm_env_cleanup_pending` read zero, the drain stopped waiting, and the environment was destroyed with the work still live — its promise stranded, which is the exact failure the barrier exists to prevent. On the built-in Tokio path the same call instead reached the `RT` slot the teardown drained and `.expect`ed on it, trapping the instance. The barrier now latches the runtime for the remainder of disposal: - `start_async_runtime` and `ensure_started` are no-ops, so nothing restarts. - `execute_future_impl` rejects with `Status::Cancelled` instead of dispatching, so a call made during disposal gets a settled promise rather than a stranded one. The rejection goes through the ordinary queued settle path, so the drain waits for it and it reaches JavaScript before `destroy()`. - `within_runtime_if_available` runs the closure without a runtime context rather than aborting on the drained slot. The latch lives in the wasm instance's linear memory, so it is scoped to that environment: a later environment gets a fresh instance and a fresh, unlatched static. It is released on module registration, the one event that means a live environment needs this addon again — and one emnapi only runs on the main thread, so a `wasm32-wasip1-threads` worker sharing this memory cannot release it mid-disposal. Every check is `cfg(target_family = "wasm")`; native codegen is unchanged. The regression guard drives the generated threadless loader end to end and fails without the latch (`startCalls=2`, reentrant promise `PENDING`). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): latch the Tokio compatibility helpers during wasm env disposal `napi_prepare_wasm_env_cleanup` latches the runtime against restart for the rest of the environment's disposal, but the latch only covered the dispatch path and `within_runtime_if_available`. `spawn`, `block_on` and `spawn_blocking` reach `RT` directly, and a synchronous export that uses one is exactly what a JavaScript callback running between the drain's macrotask turns can call. In a combined `async-runtime` + `tokio_rt` build that is a hole, not a rough edge. A custom backend owns the runtime lifecycle, so `RT`'s `LazyLock` is typically never forced and the barrier's `shutdown_async_runtime` has nothing to drain — there is no empty slot to fall over on. Reading `RT` after the barrier *constructs a brand-new Tokio runtime* and takes the work. Nothing in the threadsafe-function queue represents it, so `napi_wasm_env_cleanup_pending` reads zero, the drain stops waiting, and `Context.destroy()` arrives with the work still live. Measured on a wasm32-wasip1 build of the custom-async-runtime example, called from the barrier's own rejection handler: napi::spawn -> returned a JoinHandle napi::block_on -> ran `async { 2 }` to completion and returned 2 with `startCalls=1 shutdownCalls=1` throughout: the backend really was down. These three cannot degrade to a rejection the way an async export does. They hand back a `JoinHandle` or the future's own output, and a `JoinHandle` cannot be built without a runtime to build it on. Accepting the work is the bug; dropping it would silently lose work the caller holds a handle to. So they panic, with a message that names the helper and says the environment is being disposed. That is not a new failure mode. Every other configuration already panics here: once the barrier's shutdown has taken the runtime out of `RT`, the existing `.expect` fires on the empty slot — the documented post-`shutdown_async_runtime` behaviour of these helpers. This makes the one configuration that silently deviated behave like the rest, and replaces "Access tokio runtime failed in spawn" with a diagnostic that explains itself. The check precedes the first `RT` access, so the `LazyLock` is never forced past the barrier. What is new is the reachability. Before this branch the wasm teardown ran `shutdown_async_runtime` from `thread_cleanup`, inside `Context.destroy()`, where no JavaScript can run afterwards. The barrier plus the loader's drain deliberately open a window in which arbitrary JavaScript runs after the runtime is down, and that window is what a reentrant synchronous export walks into. Every check is `cfg(target_family = "wasm")`; native codegen is unchanged. In a pure `async-runtime` build these helpers do not exist at all (`cannot find function `spawn` in module `napi::bindgen_prelude``), so there is nothing to latch there. The regression guard drives a combined wasm32-wasip1 build through the generated eager loader, for all three helpers, both with `RT` never constructed and with it constructed and drained. Without the latch it fails on `TOKIO_HELPER_OUTCOME RETURNED: 1` — `napi::spawn` accepting work after the barrier. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(cli): mark the wasm cleanup drain complete only when it finishes Both generated loaders set their "already drained" flag before scheduling anything. The drain is the one part of disposal that can fail on its own — it yields real event-loop turns, and scheduling a macrotask is a host call, so a host-provided or patched `setImmediate` that throws makes the very first turn reject. The loaders keep disposal retryable after that: `__wasiDisposePromise` is cleared, `dispose()` can be called again. But the flag stayed set, so the retry skipped the drain entirely and destroyed the context with the barrier's settlements still in the threadsafe-function queue, where `Context.destroy()`'s cleanup hook discards them with a null env. The promises the barrier exists to settle then hang forever, and nothing is left that could ever settle them. Reproduced against the generated wasm32-wasip1 loaders: POISONED_DRAIN_FIRST REJECTED: host setImmediate is broken POISONED_DRAIN_SECOND RESOLVED POISONED_DRAIN_QUEUED PENDING <- stranded The flag is now set only once a wait has actually finished, and an in-flight drain is memoised so a concurrent caller joins it instead of starting a second one — the same shape `__destroyEmnapiContext` already uses. Reaching the turn limit still counts as finished: the wait is bounded by design, so repeating it would only spend another 128 turns on the same queue. The review flagged the eager loader. The deferred loader's `__prepareForDisposal` has the same ordering and the same consequence, so both are fixed and both are guarded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(cli): keep the wasm rollback's context when its drain cannot reach the queue Initialization rollback ran the pre-teardown barrier, drained the settlements it queued, and then destroyed the environment. The drain can fail on its own — it yields real event-loop turns, and scheduling a macrotask is a host call that a host-provided or patched `setImmediate` can make throw — and both loaders recorded that failure and destroyed anyway. `Context.destroy()` runs the threadsafe function's cleanup hook, which drains the queue with a null env and discards it, so a promise that had already escaped into JavaScript hung forever with nothing left that could ever settle it. `dispose()` already answers this correctly: a rejected drain never reaches `__continueWasiDisposal`, so the context survives and the disposal stays retryable. The rollback was the odd one out, and inconsistently so — it kept its registry record for retry while destroying the very thing the retry was for. Both loaders now stop short of destroying when the barrier or the drain did not finish, and hand the context to the machinery that can still reclaim it: the process-wide registry replay plus the `process.on('exit')` teardown for the eager CJS loader, the already-registered managed destroyer for the deferred one. The ESM browser loader has no retry — a module that throws while evaluating is permanently errored — so there the context stays until the realm goes away. That is the deliberate trade, recorded in the template: a hung promise is a silent liveness bug with no upper bound, while `Context.destroy()` does not free the wasm instance or its Memory anyway, so what is retained is the emnapi context's bookkeeping and its un-run cleanup hooks. The example's registration-failure hook now also spawns a task that completes inline. Its settle goes through the threadsafe-function queue instead of being delivered by the barrier, which is what leaves `napi_wasm_env_cleanup_pending` above zero when the rollback starts — without it the rollback's drain has nothing to wait for, never schedules a macrotask, and the failure modes of that scheduling are unreachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): retain the addon image when a threadsafe function is created `Drop for ThreadsafeFunctionHandle` retained the image only after `napi_release_threadsafe_function` failed, and that whole branch sits inside `if !aborted`. Environment teardown finalizes the threadsafe function first, and `thread_finalize_cb` sets `aborted = true`, so a handle whose last `Arc` is dropped by a foreign thread after its worker exited takes the no-op branch and never retains anything. The destructor that thread runs — `Arc` drop glue, the `RwLock` read, `napi_release_threadsafe_function` — is code in the addon image, so on Windows a `FreeLibrary` that unmapped the image leaves it executing unmapped memory. Retain at creation instead: `create_raw` is the single non-generic funnel for every `ThreadsafeFunction`, and it runs on the environment's own thread while the image is unquestionably still owned. `Env::create_deferred` needs only `napi4`, and a `JsDeferred` is `Send` precisely so a foreign thread can settle it, so `js_deferred_new_raw` has the same window in a build where no runtime pins the image; retain there too. The release-failure call stays as a backstop for a handle built through the public `ThreadsafeFunctionHandle::new`, which does not go through `create_raw`. Retention is idempotent (`std::sync::Once`), so the duplicate call is one atomic load. Measured on macOS: in a worker-only load, `thread_finalize_cb` runs on the worker's thread and the last handle drops on a different thread afterwards with `aborted = true`, confirming the branch is skipped; `dlopen(RTLD_NOLOAD)` succeeds and the second retention request is a no-op. The Windows unmap itself is argued from `FreeLibrary` semantics, not reproduced. Guarded by a request counter exposed for tests: creating a `ThreadsafeFunction` must increment it. Verified discriminating — removing the `create_raw` call takes the delta from 1 to 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): provide module_retention_requests under the noop feature The introspection counter was gated cfg(not(feature = "noop")), but examples/napi calls it whenever the target is not wasm, and CI checks that crate with -F noop. Stub it to 0 there: under noop nothing registers, so nothing ever requests a pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): pin the addon image in ThreadsafeFunctionHandle::new create_raw pinned at creation, but a handle wrapped around a raw threadsafe function through the still-public ThreadsafeFunctionHandle::new bypassed it, and the Drop-path retention is skipped in exactly the worker-teardown case (thread_finalize_cb marks the handle aborted, so Drop takes its no-op branch). Every construction funnels through new() -- create_raw included, via null() -- so the pin moves there, the create_raw call is dropped as redundant, and the dead Drop-branch call is removed with its comment updated. Mutation-tested: removing the pin from new() fails "ThreadsafeFunction creation pins the addon image"; restoring passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): pin the addon image when a JS value is captured into an Error An Error is Send, and a JS-derived one owns an Arc<ErrorRef> whose last drop can happen on a detached thread after the environment that created it - and the worker that hosted that environment - are gone. Node then unloads a worker-only addon, and ErrorRef::drop is code in that image: the thread crashes on entry, before the destructor could ever observe the aborted custom-GC handle. This is the same unload hazard the ThreadsafeFunction and deferred pins close, reachable through Error::from_unknown_without_coercion (and From<Unknown>) instead of a threadsafe function. Request module retention in ErrorRef::new, at construction, on the environment's own thread. Construction is the only point that covers every path for the same reason it is for ThreadsafeFunctionHandle::new: environment teardown marks the custom-GC handle aborted and the drop path then no-ops, so a pin placed anywhere on the drop path is skipped in exactly the worker-teardown case it exists for. Placing it in ErrorRef::new covers both construction sites - the non-coercing capture's private holder and the From<Unknown> reference - at once. The pin is taken at most once per process; repeats are one relaxed atomic increment. wasm has no loader and no image to unmap, so the pin is compiled out there, as everywhere else. Guarded next to the ThreadsafeFunction retention test with the same module_retention_requests() counter, over a synchronous capture window. Mutation-tested: with the pin removed the new test fails (counter stays flat); with it restored the test passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): resolve Reflect.getOwnPropertyDescriptor once per env at registration The non-coercing capture reads message/cause through Reflect.getOwnPropertyDescriptor precisely so that no user code runs while an error is unwinding - but it resolved Reflect off the global object on every capture, and that read is an ordinary [[Get]]. globalThis.Reflect is configurable, so user code can redefine it as an accessor, and the accessor then runs mid-capture: arbitrary user code, free to reenter the addon or hang, with side effects that clearing a pending exception cannot undo. Reproduced empirically: an accessor Reflect fired four times during a single Error::from_unknown_without_coercion of an error with one cause link. Resolve the Reflect / getOwnPropertyDescriptor pair once per env, at module registration, and keep it as own data properties of a private holder behind a single napi_ref. Capture reads the pair back off the holder - own data properties of an ordinary object that never escaped to JavaScript, so no prototype walk and no user code - and never touches the global again. This inverts the earlier per-call rationale deliberately. That design's concern was staleness ("caching only changes when a patch is observed"); the threat model that matters is reentrancy during capture, and the load-time cache ends it outright - the one unavoidable [[Get]] moves to require() of the addon, a defined moment, and a post-load patch can no longer inject code into the capture path at all. The trade is that capture ignores post-load patches of Reflect instead of observing them late; a data-property Reflect (the universal case) keeps full behaviour, and an env with no usable cache - Reflect missing or unusable at registration, or an env that never registered - degrades to an empty reason/cause, never to a [[Get]] on message. The reviewer's alternative (omit reason/cause whenever the intrinsic cannot be obtained without a [[Get]]) is circular on its own: descriptor-reading the global's Reflect property already requires getOwnPropertyDescriptor. The per-thread slot leans on the same one-env-per-OS-thread invariant as CURRENT_CUSTOM_GC_HANDLE. An env cleanup hook (napi3+, native only - wasm-ld rejects the Rust import of napi_add_env_cleanup_hook against emnapi's archive, and a wasm slot dies with its instance) deletes the holder reference and clears the slot at env teardown, so an embedder reusing the thread can never match a stale entry through a recycled env pointer. Re-registration of the same env (unload.spec.js) replaces the cache and releases the superseded reference without re-adding the hook, which Node would abort on. Guarded by three synchronous-window tests over a new describeCapturedValue example export: an accessor Reflect must not fire during capture (counter stays 0) while identity, reason and cause keep full fidelity; a deleted and a clobbered post-load Reflect keep full fidelity too. Mutation-tested: with the per-call global lookup restored, all three fail (the accessor fires, and the reason collapses to "JavaScript Error"). The 11-type identity matrix passes in both deferred_trace configurations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(cli): reject the wasm cleanup drain when settlements outlast the turn bound Both generated drains waited up to 128 macrotask turns for napi_wasm_env_cleanup_pending to reach zero and then counted reaching the bound as finished. With the counter still nonzero at the limit that outcome is indistinguishable from the stranding the drain exists to prevent: disposal marks the drain complete, destroys the emnapi context, and Context.destroy()'s cleanup hook discards the still-queued settlement with a null env - the promise it was for hangs forever with nothing left that could settle it. A worker-originated settlement whose delivery is delayed past the bound is enough. The wait stays bounded, but a still-nonzero counter at the limit now rejects with ERR_NAPI_WASI_CLEANUP_PENDING instead of setting the drained flag. That routes the timeout into the machinery the failed-drain paths already have: dispose() declines to destroy and stays retryable (each retry is one more bounded wait), the eager rollback retains the context for the registry replay and process-exit teardown, and the deferred instantiation-failure path registers the managed beforeExit destroyer. A counter that is somehow stuck nonzero therefore costs each dispose() attempt a bounded wait and a coded, retryable rejection - never a stranded promise and never a permanently wedged process, since the exit-time teardown still reclaims the context. The blind drain (no pending export) is unchanged: with nothing to consult, the bound is the contract. Zero at the limit still succeeds. A genuinely late delivery cannot be produced deterministically - the drain itself yields the very turns @emnapi/core dispatches on - so the new harness forces the counter instead: it wraps WebAssembly.Instance.prototype.exports (a plain per-instance copy; the exports' non-configurable properties bar a Proxy) and clamps only napi_wasm_env_cleanup_pending to >= 1 while stuck. Driven against both generated loaders: the first dispose() must reject with the coded error while the real queue drains unharmed (the in-flight task's promise settles), and the retry after the clamp lifts must succeed. Mutation-tested: with the throw reverted to the old "counts as finished" return, the first dispose() resolves and the harness assertions fail. The regenerated loaders also pick up the describeCapturedValue export added by the previous commit. The hand-written threadless-wasi-loader.cjs harness keeps its minimal bound-counts-as-done drain on purpose: it exists to drive the barrier from a host's perspective, not to model the retry contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): reject async tasks with the retained value on the completion path The napi_create_async_work completion path rejected its promise through JsError::into_value, whose napi_is_error gate exists for the error-object constructors (napi_throw, napi_fatal_exception, Env::create_error). On a settlement path that gate is wrong: an Error that captured an arbitrary JS value with Error::from_unknown_without_coercion - stored in an AsyncTask on the JS thread, returned from Task::compute on the libuv thread, and handed back by the default Task::reject - settled the promise with a synthesized Error instead of the captured value. 10 of the 11 retained value types came back wrong; only a genuine Error survived the gate. Route the rejection through ToNapiValue for Error, the same conversion the deferred settlement path uses: the retained value comes back verbatim when the completion callback runs on the owning env and thread - which is where napi always calls it - and the owner gates in referenced_value make every other case (an error built on the libuv thread, a capture from a foreign env or thread) fall back to synthesis instead of dereferencing a foreign reference. This is the only napi_create_async_work completion path in the crate; all three async-work entry points (AsyncTask, Env::spawn, AbortSignal) settle promises through it, and none of them serves a non-promise callback variant. The new example export carries the identity matrix for this path: all 11 value types must reject as themselves, mutation-tested against the old into_value expression (10 failures) in both deferred_trace configurations and on the wasm32-wasip1-threads lane. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda * fix(napi): vet the cached getOwnPropertyDescriptor with a registration probe The pair cached at module registration is whatever the [[Get]] of globalThis.Reflect found, so code that ran before the addon loaded can have planted an arbitrary callable there, and capture would invoke it mid-unwind on every Error::from_unknown_without_coercion. Provenance cannot be established through Node-API, full stop: the complete surface (js_native_api.h + node_api.h, through Node-API 10 and the experimental set) has no descriptor-read primitive, no non-[[Get]] access to the global, no fresh-realm primitive (napi_run_script evaluates in the same patched realm), no proxy detection, and no pristine reference to napi_strict_equals against - obtaining one is the problem itself. A Function.prototype.toString check is a call through a patchable callable, circularly. And the boundary is not defensible anyway: a pre-load adversary can just as well wrap require, the loader, or every export of the addon. Omitting reason/cause for everyone would not close that boundary; it would only take fidelity away from every non-adversarial user. What can be enforced is behavior, at a defined moment. Registration now runs the resolved candidate against a freshly created object whose shape JavaScript cannot interpose (napi_create_object + napi_define_properties): a data property must come back as a descriptor carrying the sentinel verbatim, an accessor property must be described - own get, no own value, native getter never invoked - and a missing property must yield undefined. A candidate that misreports any of these is dropped on the spot: the env degrades to an empty reason and absent cause permanently, the impostor is never invoked during capture, and the retained value is unaffected. On a re-registration the pair the first load vetted stays in place. The child-process fixture proves both directions and is mutation-tested: with the probe deleted the impostor goes unconsulted at load, and with its verdict ignored the impostor fires during capture (hits=2) and corrupts the derived reason/cause. The existing accessor-Reflect, deleted-Reflect, and clobbered-Reflect guards, plus both full lanes (native 258, wasm 255), pin the accepting …
1 parent 93e86ce commit 61fae8a

49 files changed

Lines changed: 8186 additions & 288 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/test-release.yaml‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,10 @@ jobs:
861861
test-custom-async-runtime-wasi:
862862
runs-on: ubuntu-latest
863863
name: Test custom async runtime on wasm32-wasip1
864-
timeout-minutes: 10
864+
# The last step builds this addon twice for wasm32-wasip1 — once
865+
# `async-runtime`-only, once combined with `tokio_rt` — and the two feature
866+
# sets share no cargo fingerprints, so the second is a full rebuild.
867+
timeout-minutes: 15
865868
needs: audit
866869
steps:
867870
- uses: actions/checkout@v7
@@ -908,7 +911,8 @@ jobs:
908911
run: |
909912
yarn workspace @examples/napi test \
910913
__tests__/wasi-buffer-public-api.spec.ts \
911-
__tests__/wasi-deferred-before-exit.spec.ts
914+
__tests__/wasi-deferred-before-exit.spec.ts \
915+
__tests__/wasi-env-cleanup-export.spec.ts
912916
env:
913917
NAPI_RS_TEST_THREADLESS_WASI_BUFFER: '1'
914918
- name: Test lifecycle public surface
@@ -926,6 +930,13 @@ jobs:
926930
run: yarn workspace @examples/custom-async-runtime test:workerd
927931
- name: Test examples/napi threadless public surfaces
928932
run: yarn workspace @examples/napi test:wasi-public-surfaces
933+
# Last: this rebuilds custom_async_runtime.wasm32-wasip1.wasm in place. It
934+
# is the only lane that loads the addon through a loader the test controls,
935+
# which is what makes it able to drive `napi_prepare_wasm_env_cleanup` by
936+
# hand and observe that the backend shut down while the environment was
937+
# still alive.
938+
- name: Test threadless WASI environment teardown barrier
939+
run: yarn workspace @examples/custom-async-runtime test:wasi
929940

930941
# Node-only lane: the threadless wasm32-wasip1 artifact built from the real
931942
# `napi-async-runtime` adapter driven by the real `@napi-rs/async-runtime`

‎cli/README.md‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,13 @@ if (dispose) {
5050
The symbol is present only when the loaded binding is WASI. Browser WASI
5151
loaders expose it on their default export. Disposal releases the instance: it
5252
destroys the emnapi context and then terminates the workers owned by that
53-
binding. Pending napi-rs runtime promises are settled or cancelled first only
54-
when the binary exports the `napi_prepare_wasm_env_cleanup` preparation hook
55-
(a planned napi-side follow-up); otherwise they may remain pending forever,
56-
so settle in-flight work before disposing. Concurrent calls share one
53+
binding. Before that, the loader calls the binary's
54+
`napi_prepare_wasm_env_cleanup` preparation hook, which shuts the addon's async
55+
runtime down while the environment can still call into JavaScript. A registered
56+
`AsyncRuntime` backend quiesces there and its cancelled tasks reject their
57+
promises; the built-in Tokio runtime only _starts_ draining, so with it a
58+
promise whose task is still running can still be left pending. Settle in-flight
59+
work before disposing if that matters. Concurrent calls share one
5760
promise, successful disposal is idempotent, and a failed cleanup phase can be
5861
retried by calling the same function again. Do not call addon exports after
5962
disposal completes.

‎cli/src/api/__tests__/__snapshots__/templates.spec.ts.md‎

Lines changed: 768 additions & 9 deletions
Large diffs are not rendered by default.
3.48 KB
Binary file not shown.

‎cli/src/api/__tests__/templates.spec.ts‎

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import ava, { type ExecutionContext } from 'ava'
22
import { parseSync } from 'oxc-parser'
33

44
import { createCjsBinding, createEsmBinding } from '../templates/js-binding.js'
5-
import { createWasiBrowserBinding } from '../templates/load-wasi-template.js'
5+
import {
6+
createWasiBinding,
7+
createWasiBrowserBinding,
8+
createWasiDeferredBrowserBinding,
9+
} from '../templates/load-wasi-template.js'
610
import { createWasiBrowserWorkerBinding } from '../templates/wasi-worker-template.js'
711

812
const test = ava
@@ -200,6 +204,86 @@ for (const { name, code } of cjsBindingCases) {
200204
})
201205
}
202206

207+
// `napi_prepare_wasm_env_cleanup` only *queues* the promise settlements of the
208+
// tasks it cancels: `napi_call_threadsafe_function` appends to the
209+
// threadsafe-function queue, and @emnapi/core dispatches that queue from a
210+
// macrotask two coalescing turns later. `Context.destroy()` then drains the
211+
// queue with a null env and discards it. Measured against @emnapi/core
212+
// 2.0.0-alpha.3: zero microtask checkpoints ever deliver the settlement, and it
213+
// takes exactly two macrotask turns.
214+
//
215+
// So a loader that emits the barrier and `destroy()` back to back strands
216+
// exactly the promises the barrier exists to settle, and it does so silently.
217+
// `examples/custom-async-runtime` covers the hand-written loader and the two
218+
// emitted flavors it builds (node cjs threadless, deferred/workerd)
219+
// behaviorally; the other flavors are never instantiated by any test, so assert
220+
// the shape here as well.
221+
const wasiLoaderCases: Array<{ name: string; code: string }> = [
222+
{ name: 'node cjs', code: createWasiBinding('test', '@scope/test') },
223+
{
224+
name: 'node cjs threadless',
225+
code: createWasiBinding('test', '@scope/test', 4000, 65536, false),
226+
},
227+
{ name: 'browser esm', code: createWasiBrowserBinding('test') },
228+
{ name: 'deferred/workerd', code: createWasiDeferredBrowserBinding('test') },
229+
]
230+
231+
for (const { name, code } of wasiLoaderCases) {
232+
test(`WASI loader waits for queued settlements before destroy: ${name}`, (t) => {
233+
t.true(
234+
code.includes('napi_prepare_wasm_env_cleanup'),
235+
'loader must run the pre-teardown barrier',
236+
)
237+
t.true(
238+
code.includes('napi_wasm_env_cleanup_pending'),
239+
'loader must poll napi_wasm_env_cleanup_pending; without it the barrier queues settlements that Context.destroy() then discards',
240+
)
241+
t.true(
242+
code.includes('__drainWasmEnvCleanup'),
243+
'loader must call the settlement drain on its disposal path',
244+
)
245+
t.true(
246+
code.includes('__scheduleMacrotask'),
247+
'the drain must yield real macrotask turns; microtask checkpoints never let the emnapi dispatch run',
248+
)
249+
// Initialization rollback is the same hazard, one step earlier. Registration
250+
// runs with a live environment, so a module-init hook can start a task and
251+
// *then* return an error; the barrier cancels the task and queues its
252+
// rejection, and a rollback that destroys the context in the same turn
253+
// discards it — stranding a promise that already escaped into JavaScript.
254+
t.true(
255+
initializationRollbackBody(code).includes(
256+
DRAIN_CALL_BY_ROLLBACK_FLAVOR[
257+
code.includes(EAGER_ROLLBACK_SIGNATURE) ? 'eager' : 'deferred'
258+
],
259+
),
260+
'initialization rollback must drain queued settlements before destroying the context',
261+
)
262+
})
263+
}
264+
265+
const EAGER_ROLLBACK_SIGNATURE = 'function __rollbackWasiInitialization() {'
266+
const DEFERRED_ROLLBACK_SIGNATURE = "__lifecycleState = 'failed'"
267+
const DRAIN_CALL_BY_ROLLBACK_FLAVOR = {
268+
eager: '__drainWasmEnvCleanup',
269+
deferred: '__prepareForDisposal',
270+
} as const
271+
272+
/**
273+
* The body of a loader's initialization-failure path: `__rollbackWasiInitialization`
274+
* for the eager loaders, `__createInstance`'s catch for the deferred one.
275+
* Sliced rather than searched whole-file, so a drain that only runs on the
276+
* ordinary disposal path cannot satisfy the assertion.
277+
*/
278+
function initializationRollbackBody(code: string): string {
279+
const eagerStart = code.indexOf(EAGER_ROLLBACK_SIGNATURE)
280+
if (eagerStart !== -1) {
281+
return code.slice(eagerStart, code.indexOf('\n}', eagerStart))
282+
}
283+
const deferredStart = code.indexOf(DEFERRED_ROLLBACK_SIGNATURE)
284+
return code.slice(deferredStart, code.indexOf('throw error', deferredStart))
285+
}
286+
203287
test('createEsmBinding is Node 12 compatible', (t) => {
204288
const code = createEsmBinding('test', '@scope/test', ['sum'])
205289
assertValidJS(t, code, 'esm')

0 commit comments

Comments
 (0)