Repository navigation
Commit 61fae8a
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
File tree
- .github/workflows
- cli
- src/api
- __tests__
- __snapshots__
- templates
- crates
- backend/src/codegen
- build/src
- napi
- src
- bindgen_runtime
- js_values
- js_values
- examples
- custom-async-runtime
- src
- napi
- __tests__
- __snapshots__
- src
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
861 | 861 | | |
862 | 862 | | |
863 | 863 | | |
864 | | - | |
| 864 | + | |
| 865 | + | |
| 866 | + | |
| 867 | + | |
865 | 868 | | |
866 | 869 | | |
867 | 870 | | |
| |||
908 | 911 | | |
909 | 912 | | |
910 | 913 | | |
911 | | - | |
| 914 | + | |
| 915 | + | |
912 | 916 | | |
913 | 917 | | |
914 | 918 | | |
| |||
926 | 930 | | |
927 | 931 | | |
928 | 932 | | |
| 933 | + | |
| 934 | + | |
| 935 | + | |
| 936 | + | |
| 937 | + | |
| 938 | + | |
| 939 | + | |
929 | 940 | | |
930 | 941 | | |
931 | 942 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
57 | 60 | | |
58 | 61 | | |
59 | 62 | | |
| |||
Large diffs are not rendered by default.
Binary file not shown.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
6 | 10 | | |
7 | 11 | | |
8 | 12 | | |
| |||
200 | 204 | | |
201 | 205 | | |
202 | 206 | | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
203 | 287 | | |
204 | 288 | | |
205 | 289 | | |
| |||
0 commit comments