fix: harden emnapi v2 memory, threads, and WASI packaging#220
Draft
Brooooooklyn wants to merge 120 commits into
Draft
fix: harden emnapi v2 memory, threads, and WASI packaging#220Brooooooklyn wants to merge 120 commits into
Brooooooklyn wants to merge 120 commits into
Conversation
This was referenced Jun 26, 2026
Brooooooklyn
added a commit
to Brooooooklyn/emnapi
that referenced
this pull request
Jun 30, 2026
toyobayashi#220 fixed napi_new_instance to refresh its memory view after the reentrant constructor grows (and detaches) unshared wasm memory, but the accompanying grow test only exercised napi_call_function (TestCall) — the napi_new_instance fix had zero coverage and reverting it stayed green. Add a TestNewInstance C binding that constructs through napi_new_instance, plus a test whose constructor grows wasmMemory before returning and then asserts the returned instance handle is non-corrupt (instanceof + a property reads back). Reuse the existing `wasmMemory && buffer instanceof ArrayBuffer` guard so native Node skips it. Verified on wasm32-wasip1: passes with the fix; reverting function.ts to the old makeSetValue makes it fail with "DataView.prototype.setUint32 on a detached ArrayBuffer" at napi_new_instance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
toyobayashi#220 fixed napi_new_instance to refresh its memory view after the reentrant constructor grows (and detaches) unshared wasm memory, but the accompanying grow test only exercised napi_call_function (TestCall) — the napi_new_instance fix had zero coverage and reverting it stayed green. Add a TestNewInstance C binding that constructs through napi_new_instance, plus a test whose constructor grows wasmMemory before returning and then asserts the returned instance handle is non-corrupt (instanceof + a property reads back). Reuse the existing `wasmMemory && buffer instanceof ArrayBuffer` guard so native Node skips it. Verified on wasm32-wasip1: passes with the fix; reverting function.ts to the old makeSetValue makes it fail with "DataView.prototype.setUint32 on a detached ArrayBuffer" at napi_new_instance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brooooooklyn
force-pushed
the
fix/unshared-memory-grow-views
branch
from
July 5, 2026 11:04
9db33ca to
aac760a
Compare
Brooooooklyn
force-pushed
the
fix/unshared-memory-grow-views
branch
from
July 11, 2026 03:08
1c5ee42 to
c49fb45
Compare
value/convert2c.ts gained an import from '../brand' but the vm test's require allowlist was never updated, so tsfn2/tsfn_shared_growth.test.js crashed with "Unexpected test import: ../brand" on every wasm target. The exercised path (napi_get_value_string_latin1) never calls brand helpers, so an empty stub matches the map's existing style. Verified: full `npm run test:w -w packages/test` passes locally (wasi-sdk 32, 83 test files). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
test:n runs plugin unit tests (tsfn2/v8_memory_growth, lifecycle, ...) that require @emnapi/core/plugins and packages/emnapi/dist/library_v8.js, but the node-native job only built shared/runtime/node, so Node 26 failed with MODULE_NOT_FOUND on @emnapi/core/dist/plugins/index.js. Build wasi-threads/emnapi/core too (dependency order preserved). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
Windows cannot spawn a .js file directly (spawn EFTYPE since the CVE-2024-27980 hardening), so test-release-bump.js failed in the release-tools-windows job at bump.js's npm spawn. Wrap the stub in an npm-stub.cmd shim exactly like test-release-publish.js already does. Verified: node ./script/test-release-bump.js still passes on macOS; Windows path mirrors the publish suite's proven shim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
Node.js 22.12.0 (the documented minimum) caps stable Node-API at version 9: it gates node_api_create_property_key_* and the external string declarations behind NAPI_EXPERIMENTAL and rejects stable version-10 modules at load time (node_api.cc NewEnv), so the native string/string_mt/buffer targets neither compiled (property-key function pointers were hard errors) nor could ever load there. Keep NAPI_VERSION=10 for wasm builds against emnapi's own headers and switch the native builds to NAPI_EXPERIMENTAL + NODE_API_EXPERIMENTAL_NO_WARNING (same pattern the dataview target already uses). Verified: npm run rebuild:n succeeds against real Node 22.12.0 and 26.5.0 locally; string/buffer tests load and run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
v8::internal::Isolate stored its data as plain char[1024] (alignof 1), while Internals::GetRootSlot reads it through internal::Address*. The static instance in Isolate::GetCurrent() therefore only happened to be 8-aligned on wasm64; the enlarged v8_hello_world test binding shifted the data segment so it landed on a 4-aligned address, and the first ReturnValue::Get() root load trapped under SAFE_HEAP=1 with "Aborted(alignment fault)". Declare the buffer alignas(kApiSystemPointerSize) so correctness no longer depends on surrounding layout. Verified: reproduced the exact CI stack locally (emsdk 6.0.2, MEMORY64=1 Debug); after the fix v8_hello_world passes on both wasm64 and wasm32 emscripten builds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
…rement
When a wasm thread traps during start, the worker posts
cleanup-thread{force:true} and then rethrows. The owner side terminates
the poisoned worker as soon as the cleanup message arrives, which can
make the host drop the worker's still-pending uncaught 'error' event:
the trap was never reported or rethrown in the parent, so processes
that should die with the trap (packages/test trap_in_thread) hung until
timeout on all four wasm32-wasip1-threads CI jobs.
Carry the serialized error details on the forced cleanup-thread message
itself and surface them (report + terminateAllThreads + async rethrow,
matching the direct 'error'-event path) before retiring the worker,
deduplicated once per worker via WorkerState.fatalErrorSurfaced so the
message path and a delivered 'error' event cannot double-fire. Worker
retirement/pool-replenish behavior is unchanged.
Verified: trap_in_thread passes on both libemnapi-mt and
libemnapi-basic-mt (wasi-sdk 32); full test:wt suite green on both
variants; wasi-threads unit tests green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
The native CI legs run the emnapi test suite against real Node.js, whose minimum supported version (22.12.0) lacks several APIs and behaviors the suite exercises. Gate them on detected capabilities so the same files keep full coverage on wasm targets (emnapi implements everything) and on newer Node.js, and skip precisely on old native runtimes. Every skip logs, and (except pure engine capabilities) asserts it can only happen under EMNAPI_TEST_NATIVE, so wasm runs are provably unaffected. Compile-time capability macros (headers advertise the API; emnapi's own headers always define them, real Node >= 26 defines them under NAPI_EXPERIMENTAL, 22.12.0 does not): - general: NODE_API_EXPERIMENTAL_HAS_SET_PROTOTYPE gates testSetPrototype (+ export-presence guard in general.test.js) - object: NODE_API_EXPERIMENTAL_HAS_CREATE_OBJECT_WITH_PROPERTIES gates the three TestCreateObjectWith* tests (+ export-presence guard) - sharedarraybuffer: NODE_API_EXPERIMENTAL_HAS_SHAREDARRAYBUFFER / _HAS_CREATE_EXTERNAL_SHAREDARRAYBUFFER gate all SAB tests; a new always-present sharedArrayBufferApiSupported() probe keeps the descriptor array non-empty and drives the JS skip - dataview: same macro gates the SAB branch of CreateDataView; new SupportsSharedArrayBuffer() probe drives JS skips of the SAB cases These also fix the native build with clang, where the previously implicit declarations were hard errors. Runtime capability probes: - typedarray.test.js: typeof Float16Array (JS engine capability) - typedarray_sharedarraybuffer.test.js: one-shot native-only try/catch probe of napi_create_typedarray over a SharedArrayBuffer - runjs / ref_by_node_api_version: process.versions.napi >= 10 gates the Node-API version 10 module halves. Loading is not probe-able: Node.js 22.12.0 SIGSEGVs inside napi_module_register_by_symbol when rejecting a v10 module (NewEnv throws, returns nullptr, caller dereferences it), but process.versions.napi is the runtime's own declared capability. - objwrapbasicfinalizer: child-process probe, skipping only on the exact 'Finalizer is calling a function that may affect GC state' fatal; the abort kills the process, so it cannot be probed in-process. Version-gated (probe impossible - the tested behavior is a process-lifetime teardown sequence, so any probe IS the crash): - tsfn_shutdown: Node.js 22.12.0 kills the child with a signal during threadsafe-function teardown at environment shutdown - tsfn_abort: Node.js 22.12.0 finalizes an aborted TSFN without draining its callback, tripping the binding's own assert Both verified failing on 22.12.0 and passing on 24.18/26.5; gated at the oldest verified-good major (24). - general.test.js napi_get_version assertion now expects process.versions.napi on native (the value IS the capability). Verified locally (real binaries): Node 22.12.0 native 91/91 pass (11 files log skips), Node 26.5.0 native 91/91 pass with zero skips, wasm32-wasip1 test:w 83/83 pass with zero skips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
surfaceWorkerFatalError (the forced cleanup-thread path) reported and rethrew but never consulted worker.onerror, while the native error-event path gives a registered onerror handler priority and only falls back to the default report (and its RuntimeError thread teardown) without one. Whichever channel arrived first therefore decided whether the application's onerror handler observed the trap. Mirror the native dispatch in surfaceWorkerFatalError so the exactly-once surface is order-independent: onerror first, default report only without a handler, one async rethrow. Adds a unit test covering both arrival orders with and without a registered onerror handler (exactly one rethrow, exactly one onerror call or exactly one default report). Verified: wasi-threads unit suite green; trap_in_thread integration and full test:wt (133) + 4GB variant green on wasm32-wasip1-threads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
The previous fix compiled the native string/string_mt/buffer targets with NAPI_EXPERIMENTAL unconditionally, so runtimes that do support stable Node-API version 10 (the Node 26 CI leg) stopped verifying the stable declarations and module registration. build-native.js now exports the running runtime's capability (process.versions.napi) and CMake picks stable NAPI_VERSION=10 unless that capability is below 10; only the Node.js 22.12.0 leg falls back to the experimental surface. The stable default also applies when the variable is absent (manual configures). Note: the dataview/general/object targets' all-build NAPI_EXPERIMENTAL predates this branch. Verified: Node 22.12.0 build compiles those targets experimental (and the suite passes 91/91), Node 26.5.0 compiles them stable v10 (build.ninja shows -DNAPI_VERSION=10 on all five compile units) and passes 91/91 with zero skips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
The probe treated any CreateTypedArray exception as "unsupported" and skipped the whole suite, which would silently mask a real regression on runtimes that do support SharedArrayBuffer-backed napi_create_typedarray. Skip only on the exact napi_invalid_arg signature an unsupporting runtime produces, rethrow anything else, and assert the probe's result (Uint8Array over the same buffer) when it succeeds. Verified: Node 22.12.0 still skips via the narrowed signature; Node 26.5.0 runs the suite with the probe assertions passing (91/91, zero skips). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
The hand-enumerated build list added for the node-native job omitted packages/emnapi's build-time workspace dependencies: its build script requires @emnapi/ts-transform-macro (script/build.js) and the other ts-transform packages, so a fresh runner failed with "Cannot find module '@emnapi/ts-transform-macro/dist/index.js'" on both matrix legs (local runs had stale dist outputs masking it). Use 'npm run build --workspaces --if-present' like every other job; the root workspaces array is dependency-ordered. Verified from a clean slate (all workspace dist/out outputs deleted): the exact step sequence passes on real Node 22.12.0, then the full native suite passes 91/91 on Node 22.12.0 and 26.5.0, and wasm test:w remains 83/83 with zero skips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Harden emnapi v2 for production WASI consumers discovered while validating napi-rs and Rolldown.
This remains v2-only. It does not add or test a v1 fallback.
Runtime fixes
MessageChannel, add configurableautoDestroy, and dispose host handles/listeners deterministically.WASI npm package
The published
emnapiv2 archive now carries and validates this exact matrix:Release validation now:
llvm-nm;mingw32-make, andnmake, including Windows.cmd/.batspawning.The real consumer link profiles are:
wasm32-wasip1:libemnapi-basic.awasm32-wasip1-threads:libemnapi-basic-napi-rs-mt.aStatus
toyobayashi/emnapi@44ae05d.1c5ee42be87843adcd4a410729eea37ee8a7df79.Verification
Passed locally on July 10, 2026 with Node 26.4.0:
git diff --check.All six GitHub CI targets pass at the exact draft head. The final CI-only change prevents JavaScript from draining the bounded TSFN queue before native code observes
napi_queue_full.Earlier runtime validation on this branch also passed the focused Context, memory-growth, TSFN, async-work recovery, worker-termination, wasm64, and WASI-thread suites. The full WASI-thread run reaches the independently reproduced pre-existing
async_context/gcable.test.jsfailure under Node 26.4.0.