Repository navigation
Let environments on different threads run JavaScript concurrently - #107
Merged
Merged
Conversation
The bridge held its process-wide recursive mutex g_mu across most unofficial_* calls, including the ones that run JavaScript: contextify run_script, module evaluation, microtask checkpoints, structured clone, message serialization (property getters run) and GC. While one environment was inside such a call, every other environment blocked at its next bridge call, and NapiRuntimeControl::terminate_all() blocked on the same lock, so it could not stop a script started that way. g_mu is now a plain std::mutex that guards only the env registry and the process-wide message table. The bridge does not enter V8 or call guest code while holding it. The one exception is a terminate or interrupt request from another thread: it keeps the target registered for the duration of TerminateExecution or RequestInterrupt, which V8 allows from any thread and which do not run JavaScript. An environment belongs to the thread that created it. The V8 provider enters the isolate and keeps persistent scopes there. Ordinary calls, callback registration and release from another thread now return napi_invalid_arg instead of touching V8 from that thread. Other threads may only terminate or request an interrupt. A closing flag, set under g_mu before teardown, keeps those requests away from an environment being released, and a pending interrupt no longer runs guest code during teardown. snapi_bridge_dispose() releases only the calling thread's environments and no longer drops queued messages, which belong to their queues. NapiRuntimeHooks documents the thread affinity. g_mu also used to serialize provider state that is shared by all environments. In the V8 provider: - The module-wrap maps are now always accessed under g_module_wrap_mu. Some lookups and the cleanup-hook set were unlocked; concurrent module loading crashed in EnsureModuleWrapCleanupHook without this. - Isolate construction and V8 disposal are serialized, so a new isolate cannot be allocated and registered with EdgeV8Platform, whose per-isolate state is keyed by address, while another isolate is between Dispose() and unregistration. - unofficial_napi_create_env restores V8's default pkey permissions on the creating thread (the same call #93 makes), since that thread may predate the key V8 uses for thread-isolated JIT memory on Linux x64. tests/concurrent_envs.rs runs a WAT guest per Wasmer store, on separate threads, through real import dispatch, the bridge and V8. Six of its eight tests fail on main: four entry points that blocked other environments, a host terminate that blocked, and a foreign-thread call that succeeded. The other two guard the narrower locking: a message still has exactly one consumer, and envs that come and go on several threads while loading modules neither crash nor hang. A new CI job runs the suite on Linux. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
LookupEnvState took g_mu on every N-API call to check that the env was alive and owned by the calling thread. With several envs on several threads, that one lock became the bottleneck for ordinary calls. Each thread now keeps the set of envs it created and has not released. Only that thread inserts or erases its entries, so membership proves both liveness and ownership without a lock. g_mu is left to foreign terminate/interrupt and the message table. The set is a heap object behind a plain thread_local pointer, freed with the thread's last env, so a thread-exit destructor that releases an env can still use it. bridge_call_throughput (an ignored test in tests/concurrent_envs.rs) measures napi_get_value_int32 through the bridge with one env per thread, all calling at once. With this crate built at opt-level 3, in ns per call for 1/2/4 threads: main 13.9/57.7/165.7, before this commit 11.0/62.2/156.5, after 6.0/6.1/7.4. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Releasing an env runs its finalizers, and a guest finalizer can run JavaScript. That JavaScript could not be stopped. The bridge refused terminate as soon as teardown began, and the Rust side removed the env from the application registry before the release, so NapiRuntimeControl::terminate_all() no longer saw it. Bridge: - A terminate during teardown now stops guest code that teardown is running. The bridge counts guest code entered from teardown under g_mu; while that count is non-zero, the napi_env and isolate are alive. It also sets a sticky stop flag, after which no more guest code starts. Outside guest code no JavaScript can run, so nothing else is needed. - Callback bindings are freed after the release instead of before it. JavaScript run by a finalizer could otherwise call a freed binding. - An env's bridge state has holders: the bridge until release, plus snapi_bridge_unofficial_retain_env/unretain_env. A pointer kept in the Rust registry may then outlive the env without ever being reused for another one. Rust (env.rs): - An env stays in the registry, retained, until its release returns. - Its budget charge and near-heap-limit tracker are returned only after a release the bridge accepted. NapiEnv's Drop used to free the tracker and return the budget before releasing, although finalizers can run JavaScript that reaches the tracker. A store dropped on another OS thread got the release refused and leaked the isolate uncounted. Now that env stays charged and the refusal is logged. Tests: host_terminate_reaches_javascript_a_finalizer_runs_during_release (the finalizer spins in JavaScript during release until terminate_all) and an_env_whose_store_is_dropped_on_another_thread_stays_charged. Both fail without this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The prepare-stack-trace paths take g_runtime_mu and then, through NapiV8IsContextifyContext, g_context_mu. Applying promise hooks to contextify contexts did the reverse: it held g_context_mu while LookupPromiseHooksForContext took g_runtime_mu. Two envs on different threads could deadlock. The bridge lock used to serialize these calls; it no longer does. NapiV8ApplyPromiseHooksToContextifyContexts now copies the contexts out under g_context_mu and applies the hooks after releasing it. The reversed path only compiles with V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS. Neither the cargo build nor the CMake build defines it with the prebuilt V8, so no test here can reach it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- An interrupt that V8 has queued but not yet run is dropped without a callback when the isolate is disposed, so the bridge's request leaked. The bridge now tracks pending requests per env under g_mu and frees the rest after release. The provider's own small InterruptRequest still leaks in that case. The leak is not observable from a test. - NapiRuntimeHooks now states that a store which created environments must stay on one OS thread for its whole life and be dropped there. That rules out resuming it on another thread, for example after WASIX deep sleep. - The tests' Atomics.wait timeout (20 s) now exceeds their polling deadlines (10 s). - CI: the job runs only when relevant paths change, and its cache key names the toolchain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It is the bridge's status code, not a flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 22, 2026
syrusakbary
added a commit
that referenced
this pull request
Oct 28, 2026
Brings in #107 (envs on different threads run JavaScript concurrently). Releasing an env tears down its whole agent on this branch, so #107's teardown state moves to SnapiAgentState: `closing`, `stop_requested`, `teardown_guest_calls` and the pending interrupt requests. GuestCallScope, the interrupt callback, terminate and request_interrupt reach it through the env's agent. `holders`, `g_envs` and each thread's owned-env set stay per env, since they guard the env pointers the Rust registry holds. DisposeBridgeState keeps #107's order (callback bindings freed after the release, registry entries dropped under g_mu, unserviced interrupts freed) for every env of the agent, then frees the agent. snapi_bridge_dispose releases one owned env at a time, because releasing one forgets every env of its agent. NewAgentWithPrincipalEnv registers the env under g_mu and adopts it on the creating thread. docs/realm-bound-env.md notes that #107 landed and where its state lives. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
syrusakbary
added a commit
that referenced
this pull request
Oct 28, 2026
Brings in #121, #122 and #107 through Phase 0. Bridge: realm env states follow #107's rules. create_realm and get_realm no longer hold g_mu across the provider call; AddRealmEnvState registers the state in g_envs under the lock and adopts it on the agent's thread, like NewAgentWithPrincipalEnv. Teardown marks the agent closing first, then releases agent-wide resources through the principal env. V8 contextify: GetModuleWrapState takes g_module_wrap_mu, next to Phase 1's agent-wide module lookups, which take it themselves. Promise hooks for contextify contexts are collected under g_context_mu and applied after releasing it, each with its realm env's hooks. host-JS: buffer finalizer registration borrows the HostJsAgent behind SharedAgent once, so its fields can be used together. The design doc notes that realm env states belong to their agent's thread. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
syrusakbary
added a commit
that referenced
this pull request
Oct 28, 2026
Host-JS envs share one JavaScript realm: builtins and prototypes are common to all of them, so guest code reached through one env can release another whose bridge call is still on the stack below it. A per-env count of guest calls cannot see that. Host-JS now counts guest code running on the thread, for any env, and refuses releasing any env meanwhile. The guest's malloc, which the guest layer calls when its buffer pool runs dry, counts as guest code too. The native query becomes snapi_bridge_unofficial_env_release_refused and also reports the refusals #107 added (another thread owns the env, or its release is under way), so the guest release import refuses all of them before touching the env's bookkeeping. The native test also releases an env from a finalizer. The host-JS test checks that both of two envs are refused while guest code runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
syrusakbary
added a commit
that referenced
this pull request
Nov 5, 2026
Brings in main up to #125 through #111. - The bridge's realm calls (create_realm, get_realm, run_script) no longer take g_mu. #107 made it a plain mutex that must not be held while V8 or guest code runs; RequireEnvState already keeps them on the env's thread. - #125's native Worker realm works on realm contexts. A context made for the Worker's global runs unofficial_napi_run_script through the bridge's evaluate(), with its filename and offsets, and napi_run_script through binding(). create_realm refuses with a pending TypeError, also inside the host JavaScript. - Tests from main use the realm ABI: module options version 2 with realm_or_null, and the WAT guest imports unofficial_napi_run_script. - PromiseHooks keep #116's per-env realm list; g_context_records is gone. - docs/realm-bound-env.md stays removed. docs/realms.md describes the native Worker realm and the threads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
syrusakbary
added a commit
that referenced
this pull request
Nov 5, 2026
Brings in main up to #125 through #116. - enter_realm and leave_realm in the bridge no longer take g_mu, like the other realm calls since #107. - A native Worker realm's callbacks keep #125's enterCallback / invokeCallback / leaveCallback bracket. Such a realm makes no other realms, so the function's realm is always undefined there, and dispatch keeps the bridge's three arguments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 5, 2026
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.
Problem
The bridge's process-wide recursive mutex
g_muwas held across mostunofficial_*calls, including the ones that run JavaScript: contextifyrun_script, module evaluation, microtask checkpoints, structured clone, message serialization (property getters run), GC. While one environment was inside such a call, every other environment blocked at its next bridge call, becauseLookupEnvStatetakes the same lock. A hostNapiRuntimeControl::terminate_all()blocked on it too, so it could not stop a long script started through one of those entry points. Even without contention, every N-API call paid for that lock.Change
Bridge (
src/napi_bridge_init.cc)v8::Locker). Each thread keeps the set of environments it owns.release_env, getnapi_invalid_arginstead of touching V8 there.g_muis a plainstd::mutexthat guards only the env registry and the process-wide message table. The bridge never enters V8 or calls guest code while holding it. The one exception is a terminate or interrupt request from another thread: it keeps the target registered for the duration ofTerminateExecution/RequestInterrupt, which V8 allows from any thread and which run no JavaScript.closingflag, set underg_muwhen release starts, refuses foreign interrupts.main.snapi_bridge_unofficial_retain_env/unretain_envkeep an env's bridge state allocated while the Rust registry holds its pointer. A stale pointer can then never name a newer env.snapi_bridge_dispose()releases only the calling thread's environments. It no longer drops queued messages: a message belongs to its queue and may outlive its source environment.Rust (
src/env.rs,src/ctx.rs)terminate_all()can therefore stop JavaScript its finalizers run.NapiEnv'sDropused to free the tracker and return the budget first, although finalizers can run JavaScript that reaches the tracker.NapiRuntimeHooksdocuments the thread affinity. A store that created environments must stay on one OS thread for its whole life, which also rules out resuming it on another thread after WASIX deep sleep.V8 provider (
v8/src/)g_mualso used to serialize provider state that all environments share. Without it:g_module_wrap_mu. Some lookups and the cleanup-hook set were not locked. Before this fix, the module churn test below crashed inEnsureModuleWrapCleanupHook(1 of 5 runs).EdgeV8Platformkeys per-isolate state by address and relies on an address not being reused betweenDispose()and unregistration. I did not reproduce a failure from this one alone.g_context_muwhile takingg_runtime_mu; the prepare-stack-trace path takes them in the opposite order. It now copies the contexts out first. This path only compiles withV8_ENABLE_JAVASCRIPT_PROMISE_HOOKS, which neither build defines, so no test reaches it.unofficial_napi_create_envcallsv8::ThreadIsolatedAllocator::SetDefaultPermissionsForSignalHandler()before building the isolate, the same call fix(napi): isolate bridge environments and contain guest callback exits #93 makes. A thread that predates V8's JIT protection key (Linux x64 pkeys) otherwise lacks read access to that memory. I could not exercise this locally (Apple silicon has no pkeys).Performance
bridge_call_throughputis an ignored test intests/concurrent_envs.rs. It callsnapi_get_value_int32through the bridge with one env per thread, all threads calling at once. This crate was built at opt-level 3.mainTests
tests/concurrent_envs.rsruns a small WAT guest (tests/wat_guest/mod.rs): one Wasmer store per guest, one OS thread per environment. It goes through the real import dispatch, the C++ bridge and V8, and needs no WASIX C toolchain.a_contextified_script_does_not_block_other_envs: one env waits inAtomics.waitinsidecontextify_run_script; another env, on another thread, runs scripts and wakes itmain: the second env cannot run until the wait times outa_module_evaluation_does_not_block_other_envs: the same, insidemodule_wrap_evaluatemaina_microtask_checkpoint_does_not_block_other_envs: the same, inside a microtask thatevent_loop_checkpointdrainsmaina_serializer_getter_does_not_block_other_envs: the same, inside a getter thatmessage_createrunsmainhost_terminate_reaches_an_env_inside_a_contextified_scriptmain:terminate_all()blocksforeign_threads_may_only_terminate_an_env: through the bridge's C ABImain: the foreignget_globalsucceedshost_terminate_reaches_javascript_a_finalizer_runs_during_releasean_env_whose_store_is_dropped_on_another_thread_stays_chargeda_message_has_exactly_one_consumerenvs_on_several_threads_come_and_go_while_loading_modules: 4 threads × 25 envs, boundedmain, where everything is serialized; crashed without the module-wrap fixA new workflow,
napi-native-concurrency.yml, runs the suite onubuntu-24.04when relevant paths change. It follows the native job in #93. I could not run it; drop it or fold it into another job if you prefer.Local results (macOS arm64, Rust 1.96.1, V8 11.9.9 prebuilt):
concurrent_envs: 10/10, plus 20/20 repeated runs of the whole binary.v8_host_control: 3/3.make test-napi): 97/97.snapi_jswasm-bindgen tests: 6/6.cargo fmtis clean. Native clippy shows only the existing CLI test-module warning, and host-JSwasm32clippy output is unchanged.Behavior changes
v8_host_controlsaw no refused release.snapi_bridge_dispose()no longer drops queued messages. Nothing in this crate calls it.Known follow-up (not changed here)
Message ids (
g_message_handles) are process-wide, sequential and carry no owner check. Any guest that guesses an id can take or drop another application's message. This is already the case onmain.Relation to #93
#93 also removes the process-wide lock from JavaScript execution, as part of a larger change. Both approaches are valid; which fits better is the maintainers' call.
Locking model. fix(napi): isolate bridge environments and contain guest callback exits #93 keeps
g_mu(recursive) for the registry and isolate construction, and adds a per-environment recursive mutex that every bridge call takes.request_interrupttakes the per-environment lock, so an interrupt from another thread waits for the target's current call to return.What fix(napi): isolate bridge environments and contain guest callback exits #93 adds beyond concurrency. It converts guest
napi_fatal_errorto a WASIX exit, propagates guest exits from callback-capable Node-API calls, adds an external-buffer ownership observer in the V8 provider to avoid a double free, and rejectsrelease_envfrom inside the environment's own callback. This PR does none of that; those fixes are independent of the locking change.Shared provider state. fix(napi): isolate bridge environments and contain guest callback exits #93 changes neither the module-wrap locking, nor isolate disposal, nor the lock order. Those fixes apply to it as well.
Size. fix(napi): isolate bridge environments and contain guest callback exits #93 is 19 files, +3901/−2118 against
eac0a13. Of that,src/napi_bridge_init.ccis +2717/−1826, much of it reformatting, and fix(napi): isolate bridge environments and contain guest callback exits #93 is the first of a nine-PR stack. This PR is 10 files, +1303/−176 against8deda28:Not in fix(napi): isolate bridge environments and contain guest callback exits #93. Cross-store terminate is in Allocate env ids per application and let stores terminate each other's envs #108, stacked on this one.
The branch is based on
8deda28.mainhas since gained #103 and #104;git merge-treeshows both branches merge cleanly.🤖 Generated with Claude Code