Make GuestHeap the only V8 allocation path; hard-fail if it can't be built - #49
Merged
Merged
Conversation
…built configure_instance previously fell back to leaving guest_heap unset when the guest had no linear memory or GuestHeap::get_or_create failed, which sent every subsequent V8 backing-store allocation for that env through TrackingArrayBufferAllocator's host-heap/napi_host_external_try_charge path instead. That path is unbudgeted relative to guest memory and not guest-shared, so the fallback was a silent downgrade rather than a real alternative. Fail instantiation instead: N-API imports now require the guest to have linear memory and a successfully-initialized GuestHeap.
GuestHeap is the only ArrayBuffer/Buffer/TypedArray allocation path for the imports provider (previous commit), and it charges Pool::WasmLinear directly. That made TrackingArrayBufferAllocator's host-heap fallback path in AllocateImpl/Free unreachable for this crate's only caller of unofficial_napi_set_allocation_budget_hook (env.rs::commit_isolate) — the native (non-guest) edgejs embedder never called it either, so budget_hook_ was always null there too. Remove the whole chain: Rust napi_host_external_try_charge/_uncharge/_release + EnvExternalCharge, the C bridge wrapper, the C++ hook installer, and TrackingArrayBufferAllocator's budget_hook_ member. Pool::V8External and napi_adjust_external_memory's charge_declared_external/ uncharge_declared_external are untouched — that's a separate, still-live charge path that never went through this hook.
This was referenced Jul 31, 2026
Arshia001
added a commit
to wasmerio/edgejs
that referenced
this pull request
Aug 7, 2026
…round-task queue drain) (#137) * test: add a WASIX-lane regression test for FinalizationRegistry cleanup V8 posts the callback for a FinalizationRegistry with dead targets as a deferred foreground task after GC. In the V8-imports (WASIX) lane that task was silently dropped whenever the guest hasn't bound its own foreground-task hook -- which edgejs never does, since it drives everything through unofficial_napi_process_microtasks instead. GC still correctly reclaimed the dead targets, but the registered callbacks never fired: confirmed 0/N finalized regardless of pressure or round count against the unpatched bridge, vs. the large majority finalized within the first few rounds once fixed (wasmerio/napi#49 branch fix/foreground-task-queue-drain). Node's own AbortSignal/WeakRef-based cleanup depends on this, so the practical effect was a slow, unbounded per-request leak in every long-running edgejs process. Deliberately uses real allocation-pressure + setTimeout-spaced rounds rather than v8.getHeapSnapshot()-forced GC (as guest-finalizer-memory uses): the heap-snapshot path does not reliably drive this specific mechanism, so it produced false passes even against the broken bridge. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore: bump napi submodule: drain the platform's fallback foreground-task queue Keeps this checkout's napi submodule (used by the native V8 build) in sync with wasmer/lib/napi, both now on wasmerio/napi#<pending> (fix/foreground-task-queue-drain): fixes V8-internal FinalizationRegistry cleanup callbacks never firing in the napi-v8 embedding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
2 participants
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
configure_instancenow fails instantiation outright if the guest has no linear memory orGuestHeap::get_or_createfails, instead of silently leavingguest_heapunset and lettingTrackingArrayBufferAllocatorfall back to unbudgeted, non-guest-shared host-heap allocation for V8 ArrayBuffers/Buffers/TypedArrays.napi_host_external_try_charge/_uncharge/_release,EnvExternalCharge, the C bridge wrapper, andunofficial_napi_set_allocation_budget_hook/budget_hook_):env.rs::commit_isolatewas its only caller anywhere in the codebase (native edgejs never installed it either), so once GuestHeap is mandatory the whole chain is unreachable.Pool::V8Externalandnapi_adjust_external_memory's declared-external charging are untouched — separate, still-live path.Test plan
cargo test -p wasmer-napi --lib— 36/36 green (was 38; removed 2 tests that exercised the deleted FFI functions directly, adapted a third to chargePool::V8Externaldirectly)cargo check -p wasmer-napi --features cli— clean, V8-linked buildrustfmt --edition 2024 --check— cleanStacked on #47.
🤖 Generated with Claude Code