Fix the V8+WASIX large-payload memory leak and the host-heap corruption abort - #53
Merged
Conversation
Every GuestHeap chunk built its offset allocator with `Allocator::new`, which sizes the node arena for 128Ki allocations no matter how big the chunk is. Its `reset` allocates that eagerly — a 28-byte Node plus a u32 free-list slot per allocation — so each 1 MiB chunk of guest linear memory cost roughly 2 MiB of HOST metadata. Chunks are claimed as a workload's peak grows and are never released, so that overhead accumulated for the life of the process. This was the dominant term in the V8+WASIX large-payload leak. heaptrack attributes 272 MB of the 343 MB retained by a 240 MiB echo run to `offset_allocator::Allocator::with_max_allocs` over 130 calls — one per chunk. Host RSS grew about three times the guest memory being managed, which is why the leak appeared to live half in linear memory and half on the host heap. Budget one node per 256 bytes of chunk instead (clamped), leaving ~2x headroom for the ~10 KiB buffers this heap actually serves. Exhausting a chunk's nodes is graceful — `allocate` returns None and `try_chunks` falls through to another chunk — so an under-estimate costs a little fragmentation, not correctness. Measured on the payload load test (echo-buffer, 10 MiB bodies, 1.95 GiB moved, wasmer/edgejs-v8 under wasmer#6850): net RSS growth 368 MB -> 134 MB, host anonymous RSS growth 236 MB -> 16 MB, and per-round retention now settles to 0.4 / -0.1 / 0.1 MB over the last three rounds instead of still stepping.
… threads V8 destroys backing stores from its ArrayBufferSweeper, which runs on background threads, so ExternalBackingStoreDeleter executes concurrently with the main thread creating new hints. The deleter erased from napi_env__::external_backing_store_hints -- a std::unordered_set -- and deleted the hint, with no synchronization against the two insert sites or against teardown. Concurrent mutation of the set corrupts the host heap: glibc aborts with "malloc_consolidate(): unaligned fastbin chunk detected" a few hundred requests into a payload-heavy workload, killing the runtime mid-request. GuestHeap already takes its own mutex precisely because "V8 GC threads free backing stores off the JS thread"; this container was missed. Guard the set with a process-global mutex -- global rather than a member because a deleter must read hint->env before it knows which env to lock, and that read itself races teardown. The deleter now reads env, unlinks, and claims finalize_cb in one critical section, then runs the finalizer outside the lock since finalizers re-enter N-API. Teardown detaches the whole set under the lock and collects the claimed callbacks, so exactly one of the two paths ever runs a given finalizer and teardown never touches a hint a concurrent deleter may already be freeing. Evidence: instrumenting the deleter with the calling thread id showed it firing on a non-main thread within the first round of the payload load test, and a try_lock counter on the new mutex recorded 300+ genuine concurrent accesses in a single five-round run -- each one a moment the old lock-free code was mutating the set from two threads at once. On the payload load test (echo-buffer, 256 KiB bodies, concurrency 8, five rounds), the runtime died in 2 of 6 runs before this change -- one with the malloc_consolidate signature -- and in 0 of 18 runs after.
This was referenced Aug 10, 2026
syrusakbary
approved these changes
Aug 10, 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.
Two independent bugs on the V8/N-API WASIX lane, both surfaced by moving large HTTP request bodies through
wasmer/edgejs-v8. Neither affects quickjs.1. Guest-heap chunk allocators leaked ~2 MiB of host metadata per 1 MiB chunk
GuestHeap::claim_chunk_lockedbuilt each chunk's allocator withoffset_allocator::Allocator::new(len / UNIT). That constructor hard-codesmax_allocs = 128 * 1024regardless of chunk size, and itsreset()eagerly allocatesnodes: vec plus au32free-list entry. So every 1 MiB chunk of guest linear memory cost roughly 2 MiB of host metadata. Chunks are claimed as a workload's peak grows and are never released, so that overhead accumulated for the life of the process — host RSS grew about three times the guest memory being managed.heaptrack named it outright on a run that moved only 240 MiB:
130 calls = 130 chunks.
Fix: budget one node per 256 bytes of chunk (clamped to
[1024, 32768]), which leaves ~2x headroom for the ~10 KiB buffers this heap actually serves. Exhausting a chunk's nodes is graceful —allocatereturnsNoneandtry_chunksfalls through to another chunk — so an under-estimate costs a little fragmentation, not correctness.Measured on the payload load test (echo-buffer, 10 MiB bodies, 1.95 GiB moved):
2. The external-backing-store hint set was mutated from V8 sweeper threads
V8 destroys backing stores from its ArrayBufferSweeper, which runs on background threads, so
ExternalBackingStoreDeleterexecutes concurrently with the main thread creating new hints. It erased fromnapi_env__::external_backing_store_hints— astd::unordered_set— and deleted the hint, with no synchronization against the two insert sites or against teardown. Concurrent mutation of the set corrupts the host heap: glibc aborts withmalloc_consolidate(): unaligned fastbin chunk detecteda few hundred requests into a payload-heavy workload, killing the runtime mid-request.GuestHeapalready takes its own mutex precisely because "V8 GC threads free backing stores off the JS thread". This container was missed.Fix: guard the set with a process-global mutex — global rather than a member because a deleter must read
hint->envbefore it knows which env to lock, and that read itself races teardown. The deleter now reads env, unlinks, and claimsfinalize_cbin one critical section, then runs the finalizer outside the lock (finalizers re-enter N-API). Teardown detaches the whole set under the lock and collects the claimed callbacks, so exactly one path ever runs a given finalizer and teardown never touches a hint a concurrent deleter may already be freeing.Evidence. The abort is too flaky to A/B directly, so the race was measured rather than waited for:
try_lockcounter on the new mutex recorded 300+ genuine concurrent accesses in a single five-round run — each one a moment the old lock-free code was mutating the set from two threads at once.With a config that does reproduce the abort (echo-buffer, 256 KiB bodies, concurrency 8, five rounds): 2 of 6 runs died before this change (one with the
malloc_consolidatesignature), 0 of 28 after.Testing
wasmer-napilib tests green, including two newguest_heaptests covering the node budget and a chunk's real capacity in ~10 KiB buffers.