Skip to content

Fix WASM memory view invalidation when memory grows - #248

Open
justjake wants to merge 2 commits into
mainfrom
fix-wasm-memory-view-invalidation
Open

justjake wants to merge 2 commits into
mainfrom
fix-wasm-memory-view-invalidation

Conversation

@justjake

@justjake justjake commented Feb 16, 2026 •

Copy link
Copy Markdown
Owner

Summary

When Emscripten's WASM memory grows (due to -sALLOW_MEMORY_GROWTH), all existing TypedArray views become detached because the underlying ArrayBuffer is replaced. This caused bugs where reading from views after FFI calls returned undefined (in the C correctness sense) values if memory had grown during the call.

The fix introduces RefreshableTypedArray, a wrapper that lazily recreates the TypedArray view when HEAPU8.buffer changes. This uses a simple reference comparison that only triggers view recreation when actually needed.

Affected call sites:

  • runtime.ts: executePendingJobs - reads ctxPtrOut after QTS_ExecutePendingJob
  • context.ts: newPromise - reads resolve/reject handles after QTS_NewPromiseCapability
  • context.ts: getLength - reads uint32Out after QTS_GetLength
  • context.ts: getOwnPropertyNames - reads outPtr and uint32Out after QTS_GetOwnPropertyNames

Also fixed: getOwnPropertyNames was using HEAP8.buffer instead of HEAPU8.buffer

Fixes #240

Test plan

  • All 69 existing tests pass
  • Manual testing with memory-intensive workloads that trigger growth

🤖 Generated with Claude Code

When Emscripten's WASM memory grows (due to -sALLOW_MEMORY_GROWTH), all
existing TypedArray views become detached because the underlying
ArrayBuffer is replaced. This caused bugs where reading from views after
FFI calls returned undefined values if memory had grown.

The fix introduces RefreshableTypedArray, a wrapper that lazily recreates
the TypedArray view when HEAPU8.buffer changes. This is a simple reference
comparison that only triggers view recreation when actually needed.

Affected call sites:
- runtime.ts: executePendingJobs - reads ctxPtrOut after QTS_ExecutePendingJob
- context.ts: newPromise - reads resolve/reject handles after QTS_NewPromiseCapability
- context.ts: getLength - reads uint32Out after QTS_GetLength
- context.ts: getOwnPropertyNames - reads outPtr and uint32Out after QTS_GetOwnPropertyNames

Also fixed: getOwnPropertyNames was using HEAP8.buffer instead of HEAPU8.buffer

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@justjake
justjake force-pushed the fix-wasm-memory-view-invalidation branch from 57ce055 to 48f9af3 Compare February 16, 2026 17:10
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@Federaffo

Copy link
Copy Markdown

Update?

@xfy2412

xfy2412 commented Sep 22, 2026

Copy link
Copy Markdown

Independent confirmation from a downstream user, plus the memory-intensive workload the test plan
still has unchecked.

We hit the exact site your PR lists first (runtime.ts: executePendingJobs - reads ctxPtrOut after QTS_ExecutePendingJob), and it turns out to have a nasty downstream effect. Deterministic repro, on
quickjs-emscripten@0.32.0 + @jitl/quickjs-wasmfile-release-asyncify, with a 32 MiB memory limit:

const rt = QuickJS.newRuntime()
rt.setMemoryLimit(32 * 1024 * 1024)
rt.setMaxStackSize(512 * 1024)
const vm = rt.newContext()
vm.evalCode(`Promise.resolve().then(() => {
  const a = []; for (let i = 0; i < 200000; i++) a.push({ i });
})`)
rt.executePendingJobs()
vm.dispose()
rt.dispose()
// Aborted(Assertion failed: list_empty(&rt->gc_obj_list), at: quickjs.c, JS_FreeRuntime)

What we measured while the assert fires:

  • the out-parameter view is detached by the time it is read — typedArray.byteLength === 0,
    typedArray[0] === undefined, and the view's buffer is no longer module.HEAP8.buffer;
  • the heap grew during that very call (16 MiB → 23 MiB in our run), which is why the workload size
    decides the outcome: 120k objects tear down cleanly, 160k / 200k / 300k / 500k all abort;
  • because the value is undefined and not 0, the ctxPtr === 0 guard misses it,
    contextMap.get(undefined) misses too, and the ?? this.newContext({ contextPointer: ctxPtr })
    fallback runs — which on asyncify runtimes creates a brand new JSContext that nobody disposes.
    That leaked context is what trips assert(list_empty(&rt->gc_obj_list)) in JS_FreeRuntime:
    an uncatchable WASM abort, i.e. untrusted JS can kill the host process. (Filed as list_empty(&rt->gc_obj_list) assertion on dispose after allocating many objects inside a promise job (minimal repro, no host functions) #269 before we
    understood the cause; engine choice and build flags are irrelevant — quickjs-ng aborts at the same
    threshold with the same assertion, and disabling the stack limit changes nothing.)

We prototyped the minimal local fix (read the out parameter through a view of the current heap:
new Int32Array(module.HEAPU8.buffer, ptr, 1)[0]) and with it that particular script tears down
cleanly instead of aborting. Caveat, because it matters: our first measurements were taken on an
instrumented build (a -DDUMP_LEAKS variant, ~7 KB bigger); on the stock wasm the outcome turns out to
depend on the surrounding allocation history — a script that differs only by an extra
globalThis.__k = a.length and one extra evalCode still aborts with the same one-line fix applied.
So we can confirm the cause (detached view → undefined → leaked context) but we are not claiming
a complete fix, and we are not claiming your PR closes #269 on its own. The detached read is the same
site your RefreshableTypedArray covers, so it should close that path — worth verifying on the stock
build rather than trusting our number. If it helps, I'm happy to run the sweep against your branch once
it is rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory growth invalidates ctxPtrOut, causing unmanaged context creation in executePendingJobs

3 participants