From 47ab95c657d4484ad0aac971d2c6a38b699823f7 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Wed, 13 May 2026 00:30:15 +0000 Subject: [PATCH 1/2] js_malloc for js_free --- c/interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c/interface.c b/c/interface.c index 5e9fa3ae..cc16ce84 100644 --- a/c/interface.c +++ b/c/interface.c @@ -926,7 +926,7 @@ MaybeAsync(JSValue *) QTS_GetOwnPropertyNames(JSContext *ctx, JSValue ***out_ptr } return jsvalue_to_heap(JS_GetException(ctx)); } - *out_ptrs = malloc(sizeof(JSValue) * total_props); + *out_ptrs = js_malloc(ctx, sizeof(JSValue *) * total_props); for (int i = 0; i < total_props; i++) { JSAtom atom = tab[i].atom; From 57ddca9bdfa06592ff17022175c3818760d3c201 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Wed, 13 May 2026 07:09:28 +0000 Subject: [PATCH 2/2] allocator tests --- .../quickjs-emscripten/src/allocator.test.ts | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 packages/quickjs-emscripten/src/allocator.test.ts diff --git a/packages/quickjs-emscripten/src/allocator.test.ts b/packages/quickjs-emscripten/src/allocator.test.ts new file mode 100644 index 00000000..b259a8af --- /dev/null +++ b/packages/quickjs-emscripten/src/allocator.test.ts @@ -0,0 +1,143 @@ +/** + * Allocator balance checks. + * + * Each test verifies that a sequence of operations leaves + * JSMallocState.malloc_count unchanged — i.e., every js_malloc call in the C + * layer is matched by exactly one js_free call. + * + * If malloc_count drifts downward by exactly N after N calls, a pointer was + * freed via js_free() without ever having been allocated via js_malloc() + * (e.g. by using the system malloc() instead). Over time this causes the + * internal allocator accounting to underflow, corrupting the GC threshold and + * eventually causing bad behavior. + */ + +import assert from "assert" +import { afterEach, beforeEach, describe, it } from "vitest" +import type { QuickJSContext, QuickJSWASMModule } from "." +import { + DEBUG_ASYNC, + DEBUG_SYNC, + getQuickJS, + memoizePromiseFactory as memoizeNewModule, + newQuickJSAsyncWASMModule, + newQuickJSWASMModule, + Scope, +} from "." + +// --------------------------------------------------------------------------- +// Harness + +type AllocatorCheck = (ctx: QuickJSContext) => void + +/** + * Read malloc_count from the runtime's internal allocator state. + * + * The temporary handle from computeMemoryUsage() is disposed inside a nested + * Scope so it does not inflate the count we return. + */ +function readMallocCount(ctx: QuickJSContext): number { + return Scope.withScope((s) => { + const handle = s.manage(ctx.runtime.computeMemoryUsage()) + return (ctx.dump(handle) as any).malloc_count as number + }) +} + +/** + * For each named check function, register a vitest test that: + * + * 1. Creates a fresh QuickJS context. + * 2. Warms up the computeMemoryUsage atom cache so subsequent calls do not + * allocate new atoms and shift the baseline. + * 3. Captures malloc_count before the check. + * 4. Runs the check body (which controls its own loop count and any setup). + * 5. Captures malloc_count after. + * 6. Asserts the count is unchanged. + * + * A drop equal to the number of calls indicates one unmatched js_free per + * call — a malloc/js_malloc mismatch in the C layer. + */ +function checkContextForAllocatorBalance( + getModule: () => Promise, + checks: Record, +) { + let ctx: QuickJSContext + + beforeEach(async () => { + const wasmModule = await getModule() + ctx = wasmModule.newContext() + // Warm up: ensure all property-name atoms used by computeMemoryUsage() are + // already cached so they don't shift the baseline on our before/after reads. + Scope.withScope((s) => { + s.manage(ctx.runtime.computeMemoryUsage()) + }) + }) + + afterEach(() => { + ctx.dispose() + }) + + for (const [name, check] of Object.entries(checks)) { + it(`allocator balance: ${name}`, () => { + const countBefore = readMallocCount(ctx) + check(ctx) + const countAfter = readMallocCount(ctx) + + assert.strictEqual( + countAfter, + countBefore, + `malloc_count must not drift during "${name}": ` + + `was ${countBefore}, now ${countAfter} ` + + `(drop: ${countBefore - countAfter}). ` + + `A drop equal to the number of calls indicates one unmatched ` + + `js_free per call (likely a malloc/js_malloc mismatch in the C layer).`, + ) + }) + } +} + +// --------------------------------------------------------------------------- +// Test cases + +const checks: Record = { + getOwnPropertyNames(ctx) { + const obj = ctx.newObject() + ctx.setProp(obj, "a", ctx.true) + ctx.setProp(obj, "b", ctx.true) + ctx.setProp(obj, "c", ctx.true) + ctx.setProp(obj, "d", ctx.true) + ctx.setProp(obj, "e", ctx.true) + + for (let i = 0; i < 1_000; i++) { + const result = ctx.getOwnPropertyNames(obj, { strings: true }) + if (result.error) { + result.error.dispose() + } else { + result.value.dispose() + } + } + + obj.dispose() + }, +} + +describe("Allocator balance checks", () => { + describe("DEBUG sync module", () => { + const loader = memoizeNewModule(() => newQuickJSWASMModule(DEBUG_SYNC)) + checkContextForAllocatorBalance(loader, checks) + }) + + describe("RELEASE sync module", () => { + checkContextForAllocatorBalance(getQuickJS, checks) + }) + + describe.skip("DEBUG async module", () => { + const loader = memoizeNewModule(() => newQuickJSAsyncWASMModule(DEBUG_ASYNC)) + checkContextForAllocatorBalance(loader, checks) + }) + + describe("RELEASE async module", () => { + const loader = memoizeNewModule(() => newQuickJSAsyncWASMModule()) + checkContextForAllocatorBalance(loader, checks) + }) +})