diff --git a/packages/quickjs-emscripten-core/src/context.ts b/packages/quickjs-emscripten-core/src/context.ts index 9bc7c8a0d..b5010ddd2 100644 --- a/packages/quickjs-emscripten-core/src/context.ts +++ b/packages/quickjs-emscripten-core/src/context.ts @@ -127,9 +127,11 @@ class ContextMemory extends ModuleMemory implements Disposable { } consumeJSCharPointer(ptr: JSBorrowedCharPointer): string { - const str = this.module.UTF8ToString(ptr) - this.ffi.QTS_FreeCString(this.ctx.value, ptr) - return str + try { + return this.module.UTF8ToString(ptr) + } finally { + this.ffi.QTS_FreeCString(this.ctx.value, ptr) + } } heapValueHandle(ptr: JSValuePointer, extraDispose?: () => void): JSValue { @@ -448,6 +450,13 @@ export class QuickJSContext const array = new Uint8Array(buffer) const handle = this.memory.newHeapBufferPointer(array) const ptr = this.ffi.QTS_NewArrayBuffer(this.ctx.value, handle.value.pointer, array.length) + const errorPtr = this.ffi.QTS_ResolveException(this.ctx.value, ptr) + if (errorPtr) { + // QuickJS only takes ownership of the host buffer when it succeeds in creating the ArrayBuffer. + handle.dispose() + this.ffi.QTS_FreeValuePointer(this.ctx.value, ptr) + return this.unwrapResult(this.fail(this.memory.heapValueHandle(errorPtr))) + } return this.memory.heapValueHandle(ptr) } @@ -487,9 +496,9 @@ export class QuickJSContext mutablePointerArray.value.ptr, ) const promiseHandle = this.memory.heapValueHandle(promisePtr) - const [resolveHandle, rejectHandle] = Array.from(mutablePointerArray.value.typedArray).map( - (jsvaluePtr) => this.memory.heapValueHandle(jsvaluePtr as any), - ) + const [resolveHandle, rejectHandle] = Array.from( + mutablePointerArray.value.typedArray.value, + ).map((jsvaluePtr) => this.memory.heapValueHandle(jsvaluePtr as any)) return new QuickJSDeferredPromise({ context: this, promiseHandle, @@ -806,7 +815,9 @@ export class QuickJSContext } /** - * Coverts `handle` to a JavaScript ArrayBuffer + * Coverts `handle` to a JavaScript ArrayBuffer. + * + * The returned bytes are a copy that stays valid however much the WASM heap grows afterwards. */ getArrayBuffer(handle: QuickJSHandle): Lifetime { this.runtime.assertOwned(handle) @@ -815,9 +826,12 @@ export class QuickJSContext if (!ptr) { throw new Error("Couldn't allocate memory to get ArrayBuffer") } - return new Lifetime(this.module.HEAPU8.subarray(ptr, ptr + len), undefined, () => - this.module._free(ptr), - ) + try { + // A view into the heap would be detached by the next memory growth, so copy out. + return new Lifetime(this.module.HEAPU8.slice(ptr, ptr + len)) + } finally { + this.module._free(ptr) + } } /** @@ -994,7 +1008,7 @@ export class QuickJSContext if (status < 0) { return undefined } - return this.uint32Out.value.typedArray[0] + return this.uint32Out.value.typedArray.value[0] } /** @@ -1052,9 +1066,9 @@ export class QuickJSContext if (errorPtr) { return this.fail(this.memory.heapValueHandle(errorPtr)) } - const len = this.uint32Out.value.typedArray[0] - const ptr = outPtr.value.typedArray[0] - const pointerArray = new Uint32Array(this.module.HEAP8.buffer, ptr, len) + const len = this.uint32Out.value.typedArray.value[0] + const ptr = outPtr.value.typedArray.value[0] + const pointerArray = new Uint32Array(this.module.HEAPU8.buffer, ptr, len) const handles = Array.from(pointerArray).map((ptr) => this.memory.heapValueHandle(ptr as JSValuePointer), ) diff --git a/packages/quickjs-emscripten-core/src/memory.ts b/packages/quickjs-emscripten-core/src/memory.ts index 5c6417651..d5bb54054 100644 --- a/packages/quickjs-emscripten-core/src/memory.ts +++ b/packages/quickjs-emscripten-core/src/memory.ts @@ -29,9 +29,39 @@ export interface TypedArrayConstructor { BYTES_PER_ELEMENT: number } +/** + * A TypedArray view into WASM memory that automatically refreshes when memory grows. + * + * When Emscripten's WASM memory grows (due to -sALLOW_MEMORY_GROWTH), all existing + * TypedArray views become detached because the underlying ArrayBuffer is replaced. + * This class lazily recreates the view when the buffer changes. + * + * @private + */ +export class RefreshableTypedArray { + private cachedArray: T | undefined + private lastBuffer: ArrayBufferLike | undefined + + constructor( + private readonly module: EitherModule, + private readonly kind: TypedArrayConstructor, + private readonly ptr: number, + private readonly length: number, + ) {} + + get value(): T { + const currentBuffer = this.module.HEAPU8.buffer + if (this.cachedArray === undefined || this.lastBuffer !== currentBuffer) { + this.cachedArray = new this.kind(currentBuffer, this.ptr, this.length) + this.lastBuffer = currentBuffer + } + return this.cachedArray + } +} + /** @private */ export type HeapTypedArray = Lifetime<{ - typedArray: JS + typedArray: RefreshableTypedArray ptr: C }> @@ -54,18 +84,17 @@ export class ModuleMemory { kind: TypedArrayConstructor, length: number, ): HeapTypedArray { - const zeros = new kind(new Array(length).fill(0)) - const numBytes = zeros.length * zeros.BYTES_PER_ELEMENT + const numBytes = length * kind.BYTES_PER_ELEMENT const ptr = this.module._malloc(numBytes) as C - const typedArray = new kind(this.module.HEAPU8.buffer, ptr, length) - typedArray.set(zeros) + const typedArray = new RefreshableTypedArray(this.module, kind, ptr, length) + typedArray.value.fill(0) return new Lifetime({ typedArray, ptr }, undefined, (value) => this.module._free(value.ptr)) } // TODO: shouldn't this be Uint32 instead of Int32? newMutablePointerArray( length: number, - ): Lifetime<{ typedArray: Int32Array; ptr: T }> { + ): Lifetime<{ typedArray: RefreshableTypedArray>; ptr: T }> { return this.newTypedArray(Int32Array, length) } @@ -87,8 +116,10 @@ export class ModuleMemory { } consumeHeapCharPointer(ptr: OwnedHeapCharPointer): string { - const str = this.module.UTF8ToString(ptr) - this.module._free(ptr) - return str + try { + return this.module.UTF8ToString(ptr) + } finally { + this.module._free(ptr) + } } } diff --git a/packages/quickjs-emscripten-core/src/runtime.ts b/packages/quickjs-emscripten-core/src/runtime.ts index e404f0648..061838129 100644 --- a/packages/quickjs-emscripten-core/src/runtime.ts +++ b/packages/quickjs-emscripten-core/src/runtime.ts @@ -251,7 +251,7 @@ export class QuickJSRuntime extends UsingDisposable implements Disposable { ctxPtrOut.value.ptr, ) - const ctxPtr = ctxPtrOut.value.typedArray[0] as JSContextPointer + const ctxPtr = ctxPtrOut.value.typedArray.value[0] as JSContextPointer ctxPtrOut.dispose() if (ctxPtr === 0) { // No jobs executed. diff --git a/packages/quickjs-emscripten/src/quickjs.test.ts b/packages/quickjs-emscripten/src/quickjs.test.ts index f603c256c..d2cdfeab4 100644 --- a/packages/quickjs-emscripten/src/quickjs.test.ts +++ b/packages/quickjs-emscripten/src/quickjs.test.ts @@ -1191,6 +1191,168 @@ export default "the default"; }) }) + describe("WASM memory growth", () => { + // Growing the WASM heap replaces `module.HEAPU8.buffer` and detaches every + // TypedArray view created earlier. Reading a detached view yields `undefined`. + // These tests assert our out-parameter handling reads through the *current* heap. + const getModule = () => + (vm as any).module as { + HEAPU8: Uint8Array + _malloc(n: number): number + _free(p: number): void + } + + /** Force the heap to grow now. Throws if it didn't, so the tests below can't pass vacuously. */ + function growHeap() { + const module = getModule() + const before = module.HEAPU8.buffer + // Blocks that fit in existing free space don't grow the heap, so double the size until one doesn't. + // (Compare with `!==`: assert.notStrictEqual would try to print the whole heap on failure.) + for (let mb = 16; module.HEAPU8.buffer === before; mb *= 2) { + assert.ok(mb <= 512, "heap did not grow") + module._free(module._malloc(mb * 1024 * 1024)) + } + } + + it("newTypedArray reads current heap contents after growth", () => { + const memory = (vm as any).memory as any + using array = memory.newMutablePointerArray(2) + const staleView = array.value.typedArray.value + growHeap() + assert.equal(staleView.length, 0, "precondition: the old view is detached") + + const module = getModule() + new Int32Array(module.HEAPU8.buffer, array.value.ptr, 2).set([1234, 5678]) + assert.deepEqual(Array.from(array.value.typedArray.value), [1234, 5678]) + }) + + it("newTypedArray zero-initializes", () => { + const memory = (vm as any).memory as any + // Dirty the heap first so zeroing is observable. + const module = getModule() + const dirty = module._malloc(64) + module.HEAPU8.fill(0xff, dirty, dirty + 64) + module._free(dirty) + using array = memory.newMutablePointerArray(8) + assert.deepEqual(Array.from(array.value.typedArray.value), new Array(8).fill(0)) + }) + + it(".getLength() after the heap grew since the context was created", () => { + growHeap() + using array = vm.unwrapResult(vm.evalCode(`new Array(1234).fill(0)`)) + assert.strictEqual(vm.getLength(array), 1234) + }) + + it(".getOwnPropertyNames() after the heap grew, with many keys", () => { + growHeap() + const count = 50_000 + using object = vm.unwrapResult( + vm.evalCode(`const o = {}; for (let i = 0; i < ${count}; i++) o["k" + i] = i; o`), + ) + using names = vm.unwrapResult(vm.getOwnPropertyNames(object)) + assert.strictEqual(names.length, count) + assert.strictEqual(vm.getString(names[0]), "k0") + assert.strictEqual(vm.getString(names[count - 1]), `k${count - 1}`) + }) + + it(".getArrayBuffer() result is still readable after the heap grew", () => { + using handle = vm.unwrapResult(vm.evalCode(`new Uint8Array([1, 2, 3, 4]).buffer`)) + using bytes = vm.getArrayBuffer(handle) + growHeap() + assert.deepEqual(Array.from(bytes.value), [1, 2, 3, 4]) + }) + + it(".executePendingJobs() when a job grows the heap", () => { + const module = getModule() + const bufferBefore = module.HEAPU8.buffer + // The heap may already be large from earlier tests, so the job allocates until the host + // sees the heap grow, rather than assuming a fixed amount is enough. + using heapGrew = vm.newFunction("heapGrew", () => + vm.newNumber(module.HEAPU8.buffer !== bufferBefore ? 1 : 0), + ) + vm.setProp(vm.global, "heapGrew", heapGrew) + vm.unwrapResult( + vm.evalCode(`Promise.resolve().then(() => { + const kept = [] + for (let n = 0; n < 1000; n++) { + kept.push(new ArrayBuffer(4 * 1024 * 1024)) + if (heapGrew()) return + } + throw new Error("heap never grew") + })`), + ).dispose() + const result = vm.runtime.executePendingJobs() + assert.ok(module.HEAPU8.buffer !== bufferBefore, "precondition: the job grew the heap") + // Throws if the job threw (e.g. "heap never grew"). + assert.strictEqual(vm.unwrapResult(result), 1) + // The job ran in `vm`; a stale ctxPtr would have created a second, leaked context. + assert.strictEqual((vm.runtime as any).contextMap.size, 1) + }) + }) + + describe("failure paths free host memory", () => { + /** Record every pointer malloc'd and freed while `fn` runs. */ + function trackHeap(fn: () => T): { + result: T | undefined + error: unknown + leaked: number[] + } { + const module = (vm as any).module + const { _malloc, _free } = module + const live = new Set() + module._malloc = (n: number) => { + const ptr = _malloc(n) + live.add(ptr) + return ptr + } + module._free = (ptr: number) => { + live.delete(ptr) + return _free(ptr) + } + try { + return { result: fn(), error: undefined, leaked: [...live] } + } catch (error) { + return { result: undefined, error, leaked: [...live] } + } finally { + module._malloc = _malloc + module._free = _free + live.forEach((ptr) => _free(ptr)) + } + } + + it(".getString() frees the C string if decoding throws", () => { + using handle = vm.newString("hello") + const module = (vm as any).module + const UTF8ToString = module.UTF8ToString + const ffi = (vm as any).ffi + const QTS_FreeCString = ffi.QTS_FreeCString + let freed = 0 + ffi.QTS_FreeCString = (...args: unknown[]) => { + freed++ + return QTS_FreeCString.apply(ffi, args) + } + module.UTF8ToString = () => { + throw new Error("decode failed") + } + try { + assert.throws(() => vm.getString(handle), /decode failed/) + } finally { + module.UTF8ToString = UTF8ToString + ffi.QTS_FreeCString = QTS_FreeCString + } + assert.strictEqual(freed, 1) + }) + + it(".newArrayBuffer() frees the host buffer and throws if QuickJS cannot create it", () => { + // With no headroom, allocating the ArrayBuffer object fails. + vm.runtime.setMemoryLimit(1) + const { error, leaked } = trackHeap(() => vm.newArrayBuffer(new Uint8Array([1, 2, 3]).buffer)) + vm.runtime.setMemoryLimit(-1) + assert.ok(error, "expected newArrayBuffer to throw") + assert.deepEqual(leaked, []) + }) + }) + describe("memory pressure", () => { it("can pass a large string to a C function", async () => { const jsonString = await fs.promises.readFile(