Size asyncify buffer from max stack size so deep suspensions cannot abort (part of #274)

c/interface.c

@@ -416,12 +416,17 @@ JSValue *QTS_RuntimeComputeMemoryUsage(JSRuntime *rt, JSContext *ctx) {
 }
 
 OwnedHeapChar *QTS_RuntimeDumpMemoryUsage(JSRuntime *rt) {
-  char *result = malloc(sizeof(char) * 1024);
-  FILE *memfile = fmemopen(result, 1024, "w");
+  // The report grows with the number of object classes in use; a fixed-size buffer truncated it.
+  char *result = NULL;
+  size_t size = 0;
+  FILE *memfile = open_memstream(&result, &size);
+  if (!memfile) {
+    return NULL;
+  }
   JSMemoryUsage s;
   JS_ComputeMemoryUsage(rt, &s);
   JS_DumpMemoryUsage(memfile, &s, rt);
-  fclose(memfile);
+  fclose(memfile); // flushes and NUL-terminates `result`, which the caller frees with free()
   return result;
 }
 
@@ -442,8 +447,18 @@ int QTS_BuildIsSanitizeLeak() {
 }
 
 #ifdef QTS_ASYNCIFY
-EM_JS(void, set_asyncify_stack_size, (size_t size, size_t default_size), {
-  Asyncify.StackSize = size || default_size;
+// The asyncify buffer holds the saved locals of every suspended wasm frame. That needs more bytes
+// than the native stack depth QuickJS limits with JS_SetMaxStackSize: measured up to 1.9x for
+// recursive JS across release/debug and quickjs/quickjs-ng builds. If the buffer is smaller than
+// the QuickJS limit requires, a deep suspension aborts the module ("unreachable") instead of
+// throwing a catchable "stack overflow". Leave margin over the measured worst case.
+#define QTS_ASYNCIFY_STACK_FACTOR 3
+
+EM_JS(void, set_asyncify_stack_size, (size_t size, size_t default_size, size_t factor), {
+  size = size >>> 0;
+  default_size = default_size >>> 0;
+  // malloc'd once per suspension; cap well below the 2 GiB wasm heap limit.
+  Asyncify.StackSize = size ? Math.min(Math.max(size * factor, default_size), 0x40000000) : default_size;
 });
 #endif
 
@@ -453,7 +468,7 @@ EM_JS(void, set_asyncify_stack_size, (size_t size, size_t default_size), {
 
 void QTS_RuntimeSetMaxStackSize(JSRuntime *rt, size_t stack_size) {
 #ifdef QTS_ASYNCIFY
-  set_asyncify_stack_size(stack_size, QTS_ASYNCIFY_DEFAULT_STACK_SIZE);
+  set_asyncify_stack_size(stack_size, QTS_ASYNCIFY_DEFAULT_STACK_SIZE, QTS_ASYNCIFY_STACK_FACTOR);
 #endif
   JS_SetMaxStackSize(rt, stack_size);
 }
@@ -926,7 +941,16 @@ MaybeAsync(JSValue *) QTS_GetOwnPropertyNames(JSContext *ctx, JSValue ***out_ptr
     }
     return jsvalue_to_heap(JS_GetException(ctx));
   }
-  *out_ptrs = malloc(sizeof(JSValue) * total_props);
+  // One pointer per property (the entries are JSValue*, not JSValue). Never malloc(0): NULL is our failure signal.
+  *out_ptrs = malloc(sizeof(JSValue *) * (total_props > 0 ? total_props : 1));
+  if (*out_ptrs == NULL) {
+    for (uint32_t i = 0; i < total_props; i++) {
+      JS_FreeAtom(ctx, tab[i].atom);
+    }
+    js_free(ctx, tab);
+    JS_ThrowOutOfMemory(ctx);
+    return jsvalue_to_heap(JS_GetException(ctx));
+  }
   for (int i = 0; i < total_props; i++) {
     JSAtom atom = tab[i].atom;
 
@@ -962,14 +986,23 @@ MaybeAsync(JSValue *) QTS_GetOwnPropertyNames(JSContext *ctx, JSValue ***out_ptr
 }
 
 MaybeAsync(JSValue *) QTS_Call(JSContext *ctx, JSValueConst *func_obj, JSValueConst *this_obj, int argc, JSValueConst **argv_ptrs) {
-  // convert array of pointers to array of values
-  JSValueConst argv[argc];
+  // Convert array of pointers to array of values. This is heap allocated: a variable-length
+  // array of a very large argc overflows the wasm shadow stack and traps the whole module.
+  JSValueConst *argv = NULL;
+  if (argc > 0) {
+    argv = malloc(sizeof(JSValueConst) * (size_t)argc);
+    if (!argv) {
+      return jsvalue_to_heap(JS_ThrowOutOfMemory(ctx));
+    }
+  }
   int i;
   for (i = 0; i < argc; i++) {
     argv[i] = *(argv_ptrs[i]);
   }
 
-  return jsvalue_to_heap(JS_Call(ctx, *func_obj, *this_obj, argc, argv));
+  JSValue result = JS_Call(ctx, *func_obj, *this_obj, argc, argv);
+  free(argv);
+  return jsvalue_to_heap(result);
 }
 
 /**
@@ -1564,6 +1597,9 @@ JSValue *QTS_bjson_decode(JSContext *ctx, JSValueConst *data) {
   if (!buffer)
     return jsvalue_to_heap(JS_EXCEPTION);
 
-  JSValue value = JS_ReadObject(ctx, buffer, length, 0);
+  // Must accept references: QTS_bjson_encode writes them (JS_WRITE_OBJ_REFERENCE) for shared
+  // and cyclic objects, and without this flag decoding such data fails with
+  // "object references are not allowed". Bytecode and SharedArrayBuffer stay disallowed.
+  JSValue value = JS_ReadObject(ctx, buffer, length, JS_READ_OBJ_REFERENCE);
   return jsvalue_to_heap(value);
 }

packages/quickjs-emscripten-core/src/context.ts

@@ -12,7 +12,6 @@ import type {
   EitherFFI,
   UInt32Pointer,
   JSValuePointerPointerPointer,
-  JSVoidPointer,
   HostRefId,
 } from "@jitl/quickjs-ffi-types"
 import type { JSPromiseState } from "./deferred-promise"
@@ -127,12 +126,18 @@ 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 {
+    if (!ptr) {
+      // The C side returns NULL when it cannot allocate the heap copy of a JSValue.
+      throw new Error("Failed to allocate a JSValue handle (out of memory)")
+    }
     const dispose: typeof this.freeJSValue = extraDispose
       ? (val) => {
           extraDispose()
@@ -448,6 +453,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 +499,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 +818,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<Uint8Array> {
     this.runtime.assertOwned(handle)
@@ -815,9 +829,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<Uint8Array>(this.module.HEAPU8.slice(ptr, ptr + len))
+    } finally {
+      this.module._free(ptr)
+    }
   }
 
   /**
@@ -994,7 +1011,7 @@ export class QuickJSContext
     if (status < 0) {
       return undefined
     }
-    return this.uint32Out.value.typedArray[0]
+    return this.uint32Out.value.typedArray.value[0]
   }
 
   /**
@@ -1052,13 +1069,14 @@ 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),
       )
-      this.ffi.QTS_FreeVoidPointer(this.ctx.value, ptr as JSVoidPointer)
+      // The C side allocated this array with plain malloc, not js_malloc.
+      this.module._free(ptr)
       return this.success(createDisposableArray(handles))
     })
   }

packages/quickjs-emscripten-core/src/host-ref.test.ts

@@ -110,6 +110,27 @@ describe("HostRefMap", () => {
   })
 
   describe("ID recycling (freelist)", () => {
+    it("does not hand out an id twice if it is deleted twice while its group is non-empty", () => {
+      const map = new HostRefMap()
+      const id1 = map.put({ a: 1 })
+      map.put({ b: 2 }) // keeps id1's group alive
+      map.delete(id1)
+      assert.throws(() => map.delete(id1), QuickJSHostRefInvalid)
+      const reused = map.put({ c: 3 })
+      const fresh = map.put({ d: 4 })
+      assert.strictEqual(reused, id1)
+      assert.notStrictEqual(fresh, id1)
+    })
+
+    it("keeps FIFO order across compaction of the freelist", () => {
+      const map = new HostRefMap()
+      const ids = Array.from({ length: 5000 }, (_, i) => map.put({ i }))
+      ids.forEach((id) => map.delete(id))
+      const reused = ids.map((_, i) => map.put({ i }))
+      assert.deepStrictEqual(reused, ids)
+      assert.ok(!ids.includes(map.put({ fresh: true })), "falls back to new ids once drained")
+    })
+
     it("reuses deleted IDs", () => {
       const map = new HostRefMap()
       const id1 = map.put({ first: true })

packages/quickjs-emscripten-core/src/host-ref.ts

@@ -17,7 +17,9 @@ function getGroupId(id: HostRefId): number {
  */
 export class HostRefMap {
   private nextId = INT32_MIN
+  /** FIFO queue of ids to reuse: `freelist[freelistHead..]`. */
   private freelist: HostRefId[] = []
+  private freelistHead = 0
   private groups = new Map<number, Map<HostRefId, object>>()
 
   put(value: object): HostRefId {
@@ -62,7 +64,10 @@ export class HostRefMap {
       throw new QuickJSHostRefInvalid(`host reference id ${id} is not defined`)
     }
 
-    group.delete(id)
+    if (!group.delete(id)) {
+      // Recycling an already-deleted id again would hand it out twice.
+      throw new QuickJSHostRefInvalid(`host reference id ${id} is not defined`)
+    }
     if (group.size === 0) {
       this.groups.delete(groupId)
     }
@@ -71,8 +76,14 @@ export class HostRefMap {
   }
 
   private allocateId(): HostRefId {
-    if (this.freelist.length > 0) {
-      return this.freelist.shift()!
+    if (this.freelistHead < this.freelist.length) {
+      const id = this.freelist[this.freelistHead++]
+      // Drop consumed entries occasionally; Array#shift per allocation is O(n).
+      if (this.freelistHead >= 1024 && this.freelistHead * 2 >= this.freelist.length) {
+        this.freelist = this.freelist.slice(this.freelistHead)
+        this.freelistHead = 0
+      }
+      return id
     }
 
     if (this.nextId === INVALID_HOST_REF_ID) {