Skip to content

Host-side memory handling gaps found while probing #248 (malloc NULL, getOwnPropertyNames free, HostRefMap, asyncify unwind buffer) #274

Open

Description

@jackkav

Stack: #248 → #276 → #277 → (#278 | #279 → #280 → #281 → #282). Each PR body has a link to review just its own diff.

While stress-testing #248 (heap growth) I found four related problems. All reproduce identically with and without #248. Repros were run against the published 0.32.0 wasm variants with a locally built quickjs-emscripten-core; I could not build wasm locally, so C-side items are untested proposals.

  1. Unchecked _malloc in ModuleMemory (toPointerArray, newTypedArray, newHeapCharPointer, newHeapBufferPointer). When the 2 GiB heap is full _malloc returns 0 and the host then writes at address 0, corrupting the module (later calls fail with "memory access out of bounds"). Proposal: throw. (Done in the PR below.) The C side has the same pattern (fixed in Handle allocation failure in newContext, heapValueHandle, getOwnPropertyNames (part of #275) #280 for QTS_GetOwnPropertyNames and jsvalue_to_heap) (QTS_GetOwnPropertyNames malloc(...) unchecked at c/interface.c ~929; jsvalue_to_heap returns NULL and nothing checks it).
  2. getOwnPropertyNames frees a plain-malloc'd array with QTS_FreeVoidPointer (js_free), which decrements QuickJS's malloc_count/malloc_size, so memory accounting (and setMemoryLimit) drifts. Proposal: free with the module's _free. (Done in the PR below.) Also note that array is over-allocated using sizeof(JSValue) (8 on wasm32) for 4-byte pointers (C side, not changed).
  3. HostRefMap freelist uses Array#shift: draining N freed ids is O(N²) (11s at 200k ids in a pure-JS repro). Also deleting an already-deleted id in a non-empty group queued it twice, so it was handed out twice. Proposal: head-index queue; throw on double delete. (Done in the PR below.)
  4. Asyncify unwind buffer is fixed at 80 KB (ASYNCIFY_STACK_SIZE=81920): suspending an async host function at moderate guest recursion depth crashes the module, and a crashed module stays dead. Partly addressed in Size asyncify buffer from max stack size so deep suspensions cannot abort (part of #274) #282: needs a build-flag change (or a clearer error) and a wasm build to verify. Repro: function f(d){return d===0?hostSleep():f(d-1)+0} with an asyncified hostSleep, increasing depth.

PR for items 1-3: #277 (stacked on #276 and #248). Item 4: fixed for explicit setMaxStackSize in #282 (stacked on #281); the default-size question is raised there.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions