Skip to content

Fix collectUnicodes() leak and throw when malloc() fails - #238

Open
dannyball710 wants to merge 2 commits into
harfbuzz:mainfrom
dannyball710:memory-fixes
Open

dannyball710 wants to merge 2 commits into
harfbuzz:mainfrom
dannyball710:memory-fixes

Conversation

@dannyball710

@dannyball710 dannyball710 commented Oct 13, 2026 •

Copy link
Copy Markdown

Two memory bugs I ran into in an application that shapes text with many user-supplied fonts in one process. One commit each, see the commit messages.

collectUnicodes() leak. typed_array_from_set() returned a HEAPU32 view of an array it never freed, so every call leaked 4 bytes per code point. With NotoSans-Regular.ttf (3053 code points), 20,000 calls grow RSS from 52 MiB to 291 MiB on main; with this change even 100,000 calls stay at 91 MiB. The view was also detached whenever the WASM memory grew, so a result kept across a later allocation came back empty. The new test fails on main with TypeError: Cannot perform %TypedArray%.prototype.values on a detached ArrayBuffer.

malloc() returning 0. The WASM memory cannot grow past 2 GiB, and once it is full malloc() returns 0, which every caller wrote through anyway. On main:

const data = new Uint8Array(64 * 1024 * 1024);
const blobs = [];
for (let i = 0; i < 64; i++) blobs.push(new hb.Blob(data)); // no error
hb.versionString(); // "" instead of "14.6.0"
hb.shape(font, buffer); // RuntimeError: memory access out of bounds

With real font data in the blobs the corruption can also take the process down later: making a Blob + Face + Font from 13 MB of font data 200 times in a row corrupts the memory at round 157, and when the dropped objects are finalized afterwards, a finalizer traps inside FinalizationRegistry.cleanupSome and the uncaught RuntimeError: memory access out of bounds exits Node.js with code 1.

With this change the 32nd Blob throws Error: Failed to allocate 67108864 bytes, and the module keeps working (versionString() is intact, shaping a small font still works, and finalization runs without errors). All eight exports.malloc() call sites now go through the helper. The test for this has to fill the WASM memory, so it lives in its own file to get its own module instance; it takes about 1.6 s.

Tested with make test and make doc (Emscripten 4.0.13, Node.js 22).

dannyball710 and others added 2 commits October 14, 2026 01:43
typed_array_from_set() returned a view of the array it allocated in the
WASM memory and never freed it, leaking 4 bytes per code point on every
call. The view was also detached as soon as the memory grew, after which
the returned array had length 0 and threw when iterated. Copy the result
out and free the array.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The WASM memory cannot grow past 2 GiB, and once it is full malloc()
returns 0. Every caller wrote through the returned pointer regardless:
a Blob that no longer fits copied its font data over the start of the
memory, overwriting the module's static data, and later calls returned
garbage or trapped with "memory access out of bounds". Route all
allocations through a helper that throws instead. In Face.getName(),
allocate before stackAlloc(), so that a failure cannot skip the
stackRestore().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lianghai

Copy link
Copy Markdown
Contributor

@dannyball710

Copy link
Copy Markdown
Author

Is this compiler setting relevant? https://emscripten.org/docs/tools_reference/settings_reference.html#aborting-malloc

It is related, but I don't think it can replace the check here.

harfbuzzjs builds with -s ALLOW_MEMORY_GROWTH, and with growth on Emscripten defaults ABORTING_MALLOC to 0, so malloc() returns 0 on failure. That is the case this PR handles: Blob then wrote the font data through the null pointer, over the start of the WASM memory.

-s ABORTING_MALLOC=1 would turn that into an abort, but for every failed malloc() in the module, not only the wrapper's. HarfBuzz itself checks its allocations and degrades gracefully (empty objects, a buffer marked as in error). An abort instead throws a RuntimeError from the middle of the C++ code, without unwinding it, so whatever HarfBuzz was updating is left half done and the instance can't be trusted afterwards.

The check in this PR only covers the allocations the JS wrapper makes itself, before anything has been written. The caller gets an ordinary Error, and the instance is still usable: it can free other objects and try again.

This branch has not been deployed

No deployments
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.

2 participants