Repository navigation
Add [Symbol.dispose]() to free objects without waiting for GC - #240
Open
dannyball710 wants to merge 1 commit into
Open
dannyball710 wants to merge 1 commit into
dannyball710 wants to merge 1 commit into
Conversation
FinalizationRegistry callbacks only run on a later task, so a synchronous
loop that keeps creating large blobs and faces can fill the 2 GiB WASM
memory before any of them is freed. Give every class that owns a native
object (Blob, Face, Font, FontFuncs, Buffer, DrawFuncs, PaintFuncs) a
[Symbol.dispose]() method, so it can be freed right away, or with a
`using` declaration. The classes declare that they implement the built-in
Disposable interface, and the generated typings reference the
esnext.disposable lib, so consumers that target older libs still get its
declarations.
The method frees the native object now and unregisters it from the
registry, so the finalizer does not free it again. A second call does
nothing, and any later use of the object's `ptr` throws. This lives in a
new untrack() helper next to track(); DrawFuncs and PaintFuncs now
register through track() like the other classes.
Runtimes without Symbol.dispose (Safari at the time of writing) need a
polyfill such as `Symbol.dispose ??= Symbol("Symbol.dispose");` from the
TypeScript 5.2 release notes, and it must run before harfbuzzjs is
imported, since the method key is evaluated when the module is loaded.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: #239.
Adds
[Symbol.dispose]()to the classes that own a native object (Blob, Face, Font, FontFuncs, Buffer, DrawFuncs, PaintFuncs), so an object can be freed right away, or at the end of ausingscope, instead of when it is garbage collected. See the commit message for details.Disposableinterface, and the generated typings reference theesnext.disposablelib, so consumers on olderlibsettings still type-check.disposefrees the native object, unregisters it from theFinalizationRegistryso it is not freed twice, and does nothing when called again. Any later use of the object throws. The logic lives in a newuntrack()helper next totrack(). DrawFuncs and PaintFuncs now register throughtrack()too.Symbol.dispose(Safari) need the polyfill from the TypeScript 5.2 release notes, run before harfbuzzjs is imported. MIGRATING.md mentions it. A correction to my earlier comment: TypeScript'susinghelper does not fall back toSymbol.for("Symbol.dispose"). It throws whenSymbol.disposeis missing, so the polyfill is the way, as you said.This touches
track()insrc/helpers.tslike #238 does. The conflict is trivial (keep both), and I'll rebase whichever lands second.Tested with
make testandmake doc(Emscripten 4.0.13, Node.js 22; the nativeusingtest is skipped there because Node.js 22 can't parse it), and the test suite on Node.js 24, where all 115 tests pass.