Complete FinalizationRegistry#2432
Draft
gbrail wants to merge 6 commits into
Draft
Conversation
…compliance Complete rewrite using simplified, Android-compatible approach. Changes: - NativeWeakRef: ES2021-compliant WeakRef implementation - NativeFinalizationRegistry: ES2021-compliant implementation - ScriptRuntime: Register both constructors in ES6+ block - Messages.properties: Add validation error messages - test262.properties: All 76 tests passing ES2021 Standard Compliance: - Correctly allows unregistered symbols as weak targets - Rejects registered symbols created with Symbol.for per spec - Validates target/heldValue/token same-value constraints - Proper Symbol.toStringTag on prototypes - Error messages match spec requirements Technical Implementation: - Uses Java WeakReference for WeakRef, simple and direct - Uses PhantomReference + ReferenceQueue for FinalizationRegistry - No Cleaner API dependency, not available on Android - No Context.java modifications needed - Thread-safe with ConcurrentHashMap - Cleanup callbacks processed opportunistically during register calls - Follows modern LambdaConstructor pattern like NativeWeakMap Benefits: - Only 2 files instead of 5 - Self-contained, no helper classes or separate queue management - Android-compatible using Java 8 APIs only - Addresses reviewer feedback to use simpler, standard Java APIs
Based on the starting point, ensure that finalization callbacks always happen in the context where they were originally registered as part of microtask processing.
Contributor
|
I'll review this properly later, I need to make sure I underhand the graph of references and what that implies about lifetimes. Specifically I'm worried that we're keeping |
Collaborator
Author
|
It would be cool to do a formal analysis. On that end, it probably makes
sense, for example, for the item on the referencequeue to have a weak
reference to the finalization registry in case the microtasks don't run
after the registry goes out of scope.
…On Mon, Jun 15, 2026 at 4:34 AM Duncan MacGregor ***@***.***> wrote:
*aardvark179* left a comment (mozilla/rhino#2432)
<#2432 (comment)>
I'll review this properly later, I need to make sure I underhand the graph
of references and what that implies about lifetimes. Specifically I'm
worried that we're keeping FinalisationRegistry objects alive when they
should not remain so.
—
Reply to this email directly, view it on GitHub
<#2432?email_source=notifications&email_token=AAD7I2YKAKSYP52R3KAHZRT477NM3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZQG42DSMZYGM32M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4707493837>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD7I22D4RBQPXYKVLIYWCL477NM3AVCNFSNUABDKJSXA33TNF2G64TZHMYTOMZZG43DOO2JONZXKZJ3GQ3DMMRSHA4DONJYUF3AE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
This prevents one possible form of memory leak, where finalizations are never run and references pile up.
Collaborator
Author
|
I have a bunch more ideas on this still, working... |
By default, finalization callbacks will not be called, and nothing will be enqueued on a ReferenceQueue, although the API will continue to function and appear to "register" and "unregister" callbacks to match the spec. If finalization is enabled, embedders of Rhino must somehow ensure that "processMicrotasks" is periodically called to prevent a too-large ReferenceQueue. How this is done depends on the framework in question, although microtask processing happens automatically when Promises are used.
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
3 participants
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.
This builds on the work already done in #2058
and #2431 to complete the implementation
of the FinalizationRegistry.
The all-important reference queue where finalization callbacks are called is
invoked as part of microtask processing by the Context. Applications that have
many Contexts in use at once will see finalizations happening in the one where
the registry was originally created. It uses PhantomReference and a
reference queue to ensure that finalizations happen when the JVM needs them
to happen.