Complete FinalizationRegistry#2432
Open
gbrail wants to merge 2 commits into
Open
Conversation
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:
***@***.***>
|
Collaborator
Author
|
I have a bunch more ideas on this still, working... |
…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
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.
gbrail
force-pushed
the
greg-finalization-registry
branch
from
3978ffd to
672c2e8
Compare
Collaborator
Author
|
I think this is ready for others to look at now. By default, the API is enabled but objects get garbage collected normally. "cx.setFinalizationEnabled" must be called before anything will be actually enqueued to be finalized. Finalization callbacks happen in the "processMicrotasks" method (which is called at the exit of every top-level function and script) but if someone is using Rhino in an environment where that never gets called, registered objects will pile up and possibly cause memory problems unless the queue is periodically cleaned. |
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.