Skip to content

Commit 55c414a

Browse files
committed
refactor: production-ready ES2021 FinalizationRegistry with PhantomReferences
Complete rewrite addressing all PR #2058 feedback from aardvark179: **Architecture Changes:** - Replace WeakReference with PhantomReference for correct GC semantics - Implement singleton FinalizationQueueManager with shared ReferenceQueue - Add Context integration for proper JavaScript execution timing - Thread-safe operations with ConcurrentHashMap and synchronized blocks **Key Improvements:** - Memory leak prevention with proper WeakReference usage in TokenKey - O(1) token-based unregistration with reverse index - Bounded cleanup queue (MAX_PENDING_CLEANUPS=10000) prevents OOM - Complete cleanupSome() implementation with callback override support - Comprehensive error handling with thread safety **New Components:** - FinalizationQueueManager.java - Singleton Cleaner-like infrastructure - FinalizationRegistryCleanupBehaviorTest.java - 10 cleanup behavior tests - FinalizationRegistryCrossRealmTest.java - 6 cross-realm scenarios **Test Coverage:** - 72+ tests across 5 test files - Cross-realm support validated - Internal implementation white-box testing - GC integration verified Ready for PR submission to Mozilla Rhino.
1 parent cbc348e commit 55c414a

File tree

10 files changed

+1168
-249
lines changed

10 files changed

+1168
-249
lines changed

rhino/src/main/java/org/mozilla/javascript/Context.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,13 @@ static final Context enter(Context cx, ContextFactory factory) {
520520
currentContext.set(cx);
521521
}
522522
++cx.enterCount;
523+
524+
// Process any pending finalization cleanups when Context becomes active
525+
// This ensures cleanups run even if they were enqueued while no Context was available
526+
if (cx.enterCount == 1) {
527+
FinalizationQueueManager.getInstance().processPendingCleanups(cx);
528+
}
529+
523530
return cx;
524531
}
525532

@@ -555,6 +562,10 @@ public void close() {
555562
}
556563

557564
private static void releaseContext(Context cx) {
565+
// Process any pending microtasks and finalization cleanups before releasing
566+
cx.processMicrotasks();
567+
cx.processFinalizationCleanups();
568+
558569
// do not use contextLocal.remove() here, as this might be much slower, when the same thread
559570
// creates a new context. See ContextThreadLocalBenchmark.
560571
currentContext.set(null);
@@ -2520,6 +2531,41 @@ public void processMicrotasks() {
25202531
} while (head != null);
25212532
}
25222533

2534+
/**
2535+
* Schedule a finalization cleanup task to run after microtasks. This is used by
2536+
* FinalizationRegistry to schedule cleanup callbacks. The finalization cleanup queue is not
2537+
* thread-safe.
2538+
*
2539+
* @param cleanup the cleanup task to schedule
2540+
*/
2541+
void scheduleFinalizationCleanup(Runnable cleanup) {
2542+
finalizationCleanups.add(cleanup);
2543+
}
2544+
2545+
/**
2546+
* Process all pending finalization cleanup tasks. This is called after microtasks to ensure
2547+
* proper execution order. Errors in cleanup callbacks are caught and logged but not propagated.
2548+
*/
2549+
void processFinalizationCleanups() {
2550+
Runnable cleanup;
2551+
int processed = 0;
2552+
// Process up to 100 cleanups to avoid blocking
2553+
while ((cleanup = finalizationCleanups.poll()) != null && processed < 100) {
2554+
try {
2555+
cleanup.run();
2556+
processed++;
2557+
} catch (Exception e) {
2558+
// Log but don't propagate errors from cleanup callbacks
2559+
if (hasFeature(Context.FEATURE_ENHANCED_JAVA_ACCESS)) {
2560+
e.printStackTrace();
2561+
}
2562+
}
2563+
}
2564+
2565+
// If we still have cleanups pending after processing 100,
2566+
// let them run in the next cycle to avoid blocking
2567+
}
2568+
25232569
/**
25242570
* Control whether to track unhandled promise rejections. If "track" is set to true, then the
25252571
* tracker returned by "getUnhandledPromiseTracker" must be periodically used to process the
@@ -2842,6 +2888,7 @@ public static boolean isCurrentContextStrict() {
28422888
private ClassLoader applicationClassLoader;
28432889
private UnaryOperator<Object> javaToJSONConverter;
28442890
private final ArrayDeque<Runnable> microtasks = new ArrayDeque<>();
2891+
private final ArrayDeque<Runnable> finalizationCleanups = new ArrayDeque<>();
28452892
private final UnhandledRejectionTracker unhandledPromises = new UnhandledRejectionTracker();
28462893

28472894
/** This is the list of names of objects forcing the creation of function activation records. */

0 commit comments

Comments
 (0)