GROOVY-12259: Reap orphaned SwitchPoint registry entries when their domain is collected

The live-SwitchPoint registry held its owning invalidator strongly, so a
domain discarded without ever being invalidated (a dead script class's
MetaClass/ClassInfo in a long-lived, high-churn process that never uses
categories) left its entry behind forever: no per-domain event fires for
a dead class and no bulk drain runs without category/custom-MetaClass
activity.

Registry values are now weak references to the owner, delivered to a
ReferenceQueue and reaped opportunistically from getSwitchPoint() and
drainLive() — the operations churn-heavy processes keep performing —
while bulk drains also claim not-yet-reaped orphans directly. Orphans
are always invalidated, never silently dropped: installed guard chains
embed only the SwitchPoint's internal invoker, which does not keep the
SwitchPoint object reachable, so the strong registry key is what keeps
a possibly-still-installed orphan guard retirable, and invalidating it
just forces straggler sites to re-link. This preserves the invariant
that an empty registry proves there was nothing to retire, and also
closes a latent staleness window where sites guarded on a collected
MetaClass's domain could no longer be reached by class-level
invalidation. Detach paths clear the owner reference so explicitly
handled entries never surface on the queue. Stable processes pay one
weak reference per domain and an empty queue poll on the link path.

Author: paulk-asert

src/main/java/org/apache/groovy/runtime/indy/SwitchPointInvalidator.java

@@ -19,6 +19,9 @@
 package org.apache.groovy.runtime.indy;
 
 import java.lang.invoke.SwitchPoint;
+import java.lang.ref.Reference;
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
 import java.util.List;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -47,6 +50,17 @@
  * guard (GROOVY-12258, GROOVY-12259). Registration precedes publication, so
  * an empty (or entry-less) observation proves no guard chain holds a live
  * SwitchPoint that the observer could have needed to retire.
+ * <p>
+ * Registry entries hold their owning invalidator only weakly. When an owner
+ * is collected without ever detaching (a discarded MetaClass or ClassInfo
+ * domain in a long-lived, high-script-churn process that never bulk
+ * invalidates), the orphaned SwitchPoint is <em>invalidated</em> — never
+ * silently forgotten — either by the reaper pumped from allocation and drain,
+ * or by the next bulk drain claiming it directly. Installed guards embed only
+ * the SwitchPoint's internal invoker, which does not keep the SwitchPoint
+ * object reachable, so the strong registry key is what keeps a
+ * possibly-still-installed orphan guard retirable; invalidating it merely
+ * forces straggler sites to re-link.
  *
  * @since 6.0.0
  */
@@ -61,20 +75,49 @@ public final class SwitchPointInvalidator {
     private static final SwitchPoint[] SINGLE_INVALIDATE_BUF = new SwitchPoint[1];
 
     /**
-     * Registry of all live SwitchPoints, each mapped to its owning invalidator.
-     * Keyed by SwitchPoint — each has a single-use lifecycle (allocated once,
-     * detached once), so a removal can never clobber a successor's entry the
-     * way an invalidator-keyed registry could (ABA on re-allocation).
+     * Registry of all live SwitchPoints, each mapped to a weak reference to
+     * its owning invalidator. Keyed by SwitchPoint — each has a single-use
+     * lifecycle (allocated once, detached once), so a removal can never
+     * clobber a successor's entry the way an invalidator-keyed registry could
+     * (ABA on re-allocation). Keys are strong on purpose: a guard chain keeps
+     * only the SwitchPoint's internal invoker alive, so this entry is what
+     * keeps an orphaned-but-installed guard retirable. Values are weak so a
+     * dead domain's invalidator is collectable; {@link #reapOrphans()} then
+     * invalidates (never just drops) the orphaned SwitchPoint.
      * <p>
      * Invariant: any SwitchPoint published in {@link #current} has an entry
      * here, established by registering <em>before</em> the publishing CAS.
      * Entries are removed only by the party that detaches the SwitchPoint
-     * (or by the allocator when its publishing CAS loses), so an entry whose
-     * SwitchPoint is not (yet) current is transient and self-resolving.
+     * (or by the allocator when its publishing CAS loses, or by the party
+     * that invalidates an orphan), so an entry whose SwitchPoint is not
+     * (yet) current is transient and self-resolving.
      */
-    private static final ConcurrentHashMap<SwitchPoint, SwitchPointInvalidator> LIVE =
+    private static final ConcurrentHashMap<SwitchPoint, OwnerRef> LIVE =
             new ConcurrentHashMap<>();
 
+    /**
+     * Delivery queue for {@link OwnerRef}s whose invalidator was collected
+     * while its SwitchPoint was still registered. Drained opportunistically
+     * by {@link #reapOrphans()}; empty (and cost-free) unless domains die
+     * undetached.
+     */
+    private static final ReferenceQueue<SwitchPointInvalidator> ORPHANS = new ReferenceQueue<>();
+
+    /**
+     * Weak reference from a registered SwitchPoint to its owning invalidator.
+     * Detach paths {@link Reference#clear() clear} the reference, which keeps
+     * an explicitly handled entry from ever being enqueued; only owners that
+     * die undetached reach {@link #ORPHANS}.
+     */
+    private static final class OwnerRef extends WeakReference<SwitchPointInvalidator> {
+        final SwitchPoint sp;
+
+        OwnerRef(final SwitchPoint sp, final SwitchPointInvalidator owner) {
+            super(owner, ORPHANS);
+            this.sp = sp;
+        }
+    }
+
     /** {@code null} means no live switch point (lazy allocation on next get). */
     private final AtomicReference<SwitchPoint> current = new AtomicReference<>();
     private final AtomicInteger retirementCount = new AtomicInteger();
@@ -94,20 +137,25 @@ public SwitchPointInvalidator() {
      * @return a live switch point (never {@code null})
      */
     public SwitchPoint getSwitchPoint() {
+        // Allocation is the operation churn-heavy processes keep performing,
+        // so it doubles as the reaper pump; a no-op while the queue is empty.
+        reapOrphans();
         for (;;) {
             SwitchPoint sp = current.get();
             if (sp != null) {
                 return sp;
             }
             SwitchPoint created = new SwitchPoint();
+            OwnerRef ref = new OwnerRef(created, this);
             // Register before publish: a bulk path that finds no entry is then
             // guaranteed the SwitchPoint was not visible to any guard, so
             // skipping it is safe.
-            LIVE.put(created, this);
+            LIVE.put(created, ref);
             if (current.compareAndSet(null, created)) {
                 return created;
             }
-            LIVE.remove(created); // lost the publishing race; never live
+            ref.clear();          // lost the publishing race; never live
+            LIVE.remove(created);
         }
     }
 
@@ -130,7 +178,7 @@ public void invalidate() {
     public SwitchPoint detachLive() {
         SwitchPoint sp = current.getAndSet(null);
         if (sp != null) {
-            LIVE.remove(sp);
+            deregister(sp);
             retirementCount.incrementAndGet();
         }
         return sp;
@@ -148,13 +196,24 @@ public SwitchPoint detachLive() {
      */
     boolean detachIfCurrent(final SwitchPoint sp) {
         if (current.compareAndSet(sp, null)) {
-            LIVE.remove(sp);
+            deregister(sp);
             retirementCount.incrementAndGet();
             return true;
         }
         return false;
     }
 
+    /**
+     * Removes {@code sp}'s registry entry and clears its owner reference so an
+     * explicitly detached SwitchPoint can never surface on {@link #ORPHANS}.
+     */
+    private static void deregister(final SwitchPoint sp) {
+        OwnerRef ref = LIVE.remove(sp);
+        if (ref != null) {
+            ref.clear();
+        }
+    }
+
     /**
      * Returns how many live switch points have been retired.
      *
@@ -186,21 +245,50 @@ static boolean hasLiveSwitchPoints() {
      * Entries whose SwitchPoint is no longer (or not yet) current are left in
      * place: they are either about to be published (removing them would strand
      * a live SwitchPoint unregistered, invisible to all future drains) or are
-     * being removed by their detaching owner. The weakly consistent iteration
+     * being removed by their detaching owner. Entries whose owner has been
+     * collected are claimed here directly (the reaper may not have run yet)
+     * and retired with the rest of the batch. The weakly consistent iteration
      * may miss a SwitchPoint published mid-drain — same window as the previous
      * all-classes walk; sites linking concurrently read the current category
      * state at link time.
      *
      * @param out destination list (must not be {@code null})
      */
     static void drainLive(final List<SwitchPoint> out) {
-        LIVE.forEach((sp, inv) -> {
-            if (inv.detachIfCurrent(sp)) {
+        reapOrphans();
+        LIVE.forEach((sp, ref) -> {
+            SwitchPointInvalidator inv = ref.get();
+            if (inv == null) {
+                // Owner died undetached; exactly one claimant retires the orphan.
+                if (LIVE.remove(sp, ref)) {
+                    out.add(sp);
+                }
+            } else if (inv.detachIfCurrent(sp)) {
                 out.add(sp);
             }
         });
     }
 
+    /**
+     * Retires SwitchPoints whose owning invalidator was collected while they
+     * were still registered (e.g. discarded script-class domains in processes
+     * that never bulk invalidate). Orphans are removed from the registry and
+     * <em>invalidated</em>: an installed guard chain does not keep the
+     * SwitchPoint object reachable, so a straggler site may still be guarding
+     * on it — invalidation forces such sites to re-link, which is always safe.
+     * No-op (a queue poll) unless owners have died undetached.
+     */
+    private static void reapOrphans() {
+        for (Reference<? extends SwitchPointInvalidator> r; (r = ORPHANS.poll()) != null; ) {
+            OwnerRef ref = (OwnerRef) r;
+            // Two-arg remove: lose gracefully to a concurrent drain claiming
+            // the same orphan; exactly one party invalidates.
+            if (LIVE.remove(ref.sp, ref)) {
+                invalidateIfLive(ref.sp);
+            }
+        }
+    }
+
     /**
      * Number of currently registered live SwitchPoints (tests).
      *
@@ -220,6 +308,22 @@ static boolean isRegistered(final SwitchPoint sp) {
         return LIVE.containsKey(sp);
     }
 
+    /**
+     * Clears {@code sp}'s owner reference while leaving its entry registered,
+     * simulating an owner collected before queue delivery (tests).
+     *
+     * @param sp registered SwitchPoint
+     * @return {@code true} if an entry was found and its reference cleared
+     */
+    static boolean clearOwnerRefForTesting(final SwitchPoint sp) {
+        OwnerRef ref = LIVE.get(sp);
+        if (ref != null) {
+            ref.clear();
+            return true;
+        }
+        return false;
+    }
+
     /**
      * Invalidates {@code sp} when non-null and still valid.
      *

src/test/groovy/org/apache/groovy/runtime/indy/SwitchPointInvalidatorTest.groovy

@@ -289,6 +289,47 @@ final class SwitchPointInvalidatorTest {
         }
     }
 
+    @Test
+    void drain_claimsOrphanWhoseOwnerWasCollected() {
+        def inv = new SwitchPointInvalidator()
+        SwitchPoint orphan = inv.switchPoint
+        // simulate the owner dying undetached, before queue delivery
+        assertTrue(SwitchPointInvalidator.clearOwnerRefForTesting(orphan))
+        def batch = []
+        SwitchPointInvalidator.drainLive(batch)
+        assertTrue(batch.contains(orphan), 'drain must claim an orphaned SwitchPoint')
+        assertFalse(SwitchPointInvalidator.isRegistered(orphan))
+        SwitchPoint.invalidateAll(batch as SwitchPoint[])
+        def batch2 = []
+        SwitchPointInvalidator.drainLive(batch2)
+        assertFalse(batch2.contains(orphan), 'a claimed orphan must not be re-drained')
+        if (batch2) SwitchPoint.invalidateAll(batch2 as SwitchPoint[])
+        inv.detachLive() // test-only owner is actually alive; clear its stale current
+    }
+
+    @Test
+    void reaper_invalidatesOrphanedSwitchPointAfterOwnerGc() {
+        SwitchPoint orphan = createOrphan()
+        assertTrue(SwitchPointInvalidator.isRegistered(orphan))
+        def pump = new SwitchPointInvalidator()
+        boolean reaped = false
+        for (int i = 0; i < 200 && !reaped; i++) {
+            System.gc()
+            pump.invalidate()   // detach so the next get re-enters allocation
+            pump.switchPoint    // allocation pumps the reaper
+            reaped = !SwitchPointInvalidator.isRegistered(orphan)
+            if (!reaped) Thread.sleep(10)
+        }
+        pump.invalidate()
+        assertTrue(reaped, 'orphaned SwitchPoint was never reaped from the registry')
+        assertTrue(orphan.hasBeenInvalidated(), 'reaper must invalidate, not merely forget')
+    }
+
+    /** The invalidator allocated here is unreachable once this returns. */
+    private static SwitchPoint createOrphan() {
+        new SwitchPointInvalidator().switchPoint
+    }
+
     @Test
     void concurrentGet_retryOnCasLoss() {
         // Hammer getSwitchPoint from many threads so some lose the CAS and retry.