From 0219ba3c88fc1241a609c6b969584cb74f0c4d41 Mon Sep 17 00:00:00 2001
From: Naveed Khan <dxbnaveed.k@gmail.com>
Date: Tue, 21 Jul 2026 22:00:47 +0530
Subject: [PATCH] make WrapDynaClassTest tolerate gc eviction of weak cache
 entries (1.X)

The per-classloader cache holds its CacheKey weakly and nothing else
references it, so any GC that runs between two createDynaClass calls can
evict the entry and the next caller legitimately builds a second
instance. testConcurrentCreateDynaClassReturnsSameInstance asserted
exactly one instance per round regardless, so a GC landing inside a
round failed it on CI even though the synchronized get/create/put from
PR 419 is correct. Reproducible without any concurrency: create, call
System.gc(), create again, and the second call returns a new instance.

Guard each round with a WeakReference canary: only assert the
single-instance property when the canary shows no GC ran during the
round, and skip rounds a GC invalidated. Apply the same guard to
testCreateDynaClassIsCached, which had the same sensitivity in a smaller
window. The test still fails on the unsynchronized pre-419 code and now
passes under a System.gc() hammer thread that previously made it fail
immediately with the same error as the CI run.

Signed-off-by: Naveed Khan <dxbnaveed.k@gmail.com>
---
 .../commons/beanutils/WrapDynaClassTest.java  | 24 ++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/test/java/org/apache/commons/beanutils/WrapDynaClassTest.java b/src/test/java/org/apache/commons/beanutils/WrapDynaClassTest.java
index 623d4dbef..5b7cca8c8 100644
--- a/src/test/java/org/apache/commons/beanutils/WrapDynaClassTest.java
+++ b/src/test/java/org/apache/commons/beanutils/WrapDynaClassTest.java
@@ -20,6 +20,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertSame;
 
+import java.lang.ref.WeakReference;
 import java.util.Collections;
 import java.util.IdentityHashMap;
 import java.util.Set;
@@ -56,6 +57,10 @@ public void setValue(final String value) {
      * The cache key is one bean class, so {@code createDynaClass} must hand back a single instance no matter how many threads race to populate the
      * per-classloader cache. With a plain {@code WeakHashMap} and an unsynchronized get/create/put sequence, concurrent callers could build and return
      * distinct instances for one key; this drives that race and fails if more than one instance escapes.
+     * <p>
+     * The cache holds its keys weakly and nothing else references them, so a GC that runs while a round is in flight may evict the freshly cached entry
+     * and a late thread then builds a second instance without any race. Each round therefore only asserts when a GC canary shows no collection happened
+     * during the round; rounds invalidated by a GC are skipped.
      */
     @Test
     void testConcurrentCreateDynaClassReturnsSameInstance() throws Exception {
@@ -65,6 +70,8 @@ void testConcurrentCreateDynaClassReturnsSameInstance() throws Exception {
         try {
             for (int r = 0; r < rounds; r++) {
                 WrapDynaClass.clear();
+                // Cleared only if a GC ran after this point, which is the only way a cache entry can disappear mid-round.
+                final WeakReference<Object> gcCanary = new WeakReference<>(new Object());
                 final CyclicBarrier barrier = new CyclicBarrier(threads);
                 final Set<WrapDynaClass> results = Collections.newSetFromMap(new IdentityHashMap<>());
                 final Future<?>[] futures = new Future<?>[threads];
@@ -81,6 +88,9 @@ void testConcurrentCreateDynaClassReturnsSameInstance() throws Exception {
                 for (final Future<?> f : futures) {
                     f.get();
                 }
+                if (results.size() > 1 && gcCanary.get() == null) {
+                    continue;
+                }
                 assertEquals(1, results.size(), "createDynaClass returned more than one instance for one key");
             }
         } finally {
@@ -90,13 +100,21 @@ void testConcurrentCreateDynaClassReturnsSameInstance() throws Exception {
     }
 
     /**
-     * The single-threaded cache contract: repeated calls for one bean class return the same instance until the cache is cleared.
+     * The single-threaded cache contract: repeated calls for one bean class return the same instance until the cache is cleared. A GC between the two
+     * calls may legitimately evict the weakly held cache entry, so such attempts are retried.
      */
     @Test
     void testCreateDynaClassIsCached() {
         WrapDynaClass.clear();
-        final WrapDynaClass first = WrapDynaClass.createDynaClass(ConcurrentBean.class);
-        assertSame(first, WrapDynaClass.createDynaClass(ConcurrentBean.class));
+        WeakReference<Object> gcCanary;
+        WrapDynaClass first;
+        WrapDynaClass second;
+        do {
+            gcCanary = new WeakReference<>(new Object());
+            first = WrapDynaClass.createDynaClass(ConcurrentBean.class);
+            second = WrapDynaClass.createDynaClass(ConcurrentBean.class);
+        } while (first != second && gcCanary.get() == null);
+        assertSame(first, second);
         WrapDynaClass.clear();
     }
 }