This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch 1.X
in repository https://gitbox.apache.org/repos/asf/commons-beanutils.git
The following commit(s) were added to refs/heads/1.X by this push:
new 9541e18d Make WrapDynaClassTest tolerate gc eviction of weak cache
entries (1.X) (#425)
9541e18d is described below
commit 9541e18dfc99e1a61578dcf3719649e47ef5faaa
Author: Naveed Khan <[email protected]>
AuthorDate: Tue Jul 21 17:38:24 2026 +0000
Make WrapDynaClassTest tolerate gc eviction of weak cache entries (1.X)
(#425)
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 <[email protected]>
---
.../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 623d4dbe..5b7cca8c 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 @@ package org.apache.commons.beanutils;
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 @@ class WrapDynaClassTest {
* 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 @@ class WrapDynaClassTest {
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 @@ class WrapDynaClassTest {
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 @@ class WrapDynaClassTest {
}
/**
- * 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();
}
}
