This is an automated email from the ASF dual-hosted git repository. jamesbognar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/juneau.git
commit 8c80381a4fc69d7d49702ebd820e3be137dc4207 Author: James Bognar <[email protected]> AuthorDate: Tue Aug 18 15:57:36 2026 -0400 Fix arity-1 Cache WEAK-mode self-reference leak and harden CacheN WEAK tests WEAK mode previously keyed the shared wrapperCache on a Tuple1<K> wrapper, whose strong reference kept the WeakHashMap key alive so entries were never reclaimed. WEAK now keys the WeakHashMap directly on K, dropping the Tuple1/wrapperCache indirection for that mode so entries honor the documented weak-key contract. This is a behavior change: WEAK caches now actually evict once keys become weakly reachable. To be captured in the 10.0.0 release notes. Also hardens the CacheN WEAK tests against GC-timing flakiness by asserting deterministic value-correctness plus reclamation-after-forced-GC using non-interned keys, fixing the Java-25 CI flake. --- .../apache/juneau/commons/collections/Cache.java | 83 ++++++++-- .../juneau/commons/collections/Cache2_Test.java | 141 +++++++++------- .../juneau/commons/collections/Cache3_Test.java | 141 +++++++++------- .../juneau/commons/collections/Cache4_Test.java | 141 +++++++++------- .../juneau/commons/collections/Cache5_Test.java | 141 +++++++++------- .../juneau/commons/collections/Cache_Test.java | 183 +++++++++++++-------- 6 files changed, 512 insertions(+), 318 deletions(-) diff --git a/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/collections/Cache.java b/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/collections/Cache.java index 4b4140b230..0e8558f7fb 100644 --- a/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/collections/Cache.java +++ b/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/collections/Cache.java @@ -462,16 +462,21 @@ public class Cache<K,V> { return new Builder<>(); } - // Internal map with Tuple1 keys for content-based equality (especially for arrays) - // If threadLocal is true, this is null and threadLocalMap is used instead - private final Map<Tuple1<K>,V> map; + // Internal map holding cached values. + // In WEAK mode this is a WeakHashMap keyed DIRECTLY on K (see mapKey()) so entries honor the weak-key + // contract; in all other modes it is keyed on a Tuple1<K> wrapper (see wrap()) that provides content-based + // equality for array keys. The static key type is therefore Object. + // If threadLocal is true, this is null and threadLocalMap is used instead. + private final Map<Object,V> map; @SuppressWarnings({ "java:S5164" // Cleanup method provided: cleanup() }) - private final ThreadLocal<Map<Tuple1<K>,V>> threadLocalMap; + private final ThreadLocal<Map<Object,V>> threadLocalMap; private final boolean isThreadLocal; + private final boolean isWeak; + /** * Cache of Tuple1 wrapper objects to minimize object creation on repeated get/put calls. * @@ -479,6 +484,11 @@ public class Cache<K,V> { * Uses WeakHashMap so wrappers can be GC'd when keys are no longer referenced. * This provides a significant performance improvement for caches with repeated key access. * If threadLocal is true, this is null and threadLocalWrapperCache is used instead. + * + * <p> + * <b>Left <jk>null</jk> in {@link CacheMode#WEAK WEAK} mode</b>: WEAK mode keys the backing map directly on + * {@code K} to honor weak-key semantics, and wrapping there would reintroduce a self-reference leak (see + * {@link #mapKey(Object)}). */ private final Map<K,Tuple1<K>> wrapperCache; @@ -505,12 +515,15 @@ public class Cache<K,V> { this.disableCaching = builder.cacheMode == NONE; this.supplier = builder.supplier != null ? builder.supplier : key -> null; this.isThreadLocal = builder.threadLocal; + this.isWeak = builder.cacheMode == WEAK; if (isThreadLocal) { // Thread-local mode: each thread gets its own map - if (builder.cacheMode == WEAK) { + if (isWeak) { + // WEAK mode keys the map directly on K (no Tuple1 wrapper / wrapperCache) so entries stay + // reachable exactly as long as the caller's key does. See mapKey(). this.threadLocalMap = ThreadLocal.withInitial(() -> synchronizedMap(new WeakHashMap<>())); - this.threadLocalWrapperCache = ThreadLocal.withInitial(() -> synchronizedMap(new WeakHashMap<>())); + this.threadLocalWrapperCache = null; } else { this.threadLocalMap = ThreadLocal.withInitial(() -> new ConcurrentHashMap<>()); this.threadLocalWrapperCache = ThreadLocal.withInitial(() -> synchronizedMap(new WeakHashMap<>())); @@ -519,9 +532,11 @@ public class Cache<K,V> { this.wrapperCache = null; } else { // Normal mode: shared map across all threads - if (builder.cacheMode == WEAK) { + if (isWeak) { + // WEAK mode keys the map directly on K (no Tuple1 wrapper / wrapperCache) so entries stay + // reachable exactly as long as the caller's key does. See mapKey(). this.map = synchronizedMap(new WeakHashMap<>()); - this.wrapperCache = synchronizedMap(new WeakHashMap<>()); + this.wrapperCache = null; } else { this.map = new ConcurrentHashMap<>(); this.wrapperCache = synchronizedMap(new WeakHashMap<>()); @@ -539,7 +554,8 @@ public class Cache<K,V> { */ public void clear() { getMap().clear(); - getWrapperCache().clear(); // Clean up wrapper cache + if (! isWeak) + getWrapperCache().clear(); // Clean up wrapper cache (unused in WEAK mode) } /** @@ -568,7 +584,7 @@ public class Cache<K,V> { * @return <jk>true</jk> if the cache contains the key. */ public boolean containsKey(K key) { - return getMap().containsKey(wrap(key)); + return getMap().containsKey(mapKey(key)); } /** @@ -654,7 +670,7 @@ public class Cache<K,V> { if (disableCaching) return supplier.get(); var m = getMap(); - Tuple1<K> wrapped = wrap(key); + var wrapped = mapKey(key); V v = m.get(wrapped); if (v == null) { if (size() > maxSize) @@ -715,12 +731,13 @@ public class Cache<K,V> { public V put(K key, V value) { var m = getMap(); if (value == null) { - Tuple1<K> wrapped = wrap(key); + var wrapped = mapKey(key); V result = m.remove(wrapped); - getWrapperCache().remove(key); // Clean up wrapper cache + if (! isWeak) + getWrapperCache().remove(key); // Clean up wrapper cache (unused in WEAK mode) return result; } - return m.put(wrap(key), value); + return m.put(mapKey(key), value); } /** @@ -731,9 +748,10 @@ public class Cache<K,V> { */ public V remove(K key) { var m = getMap(); - var wrapped = wrap(key); + var wrapped = mapKey(key); V result = m.remove(wrapped); - getWrapperCache().remove(key); // Clean up wrapper cache + if (! isWeak) + getWrapperCache().remove(key); // Clean up wrapper cache (unused in WEAK mode) return result; } @@ -751,7 +769,7 @@ public class Cache<K,V> { * * @return The map for the current thread. */ - private Map<Tuple1<K>,V> getMap() { return isThreadLocal ? threadLocalMap.get() : map; } + private Map<Object,V> getMap() { return isThreadLocal ? threadLocalMap.get() : map; } /** * Gets the wrapper cache for the current thread. @@ -760,6 +778,33 @@ public class Cache<K,V> { */ private Map<K,Tuple1<K>> getWrapperCache() { return isThreadLocal ? threadLocalWrapperCache.get() : wrapperCache; } + /** + * Computes the backing-map key for the given caller key. + * + * <p> + * In {@link CacheMode#WEAK WEAK} mode the key is used <b>directly</b> as the {@link WeakHashMap} key so that + * an entry stays reachable exactly as long as the caller's key {@code K} is strongly reachable elsewhere, and + * becomes GC-eligible the moment the caller drops it - the documented weak-key contract. + * + * <p> + * WEAK mode must <b>never</b> route through {@link #wrap(Object)}/{@code wrapperCache}. That map's value is a + * {@link Tuple1} which strongly holds its own weak key {@code K}, creating a + * {@code Cache -> wrapperCache -> Tuple1 -> K} strong path that pins the weak key forever - a self-reference + * leak that silently turns WEAK mode into a never-evicting FULL cache. Do not "optimize" the wrapper back in + * for WEAK mode. (Consequently array keys are matched by identity, not content, in WEAK mode; that is + * inherent to weak keying, whose retention necessarily tracks the caller's exact key object.) + * + * <p> + * All other modes wrap the key in a {@link Tuple1} (see {@link #wrap(Object)}) for content-based equality, + * which matters especially for array keys. + * + * @param key The caller's key. + * @return The object to use as the backing-map key. + */ + private Object mapKey(K key) { + return isWeak ? key : wrap(key); + } + /** * Gets or creates a Tuple1 wrapper for the given key. * @@ -767,6 +812,10 @@ public class Cache<K,V> { * The Tuple1 wrapper provides content-based equality for arrays and other objects. * By caching these wrappers, we avoid creating new Tuple1 objects on every cache access. * + * <p> + * Only used by non-{@link CacheMode#WEAK WEAK} modes - see {@link #mapKey(Object)} for why WEAK mode keys on + * {@code K} directly instead. + * * @param key The key to wrap. * @return A cached or new Tuple1 wrapper for the key. */ diff --git a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache2_Test.java b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache2_Test.java index b6cf0dc468..8aa90162ca 100644 --- a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache2_Test.java +++ b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache2_Test.java @@ -232,18 +232,18 @@ class Cache2_Test extends TestBase { }) .build(); - // First call - cache miss + // WEAK mode stores its composite keys in a weak map and those keys live only inside the cache, so any GC + // can reclaim the entries - a stable size, cache-hit count, or same-instance result cannot be asserted + // deterministically (see gcUntilEmpty). Verify value correctness, that the supplier ran, and reclamation. var result1 = x.get("user", 123); - - // Second call - cache hit var result2 = x.get("user", 123); assertEquals("user:123", result1); assertEquals("user:123", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -253,18 +253,15 @@ class Cache2_Test extends TestBase { .supplier((k1, k2) -> k1 + ":" + k2) .build(); - x.get("user", 123); - x.get("admin", 456); - x.get("guest", 789); - - assertSize(3, x); - assertEquals(0, x.getCacheHits()); - - // Verify all cached + // Distinct keys map to distinct values. WEAK entries can be reclaimed by any GC (see gcUntilEmpty), so + // verify per-key value correctness and reclamation rather than a transient size or hit count. assertEquals("user:123", x.get("user", 123)); assertEquals("admin:456", x.get("admin", 456)); assertEquals("guest:789", x.get("guest", 789)); - assertEquals(3, x.getCacheHits()); + assertEquals(0, x.getCacheHits()); // Three distinct keys - all misses, no hits yet. + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -274,9 +271,10 @@ class Cache2_Test extends TestBase { .supplier((k1, k2) -> k1 + ":" + k2) .build(); - x.get("user", 123); - x.get("admin", 456); - assertSize(2, x); + // WEAK entries can be reclaimed by any GC (keys live only inside the cache and cannot be pinned from a + // test), so assert only value correctness and that clear() deterministically empties the cache. + assertEquals("user:123", x.get("user", 123)); + assertEquals("admin:456", x.get("admin", 456)); x.clear(); assertEmpty(x); @@ -290,17 +288,16 @@ class Cache2_Test extends TestBase { .supplier((k1, k2) -> k1 + ":" + k2) .build(); - x.get("k1", 1); - x.get("k2", 2); - assertSize(2, x); + // In WEAK mode entries can be reclaimed by any GC, so the size-based eviction sequence cannot be observed + // deterministically here; that eviction behavior is covered by e01_maxSize_clearsWhenExceeded (FULL mode). + // Verify only that a weak cache built with maxSize computes values correctly and reclaims its entries. + assertEquals("k1:1", x.get("k1", 1)); + assertEquals("k2:2", x.get("k2", 2)); + assertEquals("k3:3", x.get("k3", 3)); + assertEquals("k4:4", x.get("k4", 4)); - // 3rd item doesn't trigger eviction yet - x.get("k3", 3); - assertSize(3, x); - - // 4th item triggers eviction - x.get("k4", 4); - assertSize(1, x); + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -315,18 +312,17 @@ class Cache2_Test extends TestBase { }) .build(); - // First call - cache miss + // weak() is a shortcut for cacheMode(WEAK); entries can be reclaimed by any GC (see gcUntilEmpty), so + // verify value correctness, that the supplier ran, and reclamation rather than a transient size/hit count. var result1 = x.get("user", 123); - - // Second call - cache hit var result2 = x.get("user", 123); assertEquals("user:123", result1); assertEquals("user:123", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -338,9 +334,13 @@ class Cache2_Test extends TestBase { .supplier((k1, k2) -> k1 + ":" + k2) .build(); + // Verify the chained weak() + maxSize() builder produces a working cache. WEAK entries can be reclaimed + // by any GC (see gcUntilEmpty), so assert value correctness and reclamation rather than a transient size. var result = x.get("user", 123); assertEquals("user:123", result); - assertSize(1, x); + + gcUntilEmpty(x); + assertEmpty(x); } //==================================================================================================== @@ -825,18 +825,18 @@ class Cache2_Test extends TestBase { }) .build(); - // First call - cache miss + // Thread-local WEAK mode stores composite keys in a per-thread weak map; those keys live only inside the + // cache, so any GC can reclaim the entries. Verify value correctness, that the supplier ran, and that the + // current thread's entries are reclaimed once unreferenced (gcUntilEmpty runs on this thread). var result1 = x.get("user", 123); - - // Second call - cache hit var result2 = x.get("user", 123); assertEquals("user:123", result1); assertEquals("user:123", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -877,9 +877,10 @@ class Cache2_Test extends TestBase { .supplier((k1, k2) -> k1 + ":" + k2) .build(); - x.get("user", 123); - x.get("admin", 456); - assertSize(2, x); + // Thread-local WEAK entries can be reclaimed by any GC, so assert only value correctness and that clear() + // deterministically empties the current thread's cache. + assertEquals("user:123", x.get("user", 123)); + assertEquals("admin:456", x.get("admin", 456)); x.clear(); assertEmpty(x); @@ -894,17 +895,15 @@ class Cache2_Test extends TestBase { .supplier((k1, k2) -> k1 + ":" + k2) .build(); - x.get("k1", 1); - x.get("k2", 2); - assertSize(2, x); - - // 3rd item doesn't trigger eviction yet - x.get("k3", 3); - assertSize(3, x); + // As with d05, size-based eviction can't be observed deterministically in WEAK mode; eviction is covered + // by o05_threadLocal_maxSize (non-weak). Verify value correctness and reclamation of the thread's entries. + assertEquals("k1:1", x.get("k1", 1)); + assertEquals("k2:2", x.get("k2", 2)); + assertEquals("k3:3", x.get("k3", 3)); + assertEquals("k4:4", x.get("k4", 4)); - // 4th item triggers eviction - x.get("k4", 4); - assertSize(1, x); + gcUntilEmpty(x); + assertEmpty(x); } //==================================================================================================== @@ -960,5 +959,33 @@ class Cache2_Test extends TestBase { // Cleanup before any access - threadLocalMap is null; should not throw assertDoesNotThrow(x::cleanup); } + + //==================================================================================================== + // Helpers + //==================================================================================================== + + /** + * Forces garbage collection and waits (bounded, so a test can never hang) until the specified WEAK-mode + * cache has been emptied by reclamation. + * + * <p>In WEAK mode {@code Cache2} stores its composite {@code Tuple2} keys in a weak map. Those keys are + * created internally by the cache and are reachable only from the cache itself, so any GC can reclaim the + * entries at any time. Crucially, they cannot be pinned from a test: a weak map retains an entry only while + * its exact key <i>object</i> stays strongly reachable, so holding a reconstructed equal key does not help + * (equal != identical), and holding a cached value does not help either (values never keep their keys alive). + * The WEAK-mode tests therefore assert WEAK mode's deterministic contract - values compute correctly and + * entries are reclaimed once their keys are unreferenced - instead of racing an uncontrolled GC to observe a + * transient entry count. Calling this on the current thread also covers the thread-local WEAK caches. + */ + private static void gcUntilEmpty(Cache2<?,?,?> cache) { + for (var i = 0; i < 100 && ! cache.isEmpty(); i++) { + System.gc(); + try { + Thread.sleep(5); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } } diff --git a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache3_Test.java b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache3_Test.java index 21f37dc474..f1664b0ee8 100644 --- a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache3_Test.java +++ b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache3_Test.java @@ -107,18 +107,18 @@ class Cache3_Test extends TestBase { }) .build(); - // First call - cache miss + // WEAK mode stores its composite keys in a weak map and those keys live only inside the cache, so any GC + // can reclaim the entries - a stable size, cache-hit count, or same-instance result cannot be asserted + // deterministically (see gcUntilEmpty). Verify value correctness, that the supplier ran, and reclamation. var result1 = x.get("en", "US", 1); - - // Second call - cache hit var result2 = x.get("en", "US", 1); assertEquals("en:US:1", result1); assertEquals("en:US:1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -128,18 +128,15 @@ class Cache3_Test extends TestBase { .supplier((k1, k2, k3) -> k1 + ":" + k2 + ":" + k3) .build(); - x.get("en", "US", 1); - x.get("fr", "FR", 2); - x.get("de", "DE", 3); - - assertSize(3, x); - assertEquals(0, x.getCacheHits()); - - // Verify all cached + // Distinct keys map to distinct values. WEAK entries can be reclaimed by any GC (see gcUntilEmpty), so + // verify per-key value correctness and reclamation rather than a transient size or hit count. assertEquals("en:US:1", x.get("en", "US", 1)); assertEquals("fr:FR:2", x.get("fr", "FR", 2)); assertEquals("de:DE:3", x.get("de", "DE", 3)); - assertEquals(3, x.getCacheHits()); + assertEquals(0, x.getCacheHits()); // Three distinct keys - all misses, no hits yet. + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -149,9 +146,10 @@ class Cache3_Test extends TestBase { .supplier((k1, k2, k3) -> "value") .build(); - x.get("en", "US", 1); - x.get("fr", "FR", 2); - assertSize(2, x); + // WEAK entries can be reclaimed by any GC (keys live only inside the cache and cannot be pinned from a + // test), so assert only value correctness and that clear() deterministically empties the cache. + assertEquals("value", x.get("en", "US", 1)); + assertEquals("value", x.get("fr", "FR", 2)); x.clear(); assertEmpty(x); @@ -165,17 +163,16 @@ class Cache3_Test extends TestBase { .supplier((k1, k2, k3) -> "value") .build(); - x.get("en", "US", 1); - x.get("fr", "FR", 2); - assertSize(2, x); - - // 3rd item doesn't trigger eviction yet - x.get("de", "DE", 3); - assertSize(3, x); + // In WEAK mode entries can be reclaimed by any GC, so the size-based eviction sequence cannot be observed + // deterministically here; that eviction behavior is covered by a05_maxSize (FULL mode). Verify only that a + // weak cache built with maxSize computes values correctly and reclaims its entries once unreferenced. + assertEquals("value", x.get("en", "US", 1)); + assertEquals("value", x.get("fr", "FR", 2)); + assertEquals("value", x.get("de", "DE", 3)); + assertEquals("value", x.get("es", "ES", 4)); - // 4th item triggers eviction - x.get("es", "ES", 4); - assertSize(1, x); + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -190,18 +187,17 @@ class Cache3_Test extends TestBase { }) .build(); - // First call - cache miss + // weak() is a shortcut for cacheMode(WEAK); entries can be reclaimed by any GC (see gcUntilEmpty), so + // verify value correctness, that the supplier ran, and reclamation rather than a transient size/hit count. var result1 = x.get("en", "US", 1); - - // Second call - cache hit var result2 = x.get("en", "US", 1); assertEquals("en:US:1", result1); assertEquals("en:US:1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -213,9 +209,13 @@ class Cache3_Test extends TestBase { .supplier((k1, k2, k3) -> k1 + ":" + k2 + ":" + k3) .build(); + // Verify the chained weak() + maxSize() builder produces a working cache. WEAK entries can be reclaimed + // by any GC (see gcUntilEmpty), so assert value correctness and reclamation rather than a transient size. var result = x.get("en", "US", 1); assertEquals("en:US:1", result); - assertSize(1, x); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -522,18 +522,18 @@ class Cache3_Test extends TestBase { }) .build(); - // First call - cache miss + // Thread-local WEAK mode stores composite keys in a per-thread weak map; those keys live only inside the + // cache, so any GC can reclaim the entries. Verify value correctness, that the supplier ran, and that the + // current thread's entries are reclaimed once unreferenced (gcUntilEmpty runs on this thread). var result1 = x.get("en", "US", 1); - - // Second call - cache hit var result2 = x.get("en", "US", 1); assertEquals("en:US:1", result1); assertEquals("en:US:1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -574,9 +574,10 @@ class Cache3_Test extends TestBase { .supplier((k1, k2, k3) -> "value") .build(); - x.get("en", "US", 1); - x.get("fr", "FR", 2); - assertSize(2, x); + // Thread-local WEAK entries can be reclaimed by any GC, so assert only value correctness and that clear() + // deterministically empties the current thread's cache. + assertEquals("value", x.get("en", "US", 1)); + assertEquals("value", x.get("fr", "FR", 2)); x.clear(); assertEmpty(x); @@ -591,17 +592,15 @@ class Cache3_Test extends TestBase { .supplier((k1, k2, k3) -> "value") .build(); - x.get("en", "US", 1); - x.get("fr", "FR", 2); - assertSize(2, x); - - // 3rd item doesn't trigger eviction yet - x.get("de", "DE", 3); - assertSize(3, x); + // As with a04e, size-based eviction can't be observed deterministically in WEAK mode; eviction is covered + // by f05_threadLocal_maxSize (non-weak). Verify value correctness and reclamation of the thread's entries. + assertEquals("value", x.get("en", "US", 1)); + assertEquals("value", x.get("fr", "FR", 2)); + assertEquals("value", x.get("de", "DE", 3)); + assertEquals("value", x.get("es", "ES", 4)); - // 4th item triggers eviction - x.get("es", "ES", 4); - assertSize(1, x); + gcUntilEmpty(x); + assertEmpty(x); } //==================================================================================================== @@ -657,5 +656,33 @@ class Cache3_Test extends TestBase { // Cleanup before any access - threadLocalMap is null; should not throw assertDoesNotThrow(x::cleanup); } + + //==================================================================================================== + // Helpers + //==================================================================================================== + + /** + * Forces garbage collection and waits (bounded, so a test can never hang) until the specified WEAK-mode + * cache has been emptied by reclamation. + * + * <p>In WEAK mode {@code Cache3} stores its composite {@code Tuple3} keys in a weak map. Those keys are + * created internally by the cache and are reachable only from the cache itself, so any GC can reclaim the + * entries at any time. Crucially, they cannot be pinned from a test: a weak map retains an entry only while + * its exact key <i>object</i> stays strongly reachable, so holding a reconstructed equal key does not help + * (equal != identical), and holding a cached value does not help either (values never keep their keys alive). + * The WEAK-mode tests therefore assert WEAK mode's deterministic contract - values compute correctly and + * entries are reclaimed once their keys are unreferenced - instead of racing an uncontrolled GC to observe a + * transient entry count. Calling this on the current thread also covers the thread-local WEAK caches. + */ + private static void gcUntilEmpty(Cache3<?,?,?,?> cache) { + for (var i = 0; i < 100 && ! cache.isEmpty(); i++) { + System.gc(); + try { + Thread.sleep(5); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } } diff --git a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache4_Test.java b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache4_Test.java index 3ecb45a7f0..48911e8e2e 100644 --- a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache4_Test.java +++ b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache4_Test.java @@ -108,18 +108,18 @@ class Cache4_Test extends TestBase { }) .build(); - // First call - cache miss + // WEAK mode stores its composite keys in a weak map and those keys live only inside the cache, so any GC + // can reclaim the entries - a stable size, cache-hit count, or same-instance result cannot be asserted + // deterministically (see gcUntilEmpty). Verify value correctness, that the supplier ran, and reclamation. var result1 = x.get("en", "US", "formal", 1); - - // Second call - cache hit var result2 = x.get("en", "US", "formal", 1); assertEquals("en:US:formal:1", result1); assertEquals("en:US:formal:1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -129,18 +129,15 @@ class Cache4_Test extends TestBase { .supplier((k1, k2, k3, k4) -> k1 + ":" + k2 + ":" + k3 + ":" + k4) .build(); - x.get("en", "US", "formal", 1); - x.get("fr", "FR", "formal", 2); - x.get("de", "DE", "formal", 3); - - assertSize(3, x); - assertEquals(0, x.getCacheHits()); - - // Verify all cached + // Distinct keys map to distinct values. WEAK entries can be reclaimed by any GC (see gcUntilEmpty), so + // verify per-key value correctness and reclamation rather than a transient size or hit count. assertEquals("en:US:formal:1", x.get("en", "US", "formal", 1)); assertEquals("fr:FR:formal:2", x.get("fr", "FR", "formal", 2)); assertEquals("de:DE:formal:3", x.get("de", "DE", "formal", 3)); - assertEquals(3, x.getCacheHits()); + assertEquals(0, x.getCacheHits()); // Three distinct keys - all misses, no hits yet. + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -150,9 +147,10 @@ class Cache4_Test extends TestBase { .supplier((k1, k2, k3, k4) -> "value") .build(); - x.get("en", "US", "formal", 1); - x.get("fr", "FR", "formal", 2); - assertSize(2, x); + // WEAK entries can be reclaimed by any GC (keys live only inside the cache and cannot be pinned from a + // test), so assert only value correctness and that clear() deterministically empties the cache. + assertEquals("value", x.get("en", "US", "formal", 1)); + assertEquals("value", x.get("fr", "FR", "formal", 2)); x.clear(); assertEmpty(x); @@ -166,17 +164,16 @@ class Cache4_Test extends TestBase { .supplier((k1, k2, k3, k4) -> "value") .build(); - x.get("en", "US", "formal", 1); - x.get("fr", "FR", "formal", 2); - assertSize(2, x); - - // 3rd item doesn't trigger eviction yet - x.get("de", "DE", "formal", 3); - assertSize(3, x); + // In WEAK mode entries can be reclaimed by any GC, so the size-based eviction sequence cannot be observed + // deterministically here; that eviction behavior is covered by a05_maxSize (FULL mode). Verify only that a + // weak cache built with maxSize computes values correctly and reclaims its entries once unreferenced. + assertEquals("value", x.get("en", "US", "formal", 1)); + assertEquals("value", x.get("fr", "FR", "formal", 2)); + assertEquals("value", x.get("de", "DE", "formal", 3)); + assertEquals("value", x.get("es", "ES", "formal", 4)); - // 4th item triggers eviction - x.get("es", "ES", "formal", 4); - assertSize(1, x); + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -191,18 +188,17 @@ class Cache4_Test extends TestBase { }) .build(); - // First call - cache miss + // weak() is a shortcut for cacheMode(WEAK); entries can be reclaimed by any GC (see gcUntilEmpty), so + // verify value correctness, that the supplier ran, and reclamation rather than a transient size/hit count. var result1 = x.get("en", "US", "formal", 1); - - // Second call - cache hit var result2 = x.get("en", "US", "formal", 1); assertEquals("en:US:formal:1", result1); assertEquals("en:US:formal:1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -214,9 +210,13 @@ class Cache4_Test extends TestBase { .supplier((k1, k2, k3, k4) -> k1 + ":" + k2 + ":" + k3 + ":" + k4) .build(); + // Verify the chained weak() + maxSize() builder produces a working cache. WEAK entries can be reclaimed + // by any GC (see gcUntilEmpty), so assert value correctness and reclamation rather than a transient size. var result = x.get("en", "US", "formal", 1); assertEquals("en:US:formal:1", result); - assertSize(1, x); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -522,18 +522,18 @@ class Cache4_Test extends TestBase { }) .build(); - // First call - cache miss + // Thread-local WEAK mode stores composite keys in a per-thread weak map; those keys live only inside the + // cache, so any GC can reclaim the entries. Verify value correctness, that the supplier ran, and that the + // current thread's entries are reclaimed once unreferenced (gcUntilEmpty runs on this thread). var result1 = x.get("en", "US", "formal", 1); - - // Second call - cache hit var result2 = x.get("en", "US", "formal", 1); assertEquals("en:US:formal:1", result1); assertEquals("en:US:formal:1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -574,9 +574,10 @@ class Cache4_Test extends TestBase { .supplier((k1, k2, k3, k4) -> "value") .build(); - x.get("en", "US", "formal", 1); - x.get("fr", "FR", "informal", 2); - assertSize(2, x); + // Thread-local WEAK entries can be reclaimed by any GC, so assert only value correctness and that clear() + // deterministically empties the current thread's cache. + assertEquals("value", x.get("en", "US", "formal", 1)); + assertEquals("value", x.get("fr", "FR", "informal", 2)); x.clear(); assertEmpty(x); @@ -591,17 +592,15 @@ class Cache4_Test extends TestBase { .supplier((k1, k2, k3, k4) -> "value") .build(); - x.get("en", "US", "formal", 1); - x.get("fr", "FR", "informal", 2); - assertSize(2, x); - - // 3rd item doesn't trigger eviction yet - x.get("de", "DE", "formal", 3); - assertSize(3, x); + // As with a04e, size-based eviction can't be observed deterministically in WEAK mode; eviction is covered + // by f05_threadLocal_maxSize (non-weak). Verify value correctness and reclamation of the thread's entries. + assertEquals("value", x.get("en", "US", "formal", 1)); + assertEquals("value", x.get("fr", "FR", "informal", 2)); + assertEquals("value", x.get("de", "DE", "formal", 3)); + assertEquals("value", x.get("es", "ES", "informal", 4)); - // 4th item triggers eviction - x.get("es", "ES", "informal", 4); - assertSize(1, x); + gcUntilEmpty(x); + assertEmpty(x); } //==================================================================================================== @@ -657,5 +656,33 @@ class Cache4_Test extends TestBase { // Cleanup before any access - threadLocalMap is null; should not throw assertDoesNotThrow(x::cleanup); } + + //==================================================================================================== + // Helpers + //==================================================================================================== + + /** + * Forces garbage collection and waits (bounded, so a test can never hang) until the specified WEAK-mode + * cache has been emptied by reclamation. + * + * <p>In WEAK mode {@code Cache4} stores its composite {@code Tuple4} keys in a weak map. Those keys are + * created internally by the cache and are reachable only from the cache itself, so any GC can reclaim the + * entries at any time. Crucially, they cannot be pinned from a test: a weak map retains an entry only while + * its exact key <i>object</i> stays strongly reachable, so holding a reconstructed equal key does not help + * (equal != identical), and holding a cached value does not help either (values never keep their keys alive). + * The WEAK-mode tests therefore assert WEAK mode's deterministic contract - values compute correctly and + * entries are reclaimed once their keys are unreferenced - instead of racing an uncontrolled GC to observe a + * transient entry count. Calling this on the current thread also covers the thread-local WEAK caches. + */ + private static void gcUntilEmpty(Cache4<?,?,?,?,?> cache) { + for (var i = 0; i < 100 && ! cache.isEmpty(); i++) { + System.gc(); + try { + Thread.sleep(5); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } } diff --git a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache5_Test.java b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache5_Test.java index dcf1769698..deb44369f5 100644 --- a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache5_Test.java +++ b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache5_Test.java @@ -109,18 +109,18 @@ class Cache5_Test extends TestBase { }) .build(); - // First call - cache miss + // WEAK mode stores its composite keys in a weak map and those keys live only inside the cache, so any GC + // can reclaim the entries - a stable size, cache-hit count, or same-instance result cannot be asserted + // deterministically (see gcUntilEmpty). Verify value correctness, that the supplier ran, and reclamation. var result1 = x.get("en", "US", "west", "formal", 1); - - // Second call - cache hit var result2 = x.get("en", "US", "west", "formal", 1); assertEquals("en:US:west:formal:1", result1); assertEquals("en:US:west:formal:1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -130,18 +130,15 @@ class Cache5_Test extends TestBase { .supplier((k1, k2, k3, k4, k5) -> k1 + ":" + k2 + ":" + k3 + ":" + k4 + ":" + k5) .build(); - x.get("en", "US", "west", "formal", 1); - x.get("fr", "FR", "north", "formal", 2); - x.get("de", "DE", "south", "formal", 3); - - assertSize(3, x); - assertEquals(0, x.getCacheHits()); - - // Verify all cached + // Distinct keys map to distinct values. WEAK entries can be reclaimed by any GC (see gcUntilEmpty), so + // verify per-key value correctness and reclamation rather than a transient size or hit count. assertEquals("en:US:west:formal:1", x.get("en", "US", "west", "formal", 1)); assertEquals("fr:FR:north:formal:2", x.get("fr", "FR", "north", "formal", 2)); assertEquals("de:DE:south:formal:3", x.get("de", "DE", "south", "formal", 3)); - assertEquals(3, x.getCacheHits()); + assertEquals(0, x.getCacheHits()); // Three distinct keys - all misses, no hits yet. + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -151,9 +148,10 @@ class Cache5_Test extends TestBase { .supplier((k1, k2, k3, k4, k5) -> "value") .build(); - x.get("en", "US", "west", "formal", 1); - x.get("fr", "FR", "north", "formal", 2); - assertSize(2, x); + // WEAK entries can be reclaimed by any GC (keys live only inside the cache and cannot be pinned from a + // test), so assert only value correctness and that clear() deterministically empties the cache. + assertEquals("value", x.get("en", "US", "west", "formal", 1)); + assertEquals("value", x.get("fr", "FR", "north", "formal", 2)); x.clear(); assertEmpty(x); @@ -167,17 +165,16 @@ class Cache5_Test extends TestBase { .supplier((k1, k2, k3, k4, k5) -> "value") .build(); - x.get("en", "US", "west", "formal", 1); - x.get("fr", "FR", "north", "formal", 2); - assertSize(2, x); - - // 3rd item doesn't trigger eviction yet - x.get("de", "DE", "south", "formal", 3); - assertSize(3, x); + // In WEAK mode entries can be reclaimed by any GC, so the size-based eviction sequence cannot be observed + // deterministically here; that eviction behavior is covered by a05_maxSize (FULL mode). Verify only that a + // weak cache built with maxSize computes values correctly and reclaims its entries once unreferenced. + assertEquals("value", x.get("en", "US", "west", "formal", 1)); + assertEquals("value", x.get("fr", "FR", "north", "formal", 2)); + assertEquals("value", x.get("de", "DE", "south", "formal", 3)); + assertEquals("value", x.get("es", "ES", "east", "formal", 4)); - // 4th item triggers eviction - x.get("es", "ES", "east", "formal", 4); - assertSize(1, x); + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -192,18 +189,17 @@ class Cache5_Test extends TestBase { }) .build(); - // First call - cache miss + // weak() is a shortcut for cacheMode(WEAK); entries can be reclaimed by any GC (see gcUntilEmpty), so + // verify value correctness, that the supplier ran, and reclamation rather than a transient size/hit count. var result1 = x.get("en", "US", "west", "formal", 1); - - // Second call - cache hit var result2 = x.get("en", "US", "west", "formal", 1); assertEquals("en:US:west:formal:1", result1); assertEquals("en:US:west:formal:1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -215,9 +211,13 @@ class Cache5_Test extends TestBase { .supplier((k1, k2, k3, k4, k5) -> k1 + ":" + k2 + ":" + k3 + ":" + k4 + ":" + k5) .build(); + // Verify the chained weak() + maxSize() builder produces a working cache. WEAK entries can be reclaimed + // by any GC (see gcUntilEmpty), so assert value correctness and reclamation rather than a transient size. var result = x.get("en", "US", "west", "formal", 1); assertEquals("en:US:west:formal:1", result); - assertSize(1, x); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -523,18 +523,18 @@ class Cache5_Test extends TestBase { }) .build(); - // First call - cache miss + // Thread-local WEAK mode stores composite keys in a per-thread weak map; those keys live only inside the + // cache, so any GC can reclaim the entries. Verify value correctness, that the supplier ran, and that the + // current thread's entries are reclaimed once unreferenced (gcUntilEmpty runs on this thread). var result1 = x.get("en", "US", "west", "formal", 1); - - // Second call - cache hit var result2 = x.get("en", "US", "west", "formal", 1); assertEquals("en:US:west:formal:1", result1); assertEquals("en:US:west:formal:1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, x); - assertEquals(1, x.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(x); + assertEmpty(x); } @Test @@ -575,9 +575,10 @@ class Cache5_Test extends TestBase { .supplier((k1, k2, k3, k4, k5) -> "value") .build(); - x.get("en", "US", "west", "formal", 1); - x.get("fr", "FR", "east", "informal", 2); - assertSize(2, x); + // Thread-local WEAK entries can be reclaimed by any GC, so assert only value correctness and that clear() + // deterministically empties the current thread's cache. + assertEquals("value", x.get("en", "US", "west", "formal", 1)); + assertEquals("value", x.get("fr", "FR", "east", "informal", 2)); x.clear(); assertEmpty(x); @@ -592,17 +593,15 @@ class Cache5_Test extends TestBase { .supplier((k1, k2, k3, k4, k5) -> "value") .build(); - x.get("en", "US", "west", "formal", 1); - x.get("fr", "FR", "east", "informal", 2); - assertSize(2, x); + // As with a04e, size-based eviction can't be observed deterministically in WEAK mode; eviction is covered + // by f05_threadLocal_maxSize (non-weak). Verify value correctness and reclamation of the thread's entries. + assertEquals("value", x.get("en", "US", "west", "formal", 1)); + assertEquals("value", x.get("fr", "FR", "east", "informal", 2)); + assertEquals("value", x.get("de", "DE", "north", "formal", 3)); + assertEquals("value", x.get("es", "ES", "south", "informal", 4)); - // 3rd item doesn't trigger eviction yet - x.get("de", "DE", "north", "formal", 3); - assertSize(3, x); - - // 4th item triggers eviction - x.get("es", "ES", "south", "informal", 4); - assertSize(1, x); + gcUntilEmpty(x); + assertEmpty(x); } //==================================================================================================== @@ -658,5 +657,33 @@ class Cache5_Test extends TestBase { // Cleanup before any access - threadLocalMap is null; should not throw assertDoesNotThrow(x::cleanup); } + + //==================================================================================================== + // Helpers + //==================================================================================================== + + /** + * Forces garbage collection and waits (bounded, so a test can never hang) until the specified WEAK-mode + * cache has been emptied by reclamation. + * + * <p>In WEAK mode {@code Cache5} stores its composite {@code Tuple5} keys in a weak map. Those keys are + * created internally by the cache and are reachable only from the cache itself, so any GC can reclaim the + * entries at any time. Crucially, they cannot be pinned from a test: a weak map retains an entry only while + * its exact key <i>object</i> stays strongly reachable, so holding a reconstructed equal key does not help + * (equal != identical), and holding a cached value does not help either (values never keep their keys alive). + * The WEAK-mode tests therefore assert WEAK mode's deterministic contract - values compute correctly and + * entries are reclaimed once their keys are unreferenced - instead of racing an uncontrolled GC to observe a + * transient entry count. Calling this on the current thread also covers the thread-local WEAK caches. + */ + private static void gcUntilEmpty(Cache5<?,?,?,?,?,?> cache) { + for (var i = 0; i < 100 && ! cache.isEmpty(); i++) { + System.gc(); + try { + Thread.sleep(5); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } } diff --git a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache_Test.java b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache_Test.java index 9681710137..55b030df06 100644 --- a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache_Test.java +++ b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/collections/Cache_Test.java @@ -261,24 +261,26 @@ class Cache_Test extends TestBase { .build(); var callCount = new AtomicInteger(); - // First call - cache miss - var result1 = cache.get("key1", () -> { + // WEAK mode keys its WeakHashMap directly on K, so an entry is reclaimable once the caller's key is no + // longer strongly referenced - a stable size, cache-hit count, or same-instance result cannot be asserted + // deterministically (see gcUntilEmpty). Verify value correctness, that the supplier ran, and reclamation. + // Keys must be freshly-allocated (non-interned) Strings so the entries are actually collectable - a String + // literal would be pinned by the JVM string pool and never reclaim. + var result1 = cache.get(new String("key1"), () -> { callCount.incrementAndGet(); return "value1"; }); - - // Second call - cache hit - var result2 = cache.get("key1", () -> { + var result2 = cache.get(new String("key1"), () -> { callCount.incrementAndGet(); - return "should not be called"; + return "value1"; }); assertEquals("value1", result1); assertEquals("value1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, cache); - assertEquals(1, cache.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(cache); + assertEmpty(cache); } @Test void a14_weakMode_multipleKeys() { @@ -286,18 +288,16 @@ class Cache_Test extends TestBase { .cacheMode(WEAK) .build(); - cache.get("one", () -> 1); - cache.get("two", () -> 2); - cache.get("three", () -> 3); - - assertSize(3, cache); - assertEquals(0, cache.getCacheHits()); + // Distinct keys map to distinct values. WEAK entries can be reclaimed by any GC (see gcUntilEmpty), so + // verify per-key value correctness and reclamation rather than a transient size or hit count. Freshly + // allocated (non-interned) String keys are required so the entries are actually collectable. + assertEquals(1, cache.get(new String("one"), () -> 1)); + assertEquals(2, cache.get(new String("two"), () -> 2)); + assertEquals(3, cache.get(new String("three"), () -> 3)); + assertEquals(0, cache.getCacheHits()); // Three distinct keys - all misses, no hits yet. - // Verify all cached - assertEquals(1, cache.get("one", () -> 999)); - assertEquals(2, cache.get("two", () -> 999)); - assertEquals(3, cache.get("three", () -> 999)); - assertEquals(3, cache.getCacheHits()); + gcUntilEmpty(cache); + assertEmpty(cache); } @Test void a15_weakMode_clear() { @@ -305,9 +305,10 @@ class Cache_Test extends TestBase { .cacheMode(WEAK) .build(); - cache.get("one", () -> 1); - cache.get("two", () -> 2); - assertSize(2, cache); + // WEAK entries can be reclaimed by any GC, so assert only value correctness and that clear() + // deterministically empties the cache. + assertEquals(1, cache.get("one", () -> 1)); + assertEquals(2, cache.get("two", () -> 2)); cache.clear(); assertEmpty(cache); @@ -319,18 +320,17 @@ class Cache_Test extends TestBase { .maxSize(3) .build(); - cache.get("one", () -> 1); - cache.get("two", () -> 2); - cache.get("three", () -> 3); - assertSize(3, cache); - - // 4th item doesn't trigger eviction yet - cache.get("four", () -> 4); - assertSize(4, cache); + // In WEAK mode entries can be reclaimed by any GC, so the size-based eviction sequence cannot be observed + // deterministically here; that eviction behavior is covered by a08_maxSize_eviction (FULL mode). Verify + // only that a weak cache built with maxSize computes values correctly and reclaims its entries once + // unreferenced. Freshly allocated (non-interned) String keys are required so the entries are collectable. + assertEquals(1, cache.get(new String("one"), () -> 1)); + assertEquals(2, cache.get(new String("two"), () -> 2)); + assertEquals(3, cache.get(new String("three"), () -> 3)); + assertEquals(4, cache.get(new String("four"), () -> 4)); - // 5th item triggers eviction - cache.get("five", () -> 5); - assertSize(1, cache); + gcUntilEmpty(cache); + assertEmpty(cache); } @Test void a16b_weakMethod_basicCaching() { @@ -340,24 +340,24 @@ class Cache_Test extends TestBase { .build(); var callCount = new AtomicInteger(); - // First call - cache miss - var result1 = cache.get("key1", () -> { + // weak() is a shortcut for cacheMode(WEAK); entries can be reclaimed by any GC (see gcUntilEmpty), so + // verify value correctness, that the supplier ran, and reclamation rather than a transient size/hit count. + // Keys must be freshly-allocated (non-interned) Strings so the entries are actually collectable. + var result1 = cache.get(new String("key1"), () -> { callCount.incrementAndGet(); return "value1"; }); - - // Second call - cache hit - var result2 = cache.get("key1", () -> { + var result2 = cache.get(new String("key1"), () -> { callCount.incrementAndGet(); - return "should not be called"; + return "value1"; }); assertEquals("value1", result1); assertEquals("value1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, cache); - assertEquals(1, cache.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(cache); + assertEmpty(cache); } @Test void a16c_weakMethod_chaining() { @@ -368,9 +368,14 @@ class Cache_Test extends TestBase { .supplier(k -> k.length()) .build(); - var result = cache.get("hello"); + // Verify the chained weak() + maxSize() + supplier() builder produces a working cache. WEAK entries can be + // reclaimed by any GC (see gcUntilEmpty), so assert value correctness and reclamation rather than a + // transient size. A freshly-allocated (non-interned) String key is required so the entry is collectable. + var result = cache.get(new String("hello")); assertEquals(5, result); - assertSize(1, cache); + + gcUntilEmpty(cache); + assertEmpty(cache); } //==================================================================================================== @@ -938,9 +943,14 @@ class Cache_Test extends TestBase { .supplier(k -> k.length()) .build(); - var result = cache.get("hello"); + // WEAK cache built via create(): entries can be reclaimed by any GC (see gcUntilEmpty), so assert value + // correctness and reclamation rather than a transient size. A freshly-allocated (non-interned) String key + // is required so the entry is collectable. + var result = cache.get(new String("hello")); assertEquals(5, result); - assertSize(1, cache); + + gcUntilEmpty(cache); + assertEmpty(cache); } //==================================================================================================== @@ -1133,24 +1143,25 @@ class Cache_Test extends TestBase { .build(); var callCount = new AtomicInteger(); - // First call - cache miss - var result1 = cache.get("key1", () -> { + // Thread-local WEAK mode keys a per-thread WeakHashMap directly on K; entries are reclaimable once the + // caller's key is unreferenced. Verify value correctness, that the supplier ran, and that the current + // thread's entries are reclaimed (gcUntilEmpty runs on this thread). Freshly-allocated (non-interned) + // String keys are required so the entries are collectable. + var result1 = cache.get(new String("key1"), () -> { callCount.incrementAndGet(); return "value1"; }); - - // Second call - cache hit - var result2 = cache.get("key1", () -> { + var result2 = cache.get(new String("key1"), () -> { callCount.incrementAndGet(); - return "should not be called"; + return "value1"; }); assertEquals("value1", result1); assertEquals("value1", result2); - assertSame(result1, result2); - assertEquals(1, callCount.get()); // Supplier only called once - assertSize(1, cache); - assertEquals(1, cache.getCacheHits()); + assertTrue(callCount.get() >= 1); + + gcUntilEmpty(cache); + assertEmpty(cache); } @Test void a61_threadLocal_weakMode_eachThreadHasOwnCache() throws InterruptedException, ExecutionException, TimeoutException { @@ -1188,9 +1199,10 @@ class Cache_Test extends TestBase { .cacheMode(WEAK) .build(); - cache.get("one", () -> 1); - cache.get("two", () -> 2); - assertSize(2, cache); + // Thread-local WEAK entries can be reclaimed by any GC, so assert only value correctness and that clear() + // deterministically empties the current thread's cache. + assertEquals(1, cache.get("one", () -> 1)); + assertEquals(2, cache.get("two", () -> 2)); cache.clear(); assertEmpty(cache); @@ -1203,18 +1215,16 @@ class Cache_Test extends TestBase { .maxSize(3) .build(); - cache.get("one", () -> 1); - cache.get("two", () -> 2); - cache.get("three", () -> 3); - assertSize(3, cache); - - // 4th item doesn't trigger eviction yet - cache.get("four", () -> 4); - assertSize(4, cache); + // As with a16, size-based eviction can't be observed deterministically in WEAK mode; eviction is covered + // by a58_threadLocal_maxSize (non-weak). Verify value correctness and reclamation of the thread's entries. + // Freshly allocated (non-interned) String keys are required so the entries are collectable. + assertEquals(1, cache.get(new String("one"), () -> 1)); + assertEquals(2, cache.get(new String("two"), () -> 2)); + assertEquals(3, cache.get(new String("three"), () -> 3)); + assertEquals(4, cache.get(new String("four"), () -> 4)); - // 5th item triggers eviction - cache.get("five", () -> 5); - assertSize(1, cache); + gcUntilEmpty(cache); + assertEmpty(cache); } //==================================================================================================== @@ -1280,5 +1290,32 @@ class Cache_Test extends TestBase { // Cleanup before any access - threadLocalMap and threadLocalWrapperCache are null; should not throw assertDoesNotThrow(cache::cleanup); } + + //==================================================================================================== + // Helpers + //==================================================================================================== + + /** + * Forces garbage collection and waits (bounded, so a test can never hang) until the specified WEAK-mode + * cache has been emptied by reclamation. + * + * <p>WEAK-mode {@link Cache} keys its {@link java.util.WeakHashMap} directly on {@code K}, so an entry is + * reclaimable once the caller's key is no longer strongly referenced. The WEAK-mode tests therefore assert + * WEAK mode's deterministic contract - values compute correctly and entries are reclaimed once their keys are + * unreferenced - instead of racing an uncontrolled GC to observe a transient entry count. Keys used in + * reclamation assertions must be freshly-allocated (non-interned) objects: a {@code String} literal would be + * pinned by the JVM string pool and never reclaim, spinning this loop until its bound. Calling this on the + * current thread also covers the thread-local WEAK caches. + */ + private static void gcUntilEmpty(Cache<?,?> cache) { + for (var i = 0; i < 100 && ! cache.isEmpty(); i++) { + System.gc(); + try { + Thread.sleep(5); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } }
