From cbc1cabdbec85ec91b20f63db0ac171cf37ef030 Mon Sep 17 00:00:00 2001 From: Ben Manes Date: Sun, 27 Mar 2022 11:41:50 -0700 Subject: [PATCH] Return the result of an immediate refreshAfterWrite (#688) In Guava if the CacheLoader returns a completed future on a reload then the new value is returned to the caller and overwritten in the cache. Otherwise the last read value is returned and the future overwrites when it is done. This behavior is now supported instead of the more naive fire-and-forget. A quirk in Guava is when using Cache.get(key, callable). In Guava this is wrapped as the cache loader and is used if a refresh is triggered. That causes the supplied CacheLoader's reload to not be called, which could be more surprising. However, their asMap().computeIfAbsent does not trigger a refresh on read so the value is not reloaded. As this behavior is confusing and appears accidental, Caffeine will always use the attached CacheLoader and its reload function for refreshing. --- .../caffeine/cache/BoundedLocalCache.java | 124 +++++++++------- .../caffeine/cache/RefreshAfterWriteTest.java | 140 ++++++++++++++---- .../cache/testing/GuavaCacheFromContext.java | 20 ++- .../caffeine/jsr166/JSR166TestCase.java | 37 +---- gradle/dependencies.gradle | 4 +- .../guava/compatibility/CacheLoadingTest.java | 2 +- .../cache/simulator/parser/Rewriter.java | 1 + .../policy/irr/ClockProPlusPolicy.java | 1 + .../simulator/policy/irr/ClockProPolicy.java | 1 + .../report/csv/CombinedCsvReport.java | 1 + .../cache/simulator/report/csv/PlotCsv.java | 1 + 11 files changed, 213 insertions(+), 119 deletions(-) diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java index 97d433d2c0..3aeff1631e 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java @@ -1223,8 +1223,9 @@ void demoteFromMainProtected() { * @param node the entry in the page replacement policy * @param now the current time, in nanoseconds * @param recordHit if the hit count should be incremented + * @return the refreshed value if immediately loaded, else null */ - void afterRead(Node node, long now, boolean recordHit) { + @Nullable V afterRead(Node node, long now, boolean recordHit) { if (recordHit) { statsCounter().recordHits(1); } @@ -1233,7 +1234,7 @@ void afterRead(Node node, long now, boolean recordHit) { if (shouldDrainBuffers(delayable)) { scheduleDrainBuffers(); } - refreshIfNeeded(node, now); + return refreshIfNeeded(node, now); } /** Returns if the cache should bypass the read buffer. */ @@ -1246,11 +1247,12 @@ boolean skipReadBuffer() { * * @param node the entry in the cache to refresh * @param now the current time, in nanoseconds + * @return the refreshed value if immediately loaded, else null */ @SuppressWarnings("FutureReturnValueIgnored") - void refreshIfNeeded(Node node, long now) { + @Nullable V refreshIfNeeded(Node node, long now) { if (!refreshAfterWrite()) { - return; + return null; } K key; @@ -1300,57 +1302,63 @@ void refreshIfNeeded(Node node, long now) { node.casWriteTime(refreshWriteTime, writeTime); } - if (refreshFuture[0] != null) { - refreshFuture[0].whenComplete((newValue, error) -> { - long loadTime = statsTicker().read() - startTime[0]; - if (error != null) { - if (!(error instanceof CancellationException) && !(error instanceof TimeoutException)) { - logger.log(Level.WARNING, "Exception thrown during refresh", error); - } - refreshes.remove(keyReference, refreshFuture[0]); - statsCounter().recordLoadFailure(loadTime); - return; + if (refreshFuture[0] == null) { + return null; + } + + var refreshed = refreshFuture[0].handle((newValue, error) -> { + long loadTime = statsTicker().read() - startTime[0]; + if (error != null) { + if (!(error instanceof CancellationException) && !(error instanceof TimeoutException)) { + logger.log(Level.WARNING, "Exception thrown during refresh", error); } + refreshes.remove(keyReference, refreshFuture[0]); + statsCounter().recordLoadFailure(loadTime); + return null; + } - @SuppressWarnings("unchecked") - V value = (isAsync && (newValue != null)) ? (V) refreshFuture[0] : newValue; - - boolean[] discard = new boolean[1]; - compute(key, (k, currentValue) -> { - if (currentValue == null) { - // If the entry is absent then discard the refresh and maybe notifying the listener - discard[0] = (value != null); - return null; - } else if (currentValue == value) { - // If the reloaded value is the same instance then no-op - return currentValue; - } else if (isAsync && - (newValue == Async.getIfReady((CompletableFuture) currentValue))) { - // If the completed futures hold the same value instance then no-op - return currentValue; - } else if ((currentValue == oldValue) && (node.getWriteTime() == writeTime)) { - // If the entry was not modified while in-flight (no ABA) then replace - return value; - } - // Otherwise, a write invalidated the refresh so discard it and notify the listener - discard[0] = true; - return currentValue; - }, expiry(), /* recordMiss */ false, - /* recordLoad */ false, /* recordLoadFailure */ true); + @SuppressWarnings("unchecked") + V value = (isAsync && (newValue != null)) ? (V) refreshFuture[0] : newValue; - if (discard[0]) { - notifyRemoval(key, value, RemovalCause.REPLACED); - } - if (newValue == null) { - statsCounter().recordLoadFailure(loadTime); - } else { - statsCounter().recordLoadSuccess(loadTime); + boolean[] discard = new boolean[1]; + V result = compute(key, (k, currentValue) -> { + if (currentValue == null) { + // If the entry is absent then discard the refresh and maybe notifying the listener + discard[0] = (value != null); + return null; + } else if (currentValue == value) { + // If the reloaded value is the same instance then no-op + return currentValue; + } else if (isAsync && + (newValue == Async.getIfReady((CompletableFuture) currentValue))) { + // If the completed futures hold the same value instance then no-op + return currentValue; + } else if ((currentValue == oldValue) && (node.getWriteTime() == writeTime)) { + // If the entry was not modified while in-flight (no ABA) then replace + return value; } + // Otherwise, a write invalidated the refresh so discard it and notify the listener + discard[0] = true; + return currentValue; + }, expiry(), /* recordMiss */ false, + /* recordLoad */ false, /* recordLoadFailure */ true); - refreshes.remove(keyReference, refreshFuture[0]); - }); - } + if (discard[0]) { + notifyRemoval(key, value, RemovalCause.REPLACED); + } + if (newValue == null) { + statsCounter().recordLoadFailure(loadTime); + } else { + statsCounter().recordLoadSuccess(loadTime); + } + + refreshes.remove(keyReference, refreshFuture[0]); + return result; + }); + return Async.getIfReady(refreshed); } + + return null; } /** @@ -2074,8 +2082,8 @@ public boolean containsValue(Object value) { setAccessTime(node, now); tryExpireAfterRead(node, castedKey, value, expiry(), now); } - afterRead(node, now, recordStats); - return value; + V refreshed = afterRead(node, now, recordStats); + return (refreshed == null) ? value : refreshed; } @Override @@ -2117,15 +2125,18 @@ public Map getAllPresent(Iterable keys) { if ((node == null) || ((value = node.getValue()) == null) || hasExpired(node, now)) { iter.remove(); } else { - entry.setValue(value); - if (!isComputingAsync(node)) { @SuppressWarnings("unchecked") K castedKey = (K) entry.getKey(); tryExpireAfterRead(node, castedKey, value, expiry(), now); setAccessTime(node, now); } - afterRead(node, now, /* recordHit */ false); + V refreshed = afterRead(node, now, /* recordHit */ false); + if (refreshed == null) { + entry.setValue(value); + } else { + entry.setValue(refreshed); + } } } statsCounter().recordHits(result.size()); @@ -2489,9 +2500,8 @@ public void replaceAll(BiFunction function) { tryExpireAfterRead(node, key, value, expiry(), now); setAccessTime(node, now); } - - afterRead(node, now, /* recordHit */ recordStats); - return value; + var refreshed = afterRead(node, now, /* recordHit */ recordStats); + return (refreshed == null) ? value : refreshed; } } if (recordStats) { diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java index c0f7e96cf0..773e39f471 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java @@ -29,6 +29,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static java.util.Map.entry; +import static java.util.function.Function.identity; import static org.hamcrest.Matchers.is; import static uk.org.lidalia.slf4jext.ConventionalLevelHierarchy.INFO_LEVELS; import static uk.org.lidalia.slf4jext.Level.WARN; @@ -63,6 +64,7 @@ import com.github.benmanes.caffeine.testing.ConcurrentTestHarness; import com.github.benmanes.caffeine.testing.Int; import com.github.valfirst.slf4jtest.TestLoggerFactory; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; /** @@ -114,10 +116,7 @@ public CompletableFuture asyncReload(Int key, Int oldValue, Executor execut await().untilAsserted(() -> assertThat(cache.policy().refreshes()).isEmpty()); context.ticker().advance(duration); - assertThat(cache.get(key)).isEqualTo(refresh1); - - await().untilAtomic(reloads, is(2)); - await().untilAsserted(() -> assertThat(cache).containsEntry(key, refresh2)); + assertThat(cache.get(key)).isEqualTo(refresh2); } @Test(dataProvider = "caches") @@ -340,18 +339,36 @@ public void refreshIfNeeded_error_log(CacheContext context) { /* --------------- getIfPresent --------------- */ @Test(dataProvider = "caches") - @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.NEGATIVE, + @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.IDENTITY, population = { Population.SINGLETON, Population.PARTIAL, Population.FULL }) - public void getIfPresent(LoadingCache cache, CacheContext context) { + public void getIfPresent_immediate(LoadingCache cache, CacheContext context) { context.ticker().advance(30, TimeUnit.SECONDS); assertThat(cache.getIfPresent(context.middleKey())).isEqualTo(context.middleKey().negate()); context.ticker().advance(45, TimeUnit.SECONDS); - assertThat(cache.getIfPresent(context.middleKey())).isEqualTo(context.middleKey().negate()); + assertThat(cache.getIfPresent(context.middleKey())).isEqualTo(context.middleKey()); assertThat(cache).hasSize(context.initialSize()); assertThat(context).removalNotifications().withCause(REPLACED).hasSize(1).exclusively(); } + @Test(dataProvider = "caches") + @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.ASYNC_INCOMPLETE, + population = { Population.SINGLETON, Population.PARTIAL, Population.FULL }) + public void getIfPresent_delayed(LoadingCache cache, CacheContext context) { + context.ticker().advance(30, TimeUnit.SECONDS); + assertThat(cache.getIfPresent(context.middleKey())).isEqualTo(context.middleKey().negate()); + context.ticker().advance(45, TimeUnit.SECONDS); + assertThat(cache.getIfPresent(context.middleKey())).isEqualTo(context.middleKey().negate()); + + assertThat(cache).hasSize(context.initialSize()); + assertThat(context).removalNotifications().isEmpty(); + + if (context.isCaffeine()) { + cache.policy().refreshes().get(context.middleKey()).complete(context.middleKey()); + assertThat(context).removalNotifications().withCause(REPLACED).hasSize(1).exclusively(); + } + } + @Test(dataProvider = "caches") @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.NEGATIVE, population = { Population.SINGLETON, Population.PARTIAL, Population.FULL }) @@ -368,35 +385,71 @@ public void getIfPresent_async(AsyncLoadingCache cache, CacheContext c /* --------------- getAllPresent --------------- */ @Test(dataProvider = "caches") - @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, + @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.IDENTITY, population = { Population.PARTIAL, Population.FULL }) - public void getAllPresent(LoadingCache cache, CacheContext context) { + public void getAllPresent_immediate(LoadingCache cache, CacheContext context) { int count = context.firstMiddleLastKeys().size(); context.ticker().advance(30, TimeUnit.SECONDS); cache.getAllPresent(context.firstMiddleLastKeys()); context.ticker().advance(45, TimeUnit.SECONDS); - assertThat(cache.getAllPresent(context.firstMiddleLastKeys())).hasSize(count); + assertThat(cache.getAllPresent(context.firstMiddleLastKeys())).containsExactly( + context.firstKey(), context.firstKey(), context.middleKey(), context.middleKey(), + context.lastKey(), context.lastKey()); assertThat(cache).hasSize(context.initialSize()); assertThat(context).removalNotifications().withCause(REPLACED).hasSize(count).exclusively(); } + @Test(dataProvider = "caches") + @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.ASYNC_INCOMPLETE, + population = { Population.SINGLETON, Population.PARTIAL, Population.FULL }) + public void getAllPresent_delayed(LoadingCache cache, CacheContext context) { + context.ticker().advance(30, TimeUnit.SECONDS); + var expected = cache.getAllPresent(context.firstMiddleLastKeys()); + context.ticker().advance(45, TimeUnit.SECONDS); + assertThat(cache.getAllPresent(context.firstMiddleLastKeys())) + .containsExactlyEntriesIn(expected); + + if (context.isCaffeine()) { + for (var key : context.firstMiddleLastKeys()) { + cache.policy().refreshes().get(key).complete(key); + } + assertThat(context).removalNotifications().withCause(REPLACED) + .hasSize(expected.size()).exclusively(); + } + } + /* --------------- getFunc --------------- */ @Test(dataProvider = "caches") - @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, + @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.IDENTITY, population = { Population.PARTIAL, Population.FULL }) - public void getFunc(LoadingCache cache, CacheContext context) { - Function mappingFunction = context.original()::get; + public void getFunc_immediate(LoadingCache cache, CacheContext context) { context.ticker().advance(30, TimeUnit.SECONDS); - cache.get(context.firstKey(), mappingFunction); + cache.get(context.firstKey(), identity()); context.ticker().advance(45, TimeUnit.SECONDS); - cache.get(context.lastKey(), mappingFunction); // refreshed + assertThat(cache.get(context.lastKey(), identity())).isEqualTo(context.lastKey()); assertThat(cache).hasSize(context.initialSize()); assertThat(context).removalNotifications().withCause(REPLACED).hasSize(1).exclusively(); } + @Test(dataProvider = "caches") + @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.ASYNC_INCOMPLETE, + population = { Population.PARTIAL, Population.FULL }) + public void getFunc_delayed(LoadingCache cache, CacheContext context) { + Function mappingFunction = context.original()::get; + context.ticker().advance(30, TimeUnit.SECONDS); + cache.get(context.firstKey(), mappingFunction); + context.ticker().advance(45, TimeUnit.SECONDS); + assertThat(cache.get(context.lastKey(), mappingFunction)).isEqualTo(context.lastKey().negate()); + + if (context.isCaffeine()) { + cache.policy().refreshes().get(context.lastKey()).complete(context.lastKey()); + assertThat(context).removalNotifications().withCause(REPLACED).hasSize(1).exclusively(); + } + } + @Test(dataProvider = "caches") @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, population = { Population.PARTIAL, Population.FULL }) @@ -414,18 +467,33 @@ public void getFunc_async(AsyncLoadingCache cache, CacheContext contex /* --------------- get --------------- */ @Test(dataProvider = "caches") - @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, + @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.IDENTITY, population = { Population.PARTIAL, Population.FULL }) - public void get(LoadingCache cache, CacheContext context) { + public void get_immediate(LoadingCache cache, CacheContext context) { context.ticker().advance(30, TimeUnit.SECONDS); cache.get(context.firstKey()); - cache.get(context.absentKey()); context.ticker().advance(45, TimeUnit.SECONDS); + assertThat(cache.get(context.firstKey())).isEqualTo(context.firstKey()); - assertThat(cache).containsEntry(context.firstKey(), context.firstKey().negate()); + assertThat(cache).hasSize(context.initialSize()); assertThat(context).removalNotifications().withCause(REPLACED).hasSize(1).exclusively(); } + @Test(dataProvider = "caches") + @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.ASYNC_INCOMPLETE, + population = { Population.PARTIAL, Population.FULL }) + public void get_delayed(LoadingCache cache, CacheContext context) { + context.ticker().advance(30, TimeUnit.SECONDS); + cache.get(context.firstKey()); + context.ticker().advance(45, TimeUnit.SECONDS); + assertThat(cache.get(context.firstKey())).isEqualTo(context.firstKey().negate()); + + if (context.isCaffeine()) { + cache.policy().refreshes().get(context.firstKey()).complete(context.firstKey()); + assertThat(context).removalNotifications().withCause(REPLACED).hasSize(1).exclusively(); + } + } + @Test(dataProvider = "caches") @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.IDENTITY, population = { Population.PARTIAL, Population.FULL }) @@ -436,7 +504,7 @@ public void get_async(AsyncLoadingCache cache, CacheContext context) { context.ticker().advance(45, TimeUnit.SECONDS); var oldValue = cache.getIfPresent(context.firstKey()); - assertThat(oldValue).succeedsWith(context.firstKey().negate()); + assertThat(oldValue).succeedsWith(context.firstKey()); assertThat(cache).containsEntry(context.firstKey(), context.firstKey()); assertThat(context).removalNotifications().withCause(REPLACED).hasSize(1).exclusively(); @@ -511,23 +579,45 @@ public void get_null(AsyncLoadingCache cache, CacheContext context) { @Test(dataProvider = "caches") @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.IDENTITY, population = { Population.PARTIAL, Population.FULL }) - public void getAll(LoadingCache cache, CacheContext context) { + public void getAll_immediate(LoadingCache cache, CacheContext context) { var keys = List.of(context.firstKey(), context.absentKey()); context.ticker().advance(30, TimeUnit.SECONDS); assertThat(cache.getAll(keys)).containsExactly( context.firstKey(), context.firstKey().negate(), context.absentKey(), context.absentKey()); - // Trigger a refresh, may return old values + // Trigger a refresh, ensure new values are present context.ticker().advance(45, TimeUnit.SECONDS); - cache.getAll(keys); - - // Ensure new values are present assertThat(cache.getAll(keys)).containsExactly( context.firstKey(), context.firstKey(), context.absentKey(), context.absentKey()); assertThat(context).removalNotifications().withCause(REPLACED).hasSize(1).exclusively(); } + @Test(dataProvider = "caches") + @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.ASYNC_INCOMPLETE, + population = { Population.PARTIAL, Population.FULL }) + public void getAll_delayed(LoadingCache cache, CacheContext context) { + var keys = context.firstMiddleLastKeys(); + var expected = ImmutableMap.of( + context.firstKey(), context.firstKey().negate(), + context.middleKey(), context.middleKey().negate(), + context.lastKey(), context.lastKey().negate()); + context.ticker().advance(30, TimeUnit.SECONDS); + assertThat(cache.getAll(keys)).containsExactlyEntriesIn(expected); + + // Trigger a refresh, returns old values + context.ticker().advance(45, TimeUnit.SECONDS); + assertThat(cache.getAll(keys)).containsExactlyEntriesIn(expected); + + if (context.isCaffeine()) { + for (var key : keys) { + cache.policy().refreshes().get(key).complete(key); + } + assertThat(context).removalNotifications().withCause(REPLACED) + .hasSize(keys.size()).exclusively(); + } + } + @Test(dataProvider = "caches") @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE, loader = Loader.IDENTITY, population = { Population.PARTIAL, Population.FULL }) diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/GuavaCacheFromContext.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/GuavaCacheFromContext.java index 2cdb833225..df9511c85f 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/GuavaCacheFromContext.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/GuavaCacheFromContext.java @@ -63,6 +63,8 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.common.util.concurrent.ExecutionError; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.SettableFuture; import com.google.common.util.concurrent.UncheckedExecutionException; /** @@ -71,7 +73,7 @@ @SuppressWarnings("PreferJavaTimeOverload") public final class GuavaCacheFromContext { private GuavaCacheFromContext() {} - private static final ThreadLocal error = new ThreadLocal<>(); + private static final ThreadLocal error = new ThreadLocal<>(); /** Returns a Guava-backed cache. */ @SuppressWarnings("CheckReturnValue") @@ -627,6 +629,22 @@ public V load(K key) throws Exception { throw e; } } + + @Override + @SuppressWarnings("FutureReturnValueIgnored") + public ListenableFuture reload(K key, V oldValue) throws Exception { + error.set(null); + var future = SettableFuture.create(); + delegate.asyncReload(key, oldValue, Runnable::run).whenComplete((r, e) -> { + if (e == null) { + future.set(r); + } else { + future.setException(e); + error.set(e); + } + }); + return future; + } } static class BulkLoader extends SingleLoader { diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/jsr166/JSR166TestCase.java b/caffeine/src/test/java/com/github/benmanes/caffeine/jsr166/JSR166TestCase.java index a2f40b9566..d32c6640c6 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/jsr166/JSR166TestCase.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/jsr166/JSR166TestCase.java @@ -462,24 +462,10 @@ public static void addNamedTestClasses(TestSuite suite, } } - public static final double JAVA_CLASS_VERSION; - public static final String JAVA_SPECIFICATION_VERSION; - static { - try { - JAVA_CLASS_VERSION = java.security.AccessController.doPrivileged( - new java.security.PrivilegedAction() { - @Override - public Double run() { - return Double.valueOf(System.getProperty("java.class.version"));}}); - JAVA_SPECIFICATION_VERSION = java.security.AccessController.doPrivileged( - new java.security.PrivilegedAction() { - @Override - public String run() { - return System.getProperty("java.specification.version");}}); - } catch (Throwable t) { - throw new Error(t); - } - } + public static final double JAVA_CLASS_VERSION = + Double.valueOf(System.getProperty("java.class.version")); + public static final String JAVA_SPECIFICATION_VERSION = + System.getProperty("java.specification.version"); public static boolean atLeastJava6() { return JAVA_CLASS_VERSION >= 50.0; } public static boolean atLeastJava7() { return JAVA_CLASS_VERSION >= 51.0; } @@ -1097,8 +1083,6 @@ void joinPool(ExecutorService pool) { pool.awaitTermination(MEDIUM_DELAY_MS, MILLISECONDS); } } - } catch (SecurityException ok) { - // Allowed in case test doesn't have privs } catch (InterruptedException fail) { threadFail("Unexpected InterruptedException"); } @@ -1181,15 +1165,6 @@ static boolean threadOfInterest(ThreadInfo info) { * Uninteresting threads are filtered out. */ static void dumpTestThreads() { - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - try { - System.setSecurityManager(null); - } catch (SecurityException giveUp) { - return; - } - } - System.err.println("------ stacktrace dump start ------"); for (ThreadInfo info : THREAD_MXBEAN.dumpAllThreads(true, true)) { if (threadOfInterest(info)) { @@ -1197,10 +1172,6 @@ static void dumpTestThreads() { } } System.err.println("------ stacktrace dump end ------"); - - if (sm != null) { - System.setSecurityManager(sm); - } } /** diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index c6b3b541b1..261c5de9fb 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -86,7 +86,7 @@ ext { ] pluginVersions = [ bnd: '6.2.0', - checkstyle: '10.0', + checkstyle: '10.1', coveralls: '2.12.0', dependencyCheck: '7.0.1', errorprone: '2.0.2', @@ -96,7 +96,7 @@ ext { jmhReport: '0.9.0', nexusPublish: '1.1.0', nullaway: '1.3.0', - pmd: '6.43.0', + pmd: '6.44.0', semanticVersioning: '1.1.0', shadow: '7.1.2', snyke: '0.4', diff --git a/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheLoadingTest.java b/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheLoadingTest.java index 3648cbaddc..44d0e8474b 100644 --- a/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheLoadingTest.java +++ b/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheLoadingTest.java @@ -317,7 +317,7 @@ public ListenableFuture reload(Object key, Object oldValue) { assertEquals(1, stats.hitCount()); ticker.advance(1, MILLISECONDS); - assertSame(one, cache.getIfPresent(key)); + assertSame(two, cache.getIfPresent(key)); stats = cache.stats(); assertEquals(1, stats.missCount()); assertEquals(2, stats.loadSuccessCount()); diff --git a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/parser/Rewriter.java b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/parser/Rewriter.java index c907fb83bf..f0fa36e0c1 100644 --- a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/parser/Rewriter.java +++ b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/parser/Rewriter.java @@ -47,6 +47,7 @@ * * @author ben.manes@gmail.com (Ben Manes) */ +@SuppressWarnings("PMD.ImmutableField") @Command(mixinStandardHelpOptions = true) public final class Rewriter implements Runnable { @Option(names = "--inputFiles", required = true, split = ",", description = "The trace input " diff --git a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/ClockProPlusPolicy.java b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/ClockProPlusPolicy.java index 302d94fb58..8b0d6f2af8 100644 --- a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/ClockProPlusPolicy.java +++ b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/ClockProPlusPolicy.java @@ -67,6 +67,7 @@ * @author park910113@gmail.com (Chanyoung Park) */ @PolicySpec(name = "irr.ClockProPlus") +@SuppressWarnings("PMD.ImmutableField") public final class ClockProPlusPolicy implements KeyOnlyPolicy { // Enable to print out the internal state private static final boolean debug = false; diff --git a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/ClockProPolicy.java b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/ClockProPolicy.java index 5d018309ee..866781a3d4 100644 --- a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/ClockProPolicy.java +++ b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/ClockProPolicy.java @@ -48,6 +48,7 @@ * @author park910113@gmail.com (Chanyoung Park) */ @PolicySpec(name = "irr.ClockPro") +@SuppressWarnings("PMD.ImmutableField") public final class ClockProPolicy implements KeyOnlyPolicy { // Enable to print out the internal state private static final boolean debug = false; diff --git a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/report/csv/CombinedCsvReport.java b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/report/csv/CombinedCsvReport.java index 6063685fbe..2c0ab00816 100644 --- a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/report/csv/CombinedCsvReport.java +++ b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/report/csv/CombinedCsvReport.java @@ -42,6 +42,7 @@ * * @author ben.manes@gmail.com (Ben Manes) */ +@SuppressWarnings("PMD.ImmutableField") @Command(mixinStandardHelpOptions = true) public final class CombinedCsvReport implements Runnable { @Option(names = "--inputFiles", required = true, split = ",", diff --git a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/report/csv/PlotCsv.java b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/report/csv/PlotCsv.java index e487fc5366..2b53d1267e 100644 --- a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/report/csv/PlotCsv.java +++ b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/report/csv/PlotCsv.java @@ -63,6 +63,7 @@ * * @author ben.manes@gmail.com (Ben Manes) */ +@SuppressWarnings("PMD.ImmutableField") @Command(mixinStandardHelpOptions = true) public final class PlotCsv implements Runnable { @Option(names = "--inputFile", required = true, description = "The csv file path")