From 754b19da80c55351362be66144b184058c7dba85 Mon Sep 17 00:00:00 2001 From: Ben Manes Date: Sun, 2 May 2021 18:03:26 -0700 Subject: [PATCH] Add clarifying JavaDoc (see #537) In #537, a deadlock occurs because the reload operation blocks waiting for other reloads to complete. This is to batch the individual reloads through a blocking queue, which drains tasks as work is performed. Due to in-flight reloads being tracked by a secondary map, the triggering of a reload is performed with a map computation. This ensures that a stale reload does not insert outdated data by the entry being removed when the reload completes or the original entry was removed / modified. Due to a reload blocking on a map lock, when the batch completes the future's callback to remove the entry collides on the same map lock due to hashing. This causes the task queue to never be reduced to allow more work, so the reload holding the lock is stuck forever. Due to hash collisions, other usages of the map (like eviction) may collide and fully disrupt the cache from performing any more writes. --- .../github/benmanes/caffeine/cache/AsyncCacheLoader.java | 7 +++++++ .../java/com/github/benmanes/caffeine/cache/Cache.java | 3 +++ .../com/github/benmanes/caffeine/cache/CacheLoader.java | 8 ++++++++ 3 files changed, 18 insertions(+) diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCacheLoader.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCacheLoader.java index d776f84944..20ca35d669 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCacheLoader.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCacheLoader.java @@ -48,6 +48,8 @@ public interface AsyncCacheLoader { /** * Asynchronously computes or retrieves the value corresponding to {@code key}. + *

+ * Warning: loading must not attempt to update any mappings of this cache directly. * * @param key the non-null key whose value should be loaded * @param executor the executor with which the entry is asynchronously loaded @@ -71,6 +73,8 @@ public interface AsyncCacheLoader { * This method should be overridden when bulk retrieval is significantly more efficient than many * individual lookups. Note that {@link AsyncLoadingCache#getAll} will defer to individual calls * to {@link AsyncLoadingCache#get} if this method is not overridden. + *

+ * Warning: loading must not attempt to update any mappings of this cache directly. * * @param keys the unique, non-null keys whose values should be loaded * @param executor the executor with which the entries are asynchronously loaded @@ -92,6 +96,9 @@ public interface AsyncCacheLoader { * {@code null} is computed. This method is called when an existing cache entry is refreshed by * {@link Caffeine#refreshAfterWrite}, or through a call to {@link LoadingCache#refresh}. *

+ * Warning: loading must not attempt to update any mappings of this cache directly + * or block waiting for other cache operations to complete. + *

* Note: all exceptions thrown by this method will be logged and then swallowed. * * @param key the non-null key whose value should be loaded diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java index 64fd1aaba9..e91e8916d8 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java @@ -223,6 +223,9 @@ Map getAll(Iterable keys, * Returns access to inspect and perform low-level operations on this cache based on its runtime * characteristics. These operations are optional and dependent on how the cache was constructed * and what abilities the implementation exposes. + *

+ * Warning: policy operations must not be performed from within an atomic scope of + * another cache operation. * * @return access to inspect and perform advanced operations based on the cache's characteristics */ diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheLoader.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheLoader.java index 00238950f8..95e3a6e971 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheLoader.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheLoader.java @@ -123,6 +123,8 @@ default CompletableFuture asyncLoad(K key, Executor executor) { * This method should be overridden when bulk retrieval is significantly more efficient than many * individual lookups. Note that {@link AsyncLoadingCache#getAll} will defer to individual calls * to {@link AsyncLoadingCache#get} if this method is not overridden. + *

+ * Warning: loading must not attempt to update any mappings of this cache directly. * * @param keys the unique, non-null keys whose values should be loaded * @param executor the executor that with asynchronously loads the entries @@ -151,6 +153,9 @@ default CompletableFuture asyncLoad(K key, Executor executor) { * returned. This method is called when an existing cache entry is refreshed by * {@link Caffeine#refreshAfterWrite}, or through a call to {@link LoadingCache#refresh}. *

+ * Warning: loading must not attempt to update any mappings of this cache directly + * or block waiting for other cache operations to complete. + *

* Note: all exceptions thrown by this method will be logged and then swallowed. * * @param key the non-null key whose value should be loaded @@ -172,6 +177,9 @@ default CompletableFuture asyncLoad(K key, Executor executor) { * {@code null} is computed. This method is called when an existing cache entry is refreshed by * {@link Caffeine#refreshAfterWrite}, or through a call to {@link LoadingCache#refresh}. *

+ * Warning: loading must not attempt to update any mappings of this cache directly + * or block waiting for other cache operations to complete. + *

* Note: all exceptions thrown by this method will be logged and then swallowed. * * @param key the non-null key whose value should be loaded