Add restrictions for when eviction listener can be used

An eviction listener cannot be combined with weak keys and an async
cache. This is because the entry can be evicted when the key is
collected, but the future is still in-flight. As the listener is
atomic with the map operation to remove the entry, this would
require blocking and is therefore not allowed.

caffeine/src/main/java/com/github/benmanes/caffeine/cache/Caffeine.java

@@ -492,6 +492,9 @@ <K1 extends K, V1 extends V> Weigher<K1, V1> getWeigher(boolean isAsync) {
    * entries are cleaned up as part of the routine maintenance described in the class javadoc.
    * <p>
    * This feature cannot be used in conjunction with {@link #writer}.
+   * <p>
+   * This feature cannot be used in conjunction with {@link #writer} or when {@link #weakKeys()} is
+   * combined with {@link #buildAsync}.
    *
    * @return this {@code Caffeine} instance (for chaining)
    * @throws IllegalStateException if the key strength was already set or the writer was set
@@ -861,6 +864,9 @@ Ticker getTicker() {
    * <p>
    * <b>Warning:</b> any exception thrown by {@code listener} will <i>not</i> be propagated to the
    * {@code Cache} user, only logged via a {@link Logger}.
+   * <p>
+   * This feature cannot be used in conjunction when {@link #weakKeys()} is combined with
+   * {@link #buildAsync}.
    *
    * @param evictionListener a listener instance that caches should notify each time an entry is
    *        being automatically removed due to eviction
@@ -952,7 +958,8 @@ public <K1 extends K, V1 extends V> Caffeine<K1, V1> removalListener(
    * <b>Warning:</b> any exception thrown by {@code writer} will be propagated to the {@code Cache}
    * user.
    * <p>
-   * This feature cannot be used in conjunction with {@link #weakKeys()} or {@link #buildAsync}.
+   * This feature cannot be used in conjunction with {@link #weakKeys()},
+   * {@link #evictionListener(RemovalListener)}, or {@link #buildAsync}.
    *
    * @param writer a writer instance that caches should notify each time an entry is explicitly
    *        created or modified, or removed for any reason
@@ -972,6 +979,7 @@ public <K1 extends K, V1 extends V> Caffeine<K1, V1> writer(
       @NonNull CacheWriter<? super K1, ? super V1> writer) {
     requireState(this.writer == null, "Writer was already set to %s", this.writer);
     requireState(keyStrength == null, "Weak keys may not be used with CacheWriter");
+    requireState(evictionListener == null, "Eviction listener may not be used with CacheWriter");
 
     @SuppressWarnings("unchecked")
     Caffeine<K1, V1> self = (Caffeine<K1, V1>) this;
@@ -1111,8 +1119,9 @@ public <K1 extends K, V1 extends V> LoadingCache<K1, V1> build(
    * This method does not alter the state of this {@code Caffeine} instance, so it can be invoked
    * again to create multiple independent caches.
    * <p>
-   * This construction cannot be used with {@link #weakValues()}, {@link #softValues()}, or
-   * {@link #writer(CacheWriter)}.
+   * This construction cannot be used with {@link #weakValues()}, {@link #softValues()},
+   * {@link #writer(CacheWriter)}, or when {@link #weakKeys()} are combined with
+   * {@link #evictionListener(RemovalListener)}.
    *
    * @param <K1> the key type of the cache
    * @param <V1> the value type of the cache
@@ -1122,6 +1131,8 @@ public <K1 extends K, V1 extends V> LoadingCache<K1, V1> build(
   public <K1 extends K, V1 extends V> AsyncCache<K1, V1> buildAsync() {
     requireState(valueStrength == null, "Weak or soft values can not be combined with AsyncCache");
     requireState(writer == null, "CacheWriter can not be combined with AsyncCache");
+    requireState(isStrongKeys() || (evictionListener == null),
+        "Weak keys cannot be combined eviction listener and with AsyncLoadingCache");
     requireWeightWithWeigher();
     requireNonLoadingCache();
 
@@ -1142,8 +1153,9 @@ public <K1 extends K, V1 extends V> AsyncCache<K1, V1> buildAsync() {
    * This method does not alter the state of this {@code Caffeine} instance, so it can be invoked
    * again to create multiple independent caches.
    * <p>
-   * This construction cannot be used with {@link #weakValues()}, {@link #softValues()}, or
-   * {@link #writer(CacheWriter)}.
+   * This construction cannot be used with {@link #weakValues()}, {@link #softValues()},
+   * {@link #writer(CacheWriter)}, or when {@link #weakKeys()} are combined with
+   * {@link #evictionListener(RemovalListener)}.
    *
    * @param loader the cache loader used to obtain new values
    * @param <K1> the key type of the loader
@@ -1166,8 +1178,9 @@ public <K1 extends K, V1 extends V> AsyncLoadingCache<K1, V1> buildAsync(
    * This method does not alter the state of this {@code Caffeine} instance, so it can be invoked
    * again to create multiple independent caches.
    * <p>
-   * This construction cannot be used with {@link #weakValues()}, {@link #softValues()}, or
-   * {@link #writer(CacheWriter)}.
+   * This construction cannot be used with {@link #weakValues()}, {@link #softValues()},
+   * {@link #writer(CacheWriter)}, or when {@link #weakKeys()} are combined with
+   * {@link #evictionListener(RemovalListener)}.
    *
    * @param loader the cache loader used to obtain new values
    * @param <K1> the key type of the loader
@@ -1177,9 +1190,11 @@ public <K1 extends K, V1 extends V> AsyncLoadingCache<K1, V1> buildAsync(
   @NonNull
   public <K1 extends K, V1 extends V> AsyncLoadingCache<K1, V1> buildAsync(
       @NonNull AsyncCacheLoader<? super K1, V1> loader) {
-    requireState(valueStrength == null,
-        "Weak or soft values can not be combined with AsyncLoadingCache");
-    requireState(writer == null, "CacheWriter can not be combined with AsyncLoadingCache");
+    requireState(isStrongValues(),"Weak or soft values cannot be combined with AsyncLoadingCache");
+    requireState(writer == null, "CacheWriter cannot be combined with AsyncLoadingCache");
+    requireState(isStrongKeys() || (evictionListener == null),
+        "Weak keys cannot be combined eviction listener and with AsyncLoadingCache");
+
     requireWeightWithWeigher();
     requireNonNull(loader);
 

caffeine/src/test/java/com/github/benmanes/caffeine/cache/CaffeineTest.java

@@ -128,8 +128,31 @@ public void loading_nullLoader() {
 
   /* --------------- async --------------- */
 
+  @Test(expectedExceptions = IllegalStateException.class)
+  public void async_weakValues() {
+    Caffeine.newBuilder().weakValues().buildAsync(loader);
+  }
+
+  @Test(expectedExceptions = IllegalStateException.class)
+  public void async_softValues() {
+    Caffeine.newBuilder().softValues().buildAsync(loader);
+  }
+
+  @Test(expectedExceptions = IllegalStateException.class)
+  public void async_writer() {
+    Caffeine.newBuilder().writer(writer).buildAsync(loader);
+  }
+
+  @Test(expectedExceptions = IllegalStateException.class)
+  public void async_weakKeys_evictionListener() {
+    RemovalListener<Object, Object> evictionListener = (k, v, c) -> {};
+    Caffeine.newBuilder().weakKeys().evictionListener(evictionListener).buildAsync();
+  }
+
+  /* --------------- async loader --------------- */
+
   @Test
-  public void async_nullLoader() {
+  public void asyncLoader_nullLoader() {
     try {
       Caffeine.newBuilder().buildAsync((CacheLoader<Object, Object>) null);
       Assert.fail();
@@ -143,25 +166,31 @@ public void async_nullLoader() {
 
   @Test
   @SuppressWarnings("UnnecessaryMethodReference")
-  public void async_asyncLoader() {
+  public void asyncLoader() {
     Caffeine.newBuilder().buildAsync(loader::asyncLoad);
   }
 
   @Test(expectedExceptions = IllegalStateException.class)
-  public void async_weakValues() {
+  public void asyncLoader_weakValues() {
     Caffeine.newBuilder().weakValues().buildAsync(loader);
   }
 
   @Test(expectedExceptions = IllegalStateException.class)
-  public void async_softValues() {
+  public void asyncLoader_softValues() {
     Caffeine.newBuilder().softValues().buildAsync(loader);
   }
 
   @Test(expectedExceptions = IllegalStateException.class)
-  public void async_writer() {
+  public void asyncLoader_writer() {
     Caffeine.newBuilder().writer(writer).buildAsync(loader);
   }
 
+  @Test(expectedExceptions = IllegalStateException.class)
+  public void async_asyncLoader_weakKeys_evictionListener() {
+    RemovalListener<Object, Object> evictionListener = (k, v, c) -> {};
+    Caffeine.newBuilder().weakKeys().evictionListener(evictionListener).buildAsync(loader);
+  }
+
   /* --------------- initialCapacity --------------- */
 
   @Test(expectedExceptions = IllegalArgumentException.class)

caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheGenerator.java

@@ -198,10 +198,11 @@ private boolean isCompatible(CacheContext context) {
         && !context.expires();
     boolean writerIncompatible = context.writes()
         && ((context.implementation() != Implementation.Caffeine)
-            || context.isAsync() || (context.evictionListenerType() != Listener.DEFAULT));
+            || (context.evictionListenerType() != Listener.DEFAULT)
+            || context.isAsync() || context.isWeakKeys());
     boolean evictionListenerIncompatible = (context.evictionListenerType() != Listener.DEFAULT)
         && ((context.implementation() != Implementation.Caffeine)
-            || (context.isAsync() && !context.isStrongValues()));
+            || (context.isAsync() && context.isWeakKeys()));
 
     boolean skip = asyncIncompatible || asyncLoaderIncompatible
         || writerIncompatible || evictionListenerIncompatible

caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CaffeineCacheFromContext.java

@@ -99,7 +99,7 @@ public static <K, V> Cache<K, V> newCaffeineCache(CacheContext context) {
     if (context.evictionListenerType != Listener.DEFAULT) {
       builder.evictionListener(context.evictionListener);
     }
-    if (context.isStrongKeys() && !context.isAsync()) {
+    if (context.writes()) {
       builder.writer(context.cacheWriter());
     }
     if (context.isAsync()) {