Fix compatibility between the cache compute methods and a load

guava-tests/test/com/google/common/cache/LocalCacheMapComputeTest.java

@@ -18,9 +18,17 @@
 
 import static com.google.common.truth.Truth.assertThat;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.function.IntConsumer;
 import java.util.stream.IntStream;
+
+import com.google.common.util.concurrent.UncheckedExecutionException;
+
 import junit.framework.TestCase;
 
 /** Test Java8 map.compute in concurrent cache context. */
@@ -91,6 +99,27 @@ public void testComputeIfPresent() {
     assertThat(cache.getIfPresent(key).split(delimiter)).hasLength(count + 1);
   }
 
+  public void testComputeIfPresentRemove() {
+    List<RemovalNotification<Integer, Integer>> notifications = new ArrayList<>();
+    Cache<Integer, Integer> cache = CacheBuilder.newBuilder()
+        .removalListener(new RemovalListener<Integer, Integer>() {
+          @Override public void onRemoval(RemovalNotification<Integer, Integer> notification) {
+            notifications.add(notification);
+          }
+        }).build();
+    cache.put(1, 2);
+
+    // explicitly remove the existing value
+    cache.asMap().computeIfPresent(1, (key, value) -> null);
+    assertThat(notifications).hasSize(1);
+    CacheTesting.checkEmpty(cache);
+
+    // ensure no zombie entry remains
+    cache.asMap().computeIfPresent(1, (key, value) -> null);
+    assertThat(notifications).hasSize(1);
+    CacheTesting.checkEmpty(cache);
+  }
+
   public void testUpdates() {
     cache.put(key, "1");
     // simultaneous update for same key, some null, some non-null
@@ -113,6 +142,39 @@ public void testCompute() {
     assertEquals(0, cache.size());
   }
 
+  //
+  public void testComputeWithLoad() {
+    Queue<RemovalNotification<String, String>> notifications = new ConcurrentLinkedQueue<>();
+    cache = CacheBuilder.newBuilder()
+        .removalListener(new RemovalListener<String, String>() {
+          @Override public void onRemoval(RemovalNotification<String, String> notification) {
+            notifications.add(notification);
+          }
+        })
+        .expireAfterAccess(500000, TimeUnit.MILLISECONDS)
+        .maximumSize(count)
+        .build();
+
+    cache.put(key, "1");
+    // simultaneous load and deletion
+    doParallelCacheOp(
+        count,
+        n -> {
+          try {
+            cache.get(key, () -> key);
+            cache.asMap().compute(key, (k, v) -> null);
+          } catch (ExecutionException e) {
+            throw new UncheckedExecutionException(e);
+          }
+        });
+
+    CacheTesting.checkEmpty(cache);
+    for (RemovalNotification<String, String> entry : notifications) {
+      assertThat(entry.getKey()).isNotNull();
+      assertThat(entry.getValue()).isNotNull();
+    }
+  }
+
   public void testComputeExceptionally() {
     try {
       doParallelCacheOp(

guava/src/com/google/common/cache/LocalCache.java

@@ -2188,7 +2188,7 @@ V waitForLoadingValue(ReferenceEntry<K, V> e, K key, ValueReference<K, V> valueR
     V compute(K key, int hash, BiFunction<? super K, ? super V, ? extends V> function) {
       ReferenceEntry<K, V> e;
       ValueReference<K, V> valueReference = null;
-      LoadingValueReference<K, V> loadingValueReference = null;
+      ComputingValueReference<K, V> loadingValueReference = null;
       boolean createNewEntry = true;
       V newValue;
 
@@ -2229,7 +2229,7 @@ V compute(K key, int hash, BiFunction<? super K, ? super V, ? extends V> functio
 
         // note valueReference can be an existing value or even itself another loading value if
         // the value for the key is already being computed.
-        loadingValueReference = new LoadingValueReference<>(valueReference);
+        loadingValueReference = new ComputingValueReference<>(valueReference);
 
         if (e == null) {
           createNewEntry = true;
@@ -2257,6 +2257,9 @@ V compute(K key, int hash, BiFunction<? super K, ? super V, ? extends V> functio
         } else if (createNewEntry) {
           removeLoadingValue(key, hash, loadingValueReference);
           return null;
+        } else if (valueReference.isLoading()) {
+          removeLoadingValue(key, hash, loadingValueReference);
+          return null;
         } else {
           removeEntry(e, hash, RemovalCause.EXPLICIT);
           return null;
@@ -3465,6 +3468,18 @@ void runUnlockedCleanup() {
     }
   }
 
+  static class ComputingValueReference<K, V> extends LoadingValueReference<K, V> {
+    public ComputingValueReference() {
+      super();
+    }
+    public ComputingValueReference(ValueReference<K, V> oldValue) {
+      super(oldValue);
+    }
+    @Override public boolean isLoading() {
+      return false;
+    }
+  }
+
   static class LoadingValueReference<K, V> implements ValueReference<K, V> {
     volatile ValueReference<K, V> oldValue;
 
@@ -3927,7 +3942,7 @@ long longSize() {
     Segment<K, V>[] segments = this.segments;
     long sum = 0;
     for (int i = 0; i < segments.length; ++i) {
-      sum += Math.max(0, segments[i].count); // see https://github.com/google/guava/issues/2108
+      sum += segments[i].count;
     }
     return sum;
   }