diff --git a/guava-tests/test/com/google/common/cache/LocalCacheMapComputeTest.java b/guava-tests/test/com/google/common/cache/LocalCacheMapComputeTest.java index 2c1e1e8cb4ab..3693915a4693 100644 --- a/guava-tests/test/com/google/common/cache/LocalCacheMapComputeTest.java +++ b/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> notifications = new ArrayList<>(); + Cache cache = CacheBuilder.newBuilder() + .removalListener(new RemovalListener() { + @Override public void onRemoval(RemovalNotification 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> notifications = new ConcurrentLinkedQueue<>(); + cache = CacheBuilder.newBuilder() + .removalListener(new RemovalListener() { + @Override public void onRemoval(RemovalNotification 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 entry : notifications) { + assertThat(entry.getKey()).isNotNull(); + assertThat(entry.getValue()).isNotNull(); + } + } + public void testComputeExceptionally() { try { doParallelCacheOp( diff --git a/guava/src/com/google/common/cache/LocalCache.java b/guava/src/com/google/common/cache/LocalCache.java index a485ad5970a6..7a292d8318b1 100644 --- a/guava/src/com/google/common/cache/LocalCache.java +++ b/guava/src/com/google/common/cache/LocalCache.java @@ -2188,7 +2188,7 @@ V waitForLoadingValue(ReferenceEntry e, K key, ValueReference valueR V compute(K key, int hash, BiFunction function) { ReferenceEntry e; ValueReference valueReference = null; - LoadingValueReference loadingValueReference = null; + ComputingValueReference loadingValueReference = null; boolean createNewEntry = true; V newValue; @@ -2229,7 +2229,7 @@ V compute(K key, int hash, BiFunction 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 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 extends LoadingValueReference { + public ComputingValueReference() { + super(); + } + public ComputingValueReference(ValueReference oldValue) { + super(oldValue); + } + @Override public boolean isLoading() { + return false; + } + } + static class LoadingValueReference implements ValueReference { volatile ValueReference oldValue; @@ -3927,7 +3942,7 @@ long longSize() { Segment[] 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; }