diff --git a/guava-tests/test/com/google/common/cache/LocalCacheTest.java b/guava-tests/test/com/google/common/cache/LocalCacheTest.java index 54e0737fee67..409b402d64a7 100644 --- a/guava-tests/test/com/google/common/cache/LocalCacheTest.java +++ b/guava-tests/test/com/google/common/cache/LocalCacheTest.java @@ -795,6 +795,76 @@ public void testComputeIfAbsent_removalListener() { assertThat(notifications).isEmpty(); } + public void testComputeIfAbsent_existingEntryWriteTimeNotChanged() { + FakeTicker ticker = new FakeTicker(); + LocalCache map = + makeLocalCache( + createCacheBuilder() + .concurrencyLevel(1) + .ticker(ticker) + .expireAfterWrite(10, NANOSECONDS)); + + Object key = new Object(); + Object value = new Object(); + map.put(key, value); + ReferenceEntry entry = map.getEntry(key); + long originalWriteTime = entry.getWriteTime(); + + ticker.advance(5, NANOSECONDS); + + // computeIfAbsent on an existing key should NOT update write time + Object result = map.computeIfAbsent(key, k -> new Object()); + assertThat(result).isSameInstanceAs(value); + assertThat(entry.getWriteTime()).isEqualTo(originalWriteTime); + } + + public void testComputeIfAbsent_existingEntryExpiresAfterWrite() { + FakeTicker ticker = new FakeTicker(); + LocalCache map = + makeLocalCache( + createCacheBuilder() + .concurrencyLevel(1) + .ticker(ticker) + .expireAfterWrite(10, NANOSECONDS)); + + Object key = new Object(); + Object value = new Object(); + map.put(key, value); + + // Repeatedly call computeIfAbsent before expiry + for (int i = 0; i < 5; i++) { + ticker.advance(3, NANOSECONDS); + map.computeIfAbsent(key, k -> new Object()); + } + + // After 15ns total (> 10ns expiry), the entry must be expired + assertThat(map.get(key)).isNull(); + } + + public void testCompute_returningNewValueUpdatesWriteTime() { + FakeTicker ticker = new FakeTicker(); + LocalCache map = + makeLocalCache( + createCacheBuilder() + .concurrencyLevel(1) + .ticker(ticker) + .expireAfterWrite(10, NANOSECONDS)); + + Object key = new Object(); + Object value = new Object(); + map.put(key, value); + ReferenceEntry entry = map.getEntry(key); + long originalWriteTime = entry.getWriteTime(); + + ticker.advance(5, NANOSECONDS); + + // compute() that returns a NEW value SHOULD update write time + Object newValue = new Object(); + Object result = map.compute(key, (k, v) -> newValue); + assertThat(result).isSameInstanceAs(newValue); + assertThat(entry.getWriteTime()).isGreaterThan(originalWriteTime); + } + public void testCopyEntry_computing() { CountDownLatch startSignal = new CountDownLatch(1); CountDownLatch computingSignal = new CountDownLatch(1); diff --git a/guava/src/com/google/common/cache/LocalCache.java b/guava/src/com/google/common/cache/LocalCache.java index c1e0ba26bd32..c33ecc961069 100644 --- a/guava/src/com/google/common/cache/LocalCache.java +++ b/guava/src/com/google/common/cache/LocalCache.java @@ -2275,7 +2275,16 @@ V waitForLoadingValue(ReferenceEntry e, K key, ValueReference valueR if (valueReference != null && newValue == valueReference.get()) { computingValueReference.set(newValue); e.setValueReference(valueReference); - recordWrite(e, 0, now); // no change in weight + // The value was not changed, so avoid resetting the write timestamp, + // which would incorrectly extend the expireAfterWrite deadline (see + // https://github.com/google/guava/issues/3700). + // We still need to re-add to both queues since the entry was removed + // from them above, and update the access time. + if (map.recordsAccess()) { + e.setAccessTime(now); + } + accessQueue.add(e); + writeQueue.add(e); return newValue; } try {