Skip to content

Commit 9b65365

Browse files
committedJan 14, 2025·
Avoid early expiration of an pending future due to delayed pinning (fixes #1623)
1 parent 25405d6 commit 9b65365

File tree

2 files changed

+57
-27
lines changed

2 files changed

+57
-27
lines changed
 

‎caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java

+33-27
Original file line numberDiff line numberDiff line change
@@ -1929,18 +1929,6 @@ public void run() {
19291929
accessOrderWindowDeque().offerLast(node);
19301930
}
19311931
}
1932-
1933-
// Ensure that in-flight async computation cannot expire (reset on a completion callback)
1934-
if (isComputingAsync(node.getValue())) {
1935-
synchronized (node) {
1936-
if (!Async.isReady((CompletableFuture<?>) node.getValue())) {
1937-
long expirationTime = expirationTicker().read() + ASYNC_EXPIRY;
1938-
setVariableTime(node, expirationTime);
1939-
setAccessTime(node, expirationTime);
1940-
setWriteTime(node, expirationTime);
1941-
}
1942-
}
1943-
}
19441932
}
19451933
}
19461934

@@ -2311,6 +2299,9 @@ public void putAll(Map<? extends K, ? extends V> map) {
23112299
node = nodeFactory.newNode(key, keyReferenceQueue(),
23122300
value, valueReferenceQueue(), newWeight, now);
23132301
setVariableTime(node, expireAfterCreate(key, value, expiry, now));
2302+
long expirationTime = isComputingAsync(value) ? (now + ASYNC_EXPIRY) : now;
2303+
setAccessTime(node, expirationTime);
2304+
setWriteTime(node, expirationTime);
23142305
}
23152306
prior = data.putIfAbsent(node.getKeyReference(), node);
23162307
if (prior == null) {
@@ -2391,21 +2382,22 @@ public void putAll(Map<? extends K, ? extends V> map) {
23912382
varTime = expireAfterUpdate(prior, key, value, expiry, now);
23922383
}
23932384

2385+
long expirationTime = isComputingAsync(value) ? (now + ASYNC_EXPIRY) : now;
23942386
if (mayUpdate) {
23952387
exceedsTolerance =
23962388
(expiresAfterWrite() && (now - prior.getWriteTime()) > EXPIRE_WRITE_TOLERANCE)
23972389
|| (expiresVariable()
23982390
&& Math.abs(varTime - prior.getVariableTime()) > EXPIRE_WRITE_TOLERANCE);
2391+
setWriteTime(prior, expirationTime);
23992392

24002393
prior.setValue(value, valueReferenceQueue());
24012394
prior.setWeight(newWeight);
2402-
setWriteTime(prior, now);
24032395

24042396
discardRefresh(prior.getKeyReference());
24052397
}
24062398

24072399
setVariableTime(prior, varTime);
2408-
setAccessTime(prior, now);
2400+
setAccessTime(prior, expirationTime);
24092401
}
24102402

24112403
if (expired) {
@@ -2422,9 +2414,6 @@ public void putAll(Map<? extends K, ? extends V> map) {
24222414
} else if (!onlyIfAbsent && exceedsTolerance) {
24232415
afterWrite(new UpdateTask(prior, weightedDifference));
24242416
} else {
2425-
if (mayUpdate) {
2426-
setWriteTime(prior, now);
2427-
}
24282417
afterRead(prior, now, /* recordHit= */ false);
24292418
}
24302419

@@ -2548,9 +2537,11 @@ public boolean remove(Object key, Object value) {
25482537
n.setValue(value, valueReferenceQueue());
25492538
n.setWeight(weight);
25502539

2540+
long expirationTime = isComputingAsync(value) ? (now[0] + ASYNC_EXPIRY) : now[0];
2541+
setAccessTime(n, expirationTime);
2542+
setWriteTime(n, expirationTime);
25512543
setVariableTime(n, varTime);
2552-
setAccessTime(n, now[0]);
2553-
setWriteTime(n, now[0]);
2544+
25542545
discardRefresh(k);
25552546
return n;
25562547
}
@@ -2605,9 +2596,10 @@ public boolean replace(K key, V oldValue, V newValue, boolean shouldDiscardRefre
26052596
n.setValue(newValue, valueReferenceQueue());
26062597
n.setWeight(weight);
26072598

2599+
long expirationTime = isComputingAsync(newValue) ? (now[0] + ASYNC_EXPIRY) : now[0];
2600+
setAccessTime(n, expirationTime);
2601+
setWriteTime(n, expirationTime);
26082602
setVariableTime(n, varTime);
2609-
setAccessTime(n, now[0]);
2610-
setWriteTime(n, now[0]);
26112603

26122604
if (shouldDiscardRefresh) {
26132605
discardRefresh(k);
@@ -2697,6 +2689,9 @@ public void replaceAll(BiFunction<? super K, ? super V, ? extends V> function) {
26972689
var created = nodeFactory.newNode(key, keyReferenceQueue(),
26982690
newValue[0], valueReferenceQueue(), weight[1], now[0]);
26992691
setVariableTime(created, expireAfterCreate(key, newValue[0], expiry(), now[0]));
2692+
long expirationTime = isComputingAsync(newValue[0]) ? (now[0] + ASYNC_EXPIRY) : now[0];
2693+
setAccessTime(created, expirationTime);
2694+
setWriteTime(created, expirationTime);
27002695
return created;
27012696
}
27022697

@@ -2730,8 +2725,14 @@ public void replaceAll(BiFunction<? super K, ? super V, ? extends V> function) {
27302725
n.setWeight(weight[1]);
27312726

27322727
setVariableTime(n, varTime);
2733-
setAccessTime(n, now[0]);
2734-
setWriteTime(n, now[0]);
2728+
if (isComputingAsync(newValue[0])) {
2729+
long expirationTime = now[0] + ASYNC_EXPIRY;
2730+
setAccessTime(n, expirationTime);
2731+
setWriteTime(n, expirationTime);
2732+
} else {
2733+
setAccessTime(n, now[0]);
2734+
setWriteTime(n, now[0]);
2735+
}
27352736
discardRefresh(k);
27362737
return n;
27372738
}
@@ -2867,9 +2868,12 @@ public void replaceAll(BiFunction<? super K, ? super V, ? extends V> function) {
28672868
long varTime = expireAfterCreate(key, newValue[0], expiry, now[0]);
28682869
var created = nodeFactory.newNode(keyRef, newValue[0],
28692870
valueReferenceQueue(), weight[1], now[0]);
2871+
2872+
long expirationTime = isComputingAsync(newValue[0]) ? (now[0] + ASYNC_EXPIRY) : now[0];
2873+
setAccessTime(created, expirationTime);
2874+
setWriteTime(created, expirationTime);
28702875
setVariableTime(created, varTime);
2871-
setAccessTime(created, now[0]);
2872-
setWriteTime(created, now[0]);
2876+
28732877
discardRefresh(key);
28742878
return created;
28752879
}
@@ -2920,9 +2924,11 @@ public void replaceAll(BiFunction<? super K, ? super V, ? extends V> function) {
29202924
n.setValue(newValue[0], valueReferenceQueue());
29212925
n.setWeight(weight[1]);
29222926

2927+
long expirationTime = isComputingAsync(newValue[0]) ? (now[0] + ASYNC_EXPIRY) : now[0];
2928+
setAccessTime(n, expirationTime);
2929+
setWriteTime(n, expirationTime);
29232930
setVariableTime(n, varTime);
2924-
setAccessTime(n, now[0]);
2925-
setWriteTime(n, now[0]);
2931+
29262932
discardRefresh(kr);
29272933
return n;
29282934
}

‎caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java

+24
Original file line numberDiff line numberDiff line change
@@ -2926,6 +2926,30 @@ public void expireAfterRead_disabled(BoundedLocalCache<Int, Int> cache, CacheCon
29262926
assertThat(duration).isEqualTo(expiresAt);
29272927
}
29282928

2929+
@Test(dataProvider = "caches")
2930+
@CacheSpec(mustExpireWithAnyOf = { AFTER_WRITE, VARIABLE },
2931+
expiry = { CacheExpiry.DISABLED, CacheExpiry.WRITE },
2932+
expireAfterWrite = {Expire.DISABLED, Expire.ONE_MINUTE}, expiryTime = Expire.ONE_MINUTE)
2933+
public void expireAfterWrite_writeTime(AsyncCache<Int, Int> cache, CacheContext context) {
2934+
var localCache = asBoundedLocalCache(cache);
2935+
localCache.setDrainStatusRelease(PROCESSING_TO_REQUIRED);
2936+
2937+
var future = new CompletableFuture<Int>();
2938+
cache.put(context.absentKey(), future);
2939+
assertThat(localCache.writeBuffer).isNotEmpty();
2940+
2941+
context.ticker().advance(Duration.ofMinutes(2));
2942+
future.complete(context.absentValue());
2943+
assertThat(localCache.writeBuffer).isNotEmpty();
2944+
2945+
localCache.setDrainStatusRelease(REQUIRED);
2946+
assertThat(cache.getIfPresent(context.absentKey())).isNotNull();
2947+
context.ticker().advance(Duration.ofSeconds(45));
2948+
assertThat(cache.getIfPresent(context.absentKey())).isNotNull();
2949+
context.ticker().advance(Duration.ofSeconds(15));
2950+
assertThat(cache.getIfPresent(context.absentKey())).isNull();
2951+
}
2952+
29292953
@Test
29302954
public void fixedExpireAfterWrite() {
29312955
int key = 1;

0 commit comments

Comments
 (0)
Please sign in to comment.