Skip to content

Commit a54e525

Browse files
eamonnmcmanusGoogle Java Core Libraries
authored andcommitted
Improve exception handling in cache.asMap().computeIfAbsent etc.
Don't store an exception thrown by the compute function. It doesn't appear that the stored exception is ever used. Tests still pass after this deletion. Also move the evaluation of the compute function to before the possible creation of a new entry, so if it throws an exception then no entry is created. Fixes #5438, fixes #7625, fixes #7975. RELNOTES=Handling of exceptions from compute functions in `Cache.asMap().computeIfAbsent` and the like has been improved. (We do still recommend using Caffeine rather than `com.google.common.cache`.) PiperOrigin-RevId: 805040984
1 parent 2de6495 commit a54e525

File tree

2 files changed

+5
-9
lines changed

2 files changed

+5
-9
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ public void testComputeIfPresent_error() {
139139
throw new Error();
140140
}));
141141
assertThat(cache.getIfPresent(key)).isEqualTo("1");
142-
assertThat(cache.asMap().computeIfPresent(key, (k, v) -> "2")).isEqualTo(null);
142+
assertThat(cache.asMap().computeIfPresent(key, (k, v) -> "2")).isEqualTo("2");
143143
}
144144

145145
public void testUpdates() {

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2259,6 +2259,9 @@ V waitForLoadingValue(ReferenceEntry<K, V> e, K key, ValueReference<K, V> valueR
22592259
// the value for the key is already being computed.
22602260
computingValueReference = new ComputingValueReference<>(valueReference);
22612261

2262+
// Compute the value now, so if it throws an exception we don't change anything.
2263+
newValue = computingValueReference.compute(key, function);
2264+
22622265
if (e == null) {
22632266
createNewEntry = true;
22642267
e = newEntry(key, hash, first);
@@ -2268,7 +2271,6 @@ V waitForLoadingValue(ReferenceEntry<K, V> e, K key, ValueReference<K, V> valueR
22682271
e.setValueReference(computingValueReference);
22692272
}
22702273

2271-
newValue = computingValueReference.compute(key, function);
22722274
if (newValue != null) {
22732275
if (valueReference != null && newValue == valueReference.get()) {
22742276
computingValueReference.set(newValue);
@@ -3585,13 +3587,7 @@ public ListenableFuture<V> loadFuture(K key, CacheLoader<? super K, V> loader) {
35853587
} catch (ExecutionException e) {
35863588
previousValue = null;
35873589
}
3588-
V newValue;
3589-
try {
3590-
newValue = function.apply(key, previousValue);
3591-
} catch (Throwable th) {
3592-
this.setException(th);
3593-
throw th;
3594-
}
3590+
V newValue = function.apply(key, previousValue);
35953591
this.set(newValue);
35963592
return newValue;
35973593
}

0 commit comments

Comments
 (0)