Skip to content

Commit b148b8d

Browse files
eamonnmcmanusGoogle Java Core Libraries
authored andcommitted
Don't store an exception thrown in computeIfAbsent etc.
It doesn't appear that the stored exception is ever used. Tests still pass after this deletion. 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 ce75979 commit b148b8d

File tree

2 files changed

+21
-8
lines changed

2 files changed

+21
-8
lines changed

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,23 @@ public void onRemoval(RemovalNotification<Integer, Integer> notification) {
125125
CacheTesting.checkEmpty(cache);
126126
}
127127

128+
public void testComputeIfPresent_error() {
129+
var cache = CacheBuilder.newBuilder().build();
130+
cache.put(key, "1");
131+
assertThrows(
132+
Error.class,
133+
() ->
134+
cache
135+
.asMap()
136+
.computeIfPresent(
137+
key,
138+
(k, v) -> {
139+
throw new Error();
140+
}));
141+
assertThat(cache.getIfPresent(key)).isEqualTo("1");
142+
assertThat(cache.asMap().computeIfPresent(key, (k, v) -> "2")).isEqualTo("2");
143+
}
144+
128145
public void testUpdates() {
129146
cache.put(key, "1");
130147
// simultaneous update for same key, some null, some non-null

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)