Skip to content

Commit 217afb9

Browse files
LocalCache: Fix compute() to report COLLECTED removal cause for GC'd entries
When compute() returns null for an entry whose value has been garbage collected, the removal cause should be COLLECTED, not EXPLICIT. The fix checks if the original value was null and the reference is still active (not loading), which indicates the value was garbage collected rather than explicitly removed by the user. Fixes #7985
1 parent 50f087f commit 217afb9

File tree

2 files changed

+45
-1
lines changed

2 files changed

+45
-1
lines changed

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,6 +1077,44 @@ public void testRemovalListener_collected() {
10771077
assertThat(listener.isEmpty()).isTrue();
10781078
}
10791079

1080+
// Test for https://github.com/google/guava/issues/7985
1081+
// When compute() returns null for a collected entry, the removal cause should be COLLECTED.
1082+
public void testComputeRemovalCause_collected() {
1083+
QueuingRemovalListener<Object, Object> listener = queuingRemovalListener();
1084+
LocalCache<Object, Object> map =
1085+
makeLocalCache(
1086+
createCacheBuilder().concurrencyLevel(1).softValues().removalListener(listener));
1087+
Segment<Object, Object> segment = map.segments[0];
1088+
assertThat(listener.isEmpty()).isTrue();
1089+
1090+
Object key = new Object();
1091+
Object value = new Object();
1092+
1093+
// Put an entry into the cache
1094+
map.put(key, value);
1095+
assertThat(listener.isEmpty()).isTrue();
1096+
1097+
// Get the entry and clear its value reference to simulate garbage collection
1098+
int hash = map.hash(key);
1099+
ReferenceEntry<Object, Object> entry = segment.getEntry(key, hash);
1100+
ValueReference<Object, Object> valueReference = entry.getValueReference();
1101+
1102+
// Simulate garbage collection by clearing the value reference
1103+
((java.lang.ref.Reference<?>) valueReference).clear();
1104+
1105+
// Now call compute() with a function that returns null
1106+
// Since the value was collected, the removal cause should be COLLECTED, not EXPLICIT
1107+
map.compute(key, (k, v) -> {
1108+
// The value should be null since it was collected
1109+
assertThat(v).isNull();
1110+
return null;
1111+
});
1112+
1113+
// Verify the removal notification has COLLECTED cause
1114+
assertNotified(listener, key, null, RemovalCause.COLLECTED);
1115+
assertThat(listener.isEmpty()).isTrue();
1116+
}
1117+
10801118
public void testRemovalListener_expired() {
10811119
FakeTicker ticker = new FakeTicker();
10821120
QueuingRemovalListener<Object, Object> listener = queuingRemovalListener();

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2287,7 +2287,13 @@ V waitForLoadingValue(ReferenceEntry<K, V> e, K key, ValueReference<K, V> valueR
22872287
removeLoadingValue(key, hash, computingValueReference);
22882288
return null;
22892289
} else {
2290-
removeEntry(e, hash, RemovalCause.EXPLICIT);
2290+
// If the value was garbage collected, report COLLECTED; otherwise EXPLICIT.
2291+
V oldValue = valueReference.get();
2292+
RemovalCause cause =
2293+
(oldValue == null && valueReference.isActive())
2294+
? RemovalCause.COLLECTED
2295+
: RemovalCause.EXPLICIT;
2296+
removeEntry(e, hash, cause);
22912297
return null;
22922298
}
22932299
} finally {

0 commit comments

Comments
 (0)