Cache's computeIfPresent discards if error'd entry

[ben-manes]

It appears that we still have a bug in the compute logic. The test case below should retain the previous value when the compute throws an error. Instead the failed compute is left in the cache and the prior value can be read. When the next compute occurs, it treats the exception as a null old value, ignores the mappingFunction, and removes the entry.

public void testComputeIfPresent_error() {
  var cache = CacheBuilder.newBuilder().build();
  cache.put(key, "1");
  try {
    cache.asMap().computeIfPresent(key, (key, value) -> { throw new Error(); });
  } catch (Error e) {}
  assertEquals("1", cache.getIfPresent(key));
  assertEquals("2", cache.asMap().computeIfPresent(key, (k, v) -> "2")); // fails; removes mapping
}

Caffeine's test suite runs against a Guava shim in order to verify backwards compatibility. I was trying to update that to delegate to Guava's compute implementations after #5406.

[netdpb]

Thanks! Do you have a fix in mind? We would test and accept a PR.

[ben-manes]

I think we can catch the exception and unwind back to the prior entry, as it holds the segment lock throughout. I don't like how it leaves a zombie, which causes these quirks.

[asureshraja]

@netdpb @ben-manes I raised pull request for this issue. Can you please verify https://github.com/google/guava/pull/5896