-
Notifications
You must be signed in to change notification settings - Fork 11k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix deadlock if compute function throw a RuntimeException #2799
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
This should rethrow Throwable in case of an Error. |
@ben-manes : You mean instead of catching "just" the RuntimeException (which actually is rethrown) to catch a Throwable, right? Else, I don't know which "Error" you mean. |
Yes, use Throwable. Otherwise a java.lang.Error or sneakily thrown checked exception will not be handled. |
Well, thanks to Java 8 that one was easy to fix. Done! |
I'd like to ask if there is something else I can do that this fix gets merged? |
I would also like to see this fix merged and released soon as I've seen a couple separate projects hit deadlocks on cache Thanks! |
We also experienced this issue on PROD so also would like to see it merged and released ASAP as we use this cache extensively in our projects. |
Maybe have a look at https://github.com/ben-manes/caffeine which does not suffer from this bug? |
I think this was fixed in 8210828 (25.1) |
Closing this PR as it looks like the bug was fixed. |
If the computeIfAbsent methods fails with a runtime exception, successive calls to computeIfAbsent lock due to a call to oldValue.waitForValue which never will receive a value then.
A test case to reproduce this behavior - the second computeIfAbsent will never return and your app is frozen.
The pull request restores the previous state of the cache on RuntimeException and the test will finish correctly.
`public class TestGuava extends TestCase
{
private Cache<String, Boolean> cache = CacheBuilder.newBuilder()
.maximumSize(10000)
.expireAfterAccess(30, TimeUnit.SECONDS)
.build();
private Map<String, Boolean> cacheMap = cache.asMap();
}`