Skip to content
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

EvictableCache race condition can temporarily revive old entry #22285

Closed
findepi opened this issue Jun 5, 2024 · 1 comment · Fixed by #22302
Closed

EvictableCache race condition can temporarily revive old entry #22285

findepi opened this issue Jun 5, 2024 · 1 comment · Fixed by #22302
Labels
bug Something isn't working correctness Flaky Tests

Comments

@findepi
Copy link
Member

findepi commented Jun 5, 2024

TestEvictableLoadingCache.testInvalidateAndLoadConcurrently was found to be flaky (with probability ~0.000003 on my local machine).

found by @anusudarsan

@findepi findepi added bug Something isn't working correctness labels Jun 5, 2024
@findepi
Copy link
Member Author

findepi commented Jun 5, 2024

I think this is indeed possible to get a stale value:


     Thread A                       |   Thread B                                        |   Thread C
----------------------------------------------------------------------------------------------------------------------------------------------
ENTER invalidate(42)
invalidations ++
                                                                                            change remote state
                                                                                            ENTER invalidate(42)
                                                                                            invalidations ++
                                        ENTER get(42, loader)
                                        i = invalidations
                                        t = tokens.computeIfAbsent(42, -> new) // old
t = tokens.remove(42) // found 
                                        val = dataCache.get(t, loader) // from cache
                                                                                            t = tokens.remove(42) // not found
                                                                                            EXIT invalidate(42)
                                        if i == invalidations // true
                                        tokens.putIfAbsent(42, t) // "Revive"
                                                                                            ENTER get(42, loader)
                                                                                            t = tokens.computeIfAbsent(42, -> new) // old
                                                                                            val = dataCache.get(t, loader) // from cache
                                                                                            return val                             // ‼️  old value returned despite explicit invalidation
                                                                                            EXIT get(42, loader)
                                        if !dataCache.containsKey(t) // false
                                        return val
                                        EXIT get(42, loader)
dataCache.invalidate(t) // this fixes the temporarily incorrect state
EXIT invalidate(42)

Indeed, if i revert the "Revive" code (17faae3), i can no longer reproduce the problem

@findepi findepi changed the title EvictableCache race condition EvictableCache race condition can temporarily revive old entry Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctness Flaky Tests
Development

Successfully merging a pull request may close this issue.

1 participant