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

Issue #342: CacheResponse race condition with has and get #434

Merged
merged 3 commits into from
Apr 7, 2023

Conversation

swichers
Copy link
Contributor

@swichers swichers commented Apr 7, 2023

@freekmurze
Copy link
Member

Could you summarise why this change is needed?

@swichers
Copy link
Contributor Author

swichers commented Apr 7, 2023

Discussion #342 goes into explicit detail.

#342 (comment)

There is a race condition when calling hasBeenCached and getCachedResponseFor.

hasBeenCached can return true and the cache can clear/expire before getCachedResponseFor is called, which then triggers an exception.

Technically this hasBeenCached call is redundant/less performant since it winds up doing a get() anyway ( #342 (reply in thread) ). I left it in place to minimize changes.

@freekmurze freekmurze merged commit 8f53ee8 into spatie:main Apr 7, 2023
17 checks passed
@freekmurze
Copy link
Member

Thank you for hunting down this bug!

@swichers swichers deleted the issue-342-has-get-race branch April 7, 2023 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants