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

Prevent race condition in get_with method to avoid evaluating init closure/future multiple times #195

Merged
merged 3 commits into from
Nov 5, 2022

Conversation

tatsuya6502
Copy link
Member

@tatsuya6502 tatsuya6502 commented Nov 4, 2022

In the current implementation (up to v0.9.5), there is a small chance that the following methods to evaluate the init closure/function more than once in concurrent calls:

  • get_with
  • get_with_if
  • optionally_get_with
  • try_get_with

It will happen when a call to one of these methods is made when another call to the same method on the same key is about to complete.

New test cases in he following files will reproduce the issue:

  • tests/entry_api_sync.rs
  • tests/entry_api_tokio.rs
  • tests/entry_api_actix_rt2.rs
  • tests/entry_api_async_std.rs

This PR fixes the issue by doing a double check if the value still does not exist after the lock is acquired and also extending the lock as needed.

`init` closure/future multiple times

In the current implementation, there is a small chance that the following methods to
evaluate the `init` closure/function more than once in concurrent calls:

- `get_with`
- `get_with_if`
- `optionally_get_with`
- `try_get_with`

It will happen when a call to one of these methods is made at the time when another
call to the same method is about to complete.

New test cases in `tests/entry_api_sync.rs` will reproduce the issue.

This commit contains the fix by doing a double check if the value still does not
exist after the lock is acquired and also extending the lock as needed.
@tatsuya6502 tatsuya6502 changed the title Prevent race condition in get_with methods to avoid evaluating init closure/future multiple times Prevent race condition in get_with method to avoid evaluating init closure/future multiple times Nov 4, 2022
@tatsuya6502 tatsuya6502 added the bug Something isn't working label Nov 4, 2022
@tatsuya6502 tatsuya6502 added this to the v0.9.6 milestone Nov 4, 2022
`init` closure/future multiple times

Add async test cases:
- `tests/entry_api_actix_rt2.rs`
- `tests/entry_api_async_std.rs`
- `tests/entry_api_tokio.rs`
init closure/future multiple times

Cosmetic changes mostly in source code comments.
Copy link
Member Author

@tatsuya6502 tatsuya6502 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging.

@tatsuya6502 tatsuya6502 merged commit 52febeb into master Nov 5, 2022
@tatsuya6502 tatsuya6502 deleted the prevent-eval-init-more-than-once branch November 5, 2022 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant