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

Fixed the WaitableAtomic bug with lack of notifications of updates when updating from Wait() with predicate + mutating "getter". #1001

Merged
merged 3 commits into from
May 1, 2024

Conversation

dimacurrentai
Copy link
Contributor

I hope the first commit fails the tests, and then I push the fix in the second commit.

If this plan does not work out, I'll increase the "delay" and try the "failing" case again.

Fingers crossed!

@dimacurrentai
Copy link
Contributor Author

Good, both ubuntu and macos are hung on WaitableAtomic.NotifiesOfChangesInWait, as planned. Pushing the fix.

std::unique_lock<std::mutex> lock(data_mutex_);
if (!wait_predicate(data_)) {
const data_t& data = data_;
data_condition_variable_.wait(lock, [&wait_predicate, &data] { return wait_predicate(data); });
data_condition_variable_.wait(lock, [&wait_predicate, &data]() { return wait_predicate(data); });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

CC @mzhurovich @amdrozdov -- I didn't know lambdas can be declared w/o () before, whoa!

@dimacurrentai
Copy link
Contributor Author

@mzhurovich ready for your review!

@dkorolev dkorolev requested a review from mzhurovich April 30, 2024 16:48
@dkorolev dkorolev merged commit fec2e05 into C5T:stable May 1, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants