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

Fix Rust build #631

Merged
merged 1 commit into from
Oct 19, 2021
Merged

Fix Rust build #631

merged 1 commit into from
Oct 19, 2021

Conversation

oowekyala
Copy link
Collaborator

Fixes the build issues found after merging #488 into master.

@oowekyala
Copy link
Collaborator Author

Everything looks green. For reference, the bug was caused by two threads trying to access the same physical action concurrently. This is a data race, but the smart pointer we use detected that and panicked. I replaced it with a mutex. I'm not sure how to reproduce this in a test case, as the faulty behavior is only observable in a certain interleaving of threads. That's also why the test failed only unpredictably.

@lhstrh lhstrh merged commit 724a68c into tmp-master-with-rust Oct 19, 2021
@lhstrh lhstrh deleted the fix-rust-build branch October 19, 2021 17:00
@Soroosh129
Copy link
Contributor

Everything looks green. For reference, the bug was caused by two threads trying to access the same physical action concurrently. This is a data race, but the smart pointer we use detected that and panicked. I replaced it with a mutex. I'm not sure how to reproduce this in a test case, as the faulty behavior is only observable in a certain interleaving of threads. That's also why the test failed only unpredictably.

Would you please create an issue for this?

@lhstrh
Copy link
Member

lhstrh commented Oct 19, 2021

I think this PR already fixes the issue @oowekyala describes here.

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.

3 participants