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 deadlock in injection_queue_depth_multi_thread test #6916

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jofas
Copy link
Contributor

@jofas jofas commented Oct 18, 2024

This closes #6847 by removing the deadlock from the test as discussed in #6876. The changes I made are based on this suggestion. I made the blocking tasks block by letting them wait on a message from a std::mpsc channel. This is quite convenient IMO, because we can exit the blocking tasks simply by dropping all Senders of the channel.1

Footnotes

  1. Unlike the previous implementation that used Barriers for synchronization—even if the test were to unforeseeably panic—the blocking tasks would still exit and cause the test to fail early instead of timing out.

if i != depth {
fail = Some(format!("{i} is not equal to {depth}"));
break;
'next_try: for _ in 0..10 {
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually do this kind of retrying by defining two functions, to separate out the concerns.

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.

Test injection_queue_depth_multi_thread is flaky
2 participants