Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

fix deadlock in queue drop #3903

Merged
merged 1 commit into from
Dec 19, 2016
Merged

fix deadlock in queue drop #3903

merged 1 commit into from
Dec 19, 2016

Conversation

rphmeier
Copy link
Contributor

Closes #3885

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Dec 19, 2016
@rphmeier rphmeier requested a review from arkpar December 19, 2016 16:16
{
let _more = self.verification.more_to_verify.lock().unwrap();
}

// wake up all threads waiting for more work.
self.more_to_verify.notify_all();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This call should be made under the lock as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think that would cause a deadlock in itself, since any notified thread needs to have the lock returned from the wait call.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It should not. Condvar::wait makes sure that start of wait and release of the lock are synchronized.

@arkpar arkpar added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 19, 2016
@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Dec 19, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 85.843% when pulling ae8f77b on test-mem-limit-deadlock into e0f6f8f on master.

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 19, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.005%) to 85.846% when pulling ae8f77b on test-mem-limit-deadlock into e0f6f8f on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.005%) to 85.846% when pulling ae8f77b on test-mem-limit-deadlock into e0f6f8f on master.

@gavofyork gavofyork merged commit 18965be into master Dec 19, 2016
@gavofyork gavofyork deleted the test-mem-limit-deadlock branch December 19, 2016 21:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants