-
Notifications
You must be signed in to change notification settings - Fork 714
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
[BUG] Fix more thread issues #2925
[BUG] Fix more thread issues #2925
Conversation
log of the possible deadlock solved with the first commit:
|
d451bcc
to
f5ba264
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK f5ba264
Overall a very nice refactor removing those circular depends gg good improvement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK f5ba264
This PR fixes more thread issues that ThreadSanitizer showed:
first commit:
Solve a possible 3-threads deadlock fixed by locking
mempool.cs
beforecs_inventory
(see the comment for the log)second commit:
solve the trivial data race where
nodeState
is accessed without locking the corresponding mutexthird commit:
solve all the issues that the function
getQuorumNodes
had:it
iterator was accessed without having the lock oncs_vPendingMasternodes
which was causing a possible data raceconnman->vNodes
was accessed without locking the mutexcs_vNodes
(another possible data race)Solved by using the function
connman->ForEachNode
and removing the useless getter.fourth commit:
Solve a possible deadlock: lock
cs_vPendingMasternodes
before callingconnmann->ForEachNode(...)