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

Jump to bottom

[BUG] Fix lock order issue with CMMan::CheckAndRemove and CMPayments::CleanPaymentList #2023

Merged
merged 3 commits into from Dec 4, 2020
Merged

[BUG] Fix lock order issue with CMMan::CheckAndRemove and CMPayments::CleanPaymentList #2023

merged 3 commits into from Dec 4, 2020

Conversation

random-zebra
Copy link

Fixes the following inconsistency (https://github.com/PIVX-Project/PIVX/runs/1491040884):

  2020-12-03 07:55:03 POTENTIAL DEADLOCK DETECTED
  2020-12-03 07:55:03 Previous lock order was:
  2020-12-03 07:55:03  (1) cs masternodeman.cpp:481 (in thread pivx-msghand)
  2020-12-03 07:55:03  (2) cs_mapMasternodeBlocks masternode-payments.cpp:483 (in thread pivx-msghand)
  2020-12-03 07:55:03 Current lock order is:
  2020-12-03 07:55:03  cs_mapMasternodePayeeVotes masternode-payments.cpp:619 (in thread pivx-masternodeman)
  2020-12-03 07:55:03  (2) cs_mapMasternodeBlocks masternode-payments.cpp:619 (in thread pivx-masternodeman)

Remove the "cleanup" during DB read (move it at the beginning of the thread).
Pass the masternode-count and current-height to CleanPaymentList, so there is no call to the masternode manager from within it.

Note: GA will keep failing as there are other locking issues. They will be addressed in separate PR(s).

@random-zebra random-zebra added this to the 5.0.0 milestone Dec 3, 2020
@random-zebra random-zebra self-assigned this Dec 3, 2020
furszy
furszy reviewed Dec 3, 2020
View reviewed changes
src/masternodeman.cpp Outdated Show resolved Hide resolved
@random-zebra
Cleanup: Don't clean payments directly from CMasternodePaymentDB::Read
1ec9813 
Remove fDryRun options, and perform the initial cleanup when the
relative thread starts. Remove redundant locks
furszy
furszy reviewed Dec 3, 2020
View reviewed changes
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

Code review ACK 58f5ff4, looking good.

src/masternodeman.cpp Outdated Show resolved Hide resolved
furszy
furszy approved these changes Dec 3, 2020
View reviewed changes
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

utACK c46ffac

@furszy furszy requested a review from Fuzzbawls December 3, 2020 22:20
Fuzzbawls
Fuzzbawls approved these changes Dec 4, 2020
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

utACK c46ffac

@Fuzzbawls Fuzzbawls merged commit f9a29a9 into PIVX-Project:master Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@furszy furszy furszy approved these changes

@Fuzzbawls Fuzzbawls Fuzzbawls approved these changes

Assignees

@random-zebra random-zebra

Labels
Masternode Refactoring
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@random-zebra @furszy @Fuzzbawls