Skip to content

Commit bd926a5

Browse files
knstUdjinM6
authored andcommitted
fix: drop useless mutex cs_llmq_vbc to avoid deadlock (dashpay#5749)
## Issue being fixed or feature implemented Missing changes in dashpay#5736 The prior backport of bitcoin#19438 has been needed to this particular changes: drop the mutex `cs_llmq_vbc`. This mutex can potentially cause deadlock such as: ``` 'cs_dip3list' in qt/masternodelist.cpp:135 (TRY) (in thread 'main') (2) 'cs_llmq_vbc' in llmq/utils.cpp:704 (in thread 'main') 'm_mutex' in versionbits.cpp:253 (in thread 'main') (1) 'cs_main' in node/blockstorage.cpp:77 (in thread 'main') Current lock order is: 'cs_Shutdown' in init.cpp:220 (TRY) (in thread 'shutoff') (1) 'cs_main' in init.cpp:328 (in thread 'shutoff') (2) 'llmq::cs_llmq_vbc' in llmq/context.cpp:64 (in thread 'shutoff') Assertion failed: detected inconsistent lock order for 'llmq::cs_llmq_vbc' in llmq/context.cpp:64 (in thread 'shutoff'), details in debug log. ``` ## What was done? Drop `cs_llmq_vbc` mutex from llmq/utils ## How Has This Been Tested? Re-started app several times -> no other deadlock happens. ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone --------- Co-authored-by: UdjinM6 <[email protected]>
1 parent bab3aa1 commit bd926a5

File tree

3 files changed

+4
-13
lines changed

3 files changed

+4
-13
lines changed

src/llmq/context.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,7 @@ LLMQContext::~LLMQContext() {
6060
llmq::chainLocksHandler.reset();
6161
llmq::quorumManager.reset();
6262
llmq::quorumBlockProcessor.reset();
63-
{
64-
LOCK(llmq::cs_llmq_vbc);
65-
llmq::llmq_versionbitscache.Clear();
66-
}
63+
llmq::llmq_versionbitscache.Clear();
6764
}
6865

6966
void LLMQContext::Interrupt() {

src/llmq/utils.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ std::optional<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(co
3535
namespace llmq
3636
{
3737

38-
Mutex cs_llmq_vbc;
3938
VersionBitsCache llmq_versionbitscache;
4039

4140
namespace utils
@@ -708,27 +707,23 @@ bool IsV19Active(gsl::not_null<const CBlockIndex*> pindex)
708707

709708
bool IsV20Active(gsl::not_null<const CBlockIndex*> pindex)
710709
{
711-
LOCK(cs_llmq_vbc);
712710
return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20) == ThresholdState::ACTIVE;
713711
}
714712

715713
bool IsMNRewardReallocationActive(gsl::not_null<const CBlockIndex*> pindex)
716714
{
717715
if (!IsV20Active(pindex)) return false;
718716

719-
LOCK(cs_llmq_vbc);
720717
return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_MN_RR) == ThresholdState::ACTIVE;
721718
}
722719

723720
ThresholdState GetV20State(gsl::not_null<const CBlockIndex*> pindex)
724721
{
725-
LOCK(cs_llmq_vbc);
726722
return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20);
727723
}
728724

729725
int GetV20Since(gsl::not_null<const CBlockIndex*> pindex)
730726
{
731-
LOCK(cs_llmq_vbc);
732727
return llmq_versionbitscache.StateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20);
733728
}
734729

@@ -1005,7 +1000,6 @@ bool IsQuorumTypeEnabledInternal(Consensus::LLMQType llmqType, const CQuorumMana
10051000
return true;
10061001

10071002
case Consensus::LLMQType::LLMQ_TEST_V17: {
1008-
LOCK(cs_llmq_vbc);
10091003
return llmq_versionbitscache.State(pindex, consensusParams, Consensus::DEPLOYMENT_TESTDUMMY) == ThresholdState::ACTIVE;
10101004
}
10111005
case Consensus::LLMQType::LLMQ_100_67:

src/llmq/utils.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ namespace llmq
3030
class CQuorumManager;
3131
class CQuorumSnapshot;
3232

33-
// Use a separate cache instance instead of versionbitscache to avoid locking cs_main
33+
// A separate cache instance instead of versionbitscache has been introduced to avoid locking cs_main
3434
// and dealing with all kinds of deadlocks.
35-
extern Mutex cs_llmq_vbc;
36-
extern VersionBitsCache llmq_versionbitscache GUARDED_BY(cs_llmq_vbc);
35+
// TODO: drop llmq_versionbitscache completely so far as VersionBitsCache do not uses anymore cs_main
36+
extern VersionBitsCache llmq_versionbitscache;
3737

3838
static const bool DEFAULT_ENABLE_QUORUM_DATA_RECOVERY = true;
3939

0 commit comments

Comments
 (0)