Skip to content

Commit eb3f601

Browse files
knstUdjinM6
andauthored
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 41b3418 commit eb3f601

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
@@ -693,27 +692,23 @@ bool IsV19Active(gsl::not_null<const CBlockIndex*> pindex)
693692

694693
bool IsV20Active(gsl::not_null<const CBlockIndex*> pindex)
695694
{
696-
LOCK(cs_llmq_vbc);
697695
return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20) == ThresholdState::ACTIVE;
698696
}
699697

700698
bool IsMNRewardReallocationActive(gsl::not_null<const CBlockIndex*> pindex)
701699
{
702700
if (!IsV20Active(pindex)) return false;
703701

704-
LOCK(cs_llmq_vbc);
705702
return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_MN_RR) == ThresholdState::ACTIVE;
706703
}
707704

708705
ThresholdState GetV20State(gsl::not_null<const CBlockIndex*> pindex)
709706
{
710-
LOCK(cs_llmq_vbc);
711707
return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20);
712708
}
713709

714710
int GetV20Since(gsl::not_null<const CBlockIndex*> pindex)
715711
{
716-
LOCK(cs_llmq_vbc);
717712
return llmq_versionbitscache.StateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20);
718713
}
719714

@@ -980,7 +975,6 @@ bool IsQuorumTypeEnabledInternal(Consensus::LLMQType llmqType, const CQuorumMana
980975
return true;
981976

982977
case Consensus::LLMQType::LLMQ_TEST_V17: {
983-
LOCK(cs_llmq_vbc);
984978
return llmq_versionbitscache.State(pindex, consensusParams, Consensus::DEPLOYMENT_TESTDUMMY) == ThresholdState::ACTIVE;
985979
}
986980
case Consensus::LLMQType::LLMQ_100_67:

src/llmq/utils.h

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

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

3737
static const bool DEFAULT_ENABLE_QUORUM_DATA_RECOVERY = true;
3838

0 commit comments

Comments
 (0)