fix: drop useless mutex cs_llmq_vbc to avoid deadlock (#5749)

## Issue being fixed or feature implemented
Missing changes in https://github.com/dashpay/dash/pull/5736
The prior backport of https://github.com/bitcoin/bitcoin/pull/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 <UdjinM6@users.noreply.github.com>
This commit is contained in:
Konstantin Akimov 2023-12-04 17:38:47 +07:00 committed by Odysseas Gabrielides
parent bab3aa13ad
commit bd926a52a0
3 changed files with 4 additions and 13 deletions

View File

@ -60,10 +60,7 @@ LLMQContext::~LLMQContext() {
llmq::chainLocksHandler.reset(); llmq::chainLocksHandler.reset();
llmq::quorumManager.reset(); llmq::quorumManager.reset();
llmq::quorumBlockProcessor.reset(); llmq::quorumBlockProcessor.reset();
{ llmq::llmq_versionbitscache.Clear();
LOCK(llmq::cs_llmq_vbc);
llmq::llmq_versionbitscache.Clear();
}
} }
void LLMQContext::Interrupt() { void LLMQContext::Interrupt() {

View File

@ -35,7 +35,6 @@ std::optional<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(co
namespace llmq namespace llmq
{ {
Mutex cs_llmq_vbc;
VersionBitsCache llmq_versionbitscache; VersionBitsCache llmq_versionbitscache;
namespace utils namespace utils
@ -708,7 +707,6 @@ bool IsV19Active(gsl::not_null<const CBlockIndex*> pindex)
bool IsV20Active(gsl::not_null<const CBlockIndex*> pindex) bool IsV20Active(gsl::not_null<const CBlockIndex*> pindex)
{ {
LOCK(cs_llmq_vbc);
return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20) == ThresholdState::ACTIVE; return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20) == ThresholdState::ACTIVE;
} }
@ -716,19 +714,16 @@ bool IsMNRewardReallocationActive(gsl::not_null<const CBlockIndex*> pindex)
{ {
if (!IsV20Active(pindex)) return false; if (!IsV20Active(pindex)) return false;
LOCK(cs_llmq_vbc);
return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_MN_RR) == ThresholdState::ACTIVE; return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_MN_RR) == ThresholdState::ACTIVE;
} }
ThresholdState GetV20State(gsl::not_null<const CBlockIndex*> pindex) ThresholdState GetV20State(gsl::not_null<const CBlockIndex*> pindex)
{ {
LOCK(cs_llmq_vbc);
return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20); return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20);
} }
int GetV20Since(gsl::not_null<const CBlockIndex*> pindex) int GetV20Since(gsl::not_null<const CBlockIndex*> pindex)
{ {
LOCK(cs_llmq_vbc);
return llmq_versionbitscache.StateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20); return llmq_versionbitscache.StateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20);
} }
@ -1005,7 +1000,6 @@ bool IsQuorumTypeEnabledInternal(Consensus::LLMQType llmqType, const CQuorumMana
return true; return true;
case Consensus::LLMQType::LLMQ_TEST_V17: { case Consensus::LLMQType::LLMQ_TEST_V17: {
LOCK(cs_llmq_vbc);
return llmq_versionbitscache.State(pindex, consensusParams, Consensus::DEPLOYMENT_TESTDUMMY) == ThresholdState::ACTIVE; return llmq_versionbitscache.State(pindex, consensusParams, Consensus::DEPLOYMENT_TESTDUMMY) == ThresholdState::ACTIVE;
} }
case Consensus::LLMQType::LLMQ_100_67: case Consensus::LLMQType::LLMQ_100_67:

View File

@ -30,10 +30,10 @@ namespace llmq
class CQuorumManager; class CQuorumManager;
class CQuorumSnapshot; class CQuorumSnapshot;
// Use a separate cache instance instead of versionbitscache to avoid locking cs_main // A separate cache instance instead of versionbitscache has been introduced to avoid locking cs_main
// and dealing with all kinds of deadlocks. // and dealing with all kinds of deadlocks.
extern Mutex cs_llmq_vbc; // TODO: drop llmq_versionbitscache completely so far as VersionBitsCache do not uses anymore cs_main
extern VersionBitsCache llmq_versionbitscache GUARDED_BY(cs_llmq_vbc); extern VersionBitsCache llmq_versionbitscache;
static const bool DEFAULT_ENABLE_QUORUM_DATA_RECOVERY = true; static const bool DEFAULT_ENABLE_QUORUM_DATA_RECOVERY = true;