From c15c8093cc4de78f649efbeb3f3b96d000310db0 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 4 Dec 2023 01:37:51 +0700 Subject: [PATCH 1/3] fix: drop useless mutex cs_llmq_vbc This mutex can potentially cause deadlock '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. --- src/llmq/context.cpp | 4 ---- src/llmq/utils.cpp | 6 ------ src/llmq/utils.h | 3 +-- 3 files changed, 1 insertion(+), 12 deletions(-) diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index 2162f36ed366..a7f77c2fb069 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -60,10 +60,6 @@ LLMQContext::~LLMQContext() { llmq::chainLocksHandler.reset(); llmq::quorumManager.reset(); llmq::quorumBlockProcessor.reset(); - { - LOCK(llmq::cs_llmq_vbc); - llmq::llmq_versionbitscache.Clear(); - } } void LLMQContext::Interrupt() { diff --git a/src/llmq/utils.cpp b/src/llmq/utils.cpp index f171634c5d10..4cf0532978f2 100644 --- a/src/llmq/utils.cpp +++ b/src/llmq/utils.cpp @@ -35,7 +35,6 @@ std::optional> GetNonNullCoinbaseChainlock(co namespace llmq { -Mutex cs_llmq_vbc; VersionBitsCache llmq_versionbitscache; namespace utils @@ -693,7 +692,6 @@ bool IsV19Active(gsl::not_null pindex) bool IsV20Active(gsl::not_null pindex) { - LOCK(cs_llmq_vbc); return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20) == ThresholdState::ACTIVE; } @@ -701,19 +699,16 @@ bool IsMNRewardReallocationActive(gsl::not_null pindex) { if (!IsV20Active(pindex)) return false; - LOCK(cs_llmq_vbc); return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_MN_RR) == ThresholdState::ACTIVE; } ThresholdState GetV20State(gsl::not_null pindex) { - LOCK(cs_llmq_vbc); return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20); } int GetV20Since(gsl::not_null pindex) { - LOCK(cs_llmq_vbc); return llmq_versionbitscache.StateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20); } @@ -980,7 +975,6 @@ bool IsQuorumTypeEnabledInternal(Consensus::LLMQType llmqType, const CQuorumMana return true; case Consensus::LLMQType::LLMQ_TEST_V17: { - LOCK(cs_llmq_vbc); return llmq_versionbitscache.State(pindex, consensusParams, Consensus::DEPLOYMENT_TESTDUMMY) == ThresholdState::ACTIVE; } case Consensus::LLMQType::LLMQ_100_67: diff --git a/src/llmq/utils.h b/src/llmq/utils.h index f9e37dfed4b2..cad4b212699a 100644 --- a/src/llmq/utils.h +++ b/src/llmq/utils.h @@ -31,8 +31,7 @@ class CQuorumSnapshot; // Use a separate cache instance instead of versionbitscache to avoid locking cs_main // and dealing with all kinds of deadlocks. -extern Mutex cs_llmq_vbc; -extern VersionBitsCache llmq_versionbitscache GUARDED_BY(cs_llmq_vbc); +extern VersionBitsCache llmq_versionbitscache; static const bool DEFAULT_ENABLE_QUORUM_DATA_RECOVERY = true; From e4aa2103b6585d948f3373c8a078416aeb471ffc Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 4 Dec 2023 02:22:36 +0700 Subject: [PATCH 2/3] chore: add TODO to drop llmq_versionbitscache completely --- src/llmq/utils.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/llmq/utils.h b/src/llmq/utils.h index cad4b212699a..49d11375a34f 100644 --- a/src/llmq/utils.h +++ b/src/llmq/utils.h @@ -29,8 +29,9 @@ namespace llmq class CQuorumManager; 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. +// TODO: drop llmq_versionbitscache completely so far as VersionBitsCache do not uses anymore cs_main extern VersionBitsCache llmq_versionbitscache; static const bool DEFAULT_ENABLE_QUORUM_DATA_RECOVERY = true; From fa95a3abdd0e98d705bb4d4e70561c5fe9ec8c4b Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sun, 3 Dec 2023 23:57:20 +0300 Subject: [PATCH 3/3] keep `llmq::llmq_versionbitscache.Clear();` --- src/llmq/context.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index a7f77c2fb069..35836755619a 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -60,6 +60,7 @@ LLMQContext::~LLMQContext() { llmq::chainLocksHandler.reset(); llmq::quorumManager.reset(); llmq::quorumBlockProcessor.reset(); + llmq::llmq_versionbitscache.Clear(); } void LLMQContext::Interrupt() {