From 5781bd5ee3bda35c5656b80c82cf36a38765a81d Mon Sep 17 00:00:00 2001 From: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com> Date: Thu, 28 Oct 2021 15:11:34 -0400 Subject: [PATCH] refactor: more llmq refactoring (#4552) * replace raw owning ptr with unique ptr Signed-off-by: pasta * Add GUARDED_BY annotation to llmq_versionbitscache Signed-off-by: pasta * limit scope of locking cs_llmq_vbc Signed-off-by: pasta * use llmq_versionbitscache instead of versionbitscache in UpdatedBlockTip to avoid cs_main locking Signed-off-by: pasta * drop unneeded cs_main ::mempool.cs * lock cs_main and mempool.cs in Db::Upgrade --- src/llmq/chainlocks.cpp | 8 +++----- src/llmq/chainlocks.h | 4 ++-- src/llmq/instantsend.cpp | 9 +++------ src/llmq/utils.cpp | 5 +---- src/llmq/utils.h | 2 +- 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/llmq/chainlocks.cpp b/src/llmq/chainlocks.cpp index afda84351c..589430a15a 100644 --- a/src/llmq/chainlocks.cpp +++ b/src/llmq/chainlocks.cpp @@ -37,17 +37,15 @@ std::string CChainLockSig::ToString() const CChainLocksHandler::CChainLocksHandler() { - scheduler = new CScheduler(); - CScheduler::Function serviceLoop = std::bind(&CScheduler::serviceQueue, scheduler); - scheduler_thread = new std::thread(std::bind(&TraceThread, "cl-schdlr", serviceLoop)); + scheduler = std::make_unique(); + CScheduler::Function serviceLoop = std::bind(&CScheduler::serviceQueue, scheduler.get()); + scheduler_thread = std::make_unique(std::bind(&TraceThread, "cl-schdlr", serviceLoop)); } CChainLocksHandler::~CChainLocksHandler() { scheduler->stop(); scheduler_thread->join(); - delete scheduler_thread; - delete scheduler; } void CChainLocksHandler::Start() diff --git a/src/llmq/chainlocks.h b/src/llmq/chainlocks.h index 85538353eb..d44b9da341 100644 --- a/src/llmq/chainlocks.h +++ b/src/llmq/chainlocks.h @@ -52,8 +52,8 @@ class CChainLocksHandler : public CRecoveredSigsListener static constexpr int64_t WAIT_FOR_ISLOCK_TIMEOUT = 10 * 60; private: - CScheduler* scheduler; - std::thread* scheduler_thread; + std::unique_ptr scheduler; + std::unique_ptr scheduler_thread; mutable CCriticalSection cs; std::atomic tryLockChainTipScheduled{false}; std::atomic isEnabled{false}; diff --git a/src/llmq/instantsend.cpp b/src/llmq/instantsend.cpp index 5d47c39a59..c7d3b52047 100644 --- a/src/llmq/instantsend.cpp +++ b/src/llmq/instantsend.cpp @@ -58,11 +58,9 @@ CInstantSendDb::CInstantSendDb(bool unitTests, bool fWipe) db = std::make_unique(unitTests ? "" : (GetDataDir() / "llmq/isdb"), 32 << 20, unitTests, fWipe); } -void CInstantSendDb::Upgrade() EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs) +void CInstantSendDb::Upgrade() { - AssertLockHeld(cs_main); - AssertLockHeld(::mempool.cs); - + LOCK2(cs_main, ::mempool.cs); LOCK(cs_db); int v{0}; if (!db->Read(DB_VERSION, v) || v < CInstantSendDb::CURRENT_VERSION) { @@ -1270,8 +1268,7 @@ void CInstantSendManager::NotifyChainLock(const CBlockIndex* pindexChainLock) void CInstantSendManager::UpdatedBlockTip(const CBlockIndex* pindexNew) { if (!fUpgradedDB) { - LOCK2(cs_main, ::mempool.cs); // for GetTransaction in Upgrade - if (VersionBitsState(pindexNew, Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0020, versionbitscache) == ThresholdState::ACTIVE) { + if (WITH_LOCK(cs_llmq_vbc, return VersionBitsState(pindexNew, Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0020, llmq_versionbitscache) == ThresholdState::ACTIVE)) { db.Upgrade(); fUpgradedDB = true; } diff --git a/src/llmq/utils.cpp b/src/llmq/utils.cpp index 5592fc91c9..b78857cd99 100644 --- a/src/llmq/utils.cpp +++ b/src/llmq/utils.cpp @@ -301,10 +301,7 @@ bool CLLMQUtils::IsQuorumActive(Consensus::LLMQType llmqType, const uint256& quo bool CLLMQUtils::IsQuorumTypeEnabled(Consensus::LLMQType llmqType, const CBlockIndex* pindex) { - LOCK(cs_llmq_vbc); - const Consensus::Params& consensusParams = Params().GetConsensus(); - bool f_dip0020_Active = VersionBitsState(pindex, consensusParams, Consensus::DEPLOYMENT_DIP0020, llmq_versionbitscache) == ThresholdState::ACTIVE; switch (llmqType) { @@ -314,7 +311,7 @@ bool CLLMQUtils::IsQuorumTypeEnabled(Consensus::LLMQType llmqType, const CBlockI break; case Consensus::LLMQType::LLMQ_100_67: case Consensus::LLMQType::LLMQ_TEST_V17: - if (!f_dip0020_Active) { + if (LOCK(cs_llmq_vbc); VersionBitsState(pindex, consensusParams, Consensus::DEPLOYMENT_DIP0020, llmq_versionbitscache) != ThresholdState::ACTIVE) { return false; } break; diff --git a/src/llmq/utils.h b/src/llmq/utils.h index 1708ed7424..c7de9b9f4f 100644 --- a/src/llmq/utils.h +++ b/src/llmq/utils.h @@ -25,7 +25,7 @@ namespace llmq // Use a separate cache instance instead of versionbitscache to avoid locking cs_main // and dealing with all kinds of deadlocks. extern CCriticalSection cs_llmq_vbc; -extern VersionBitsCache llmq_versionbitscache; +extern VersionBitsCache llmq_versionbitscache GUARDED_BY(cs_llmq_vbc); static const bool DEFAULT_ENABLE_QUORUM_DATA_RECOVERY = true;