From 4820a6338ba15c2f41099af3508519d97f60a016 Mon Sep 17 00:00:00 2001 From: ale Date: Mon, 25 Mar 2024 18:59:35 +0100 Subject: [PATCH 1/5] promote quorumSigSharesManager to unique pointer --- src/llmq/quorums_init.cpp | 5 ++--- src/llmq/quorums_signing_shares.cpp | 2 +- src/llmq/quorums_signing_shares.h | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/llmq/quorums_init.cpp b/src/llmq/quorums_init.cpp index ec8f97cc967d7..d47ffb5e0539b 100644 --- a/src/llmq/quorums_init.cpp +++ b/src/llmq/quorums_init.cpp @@ -27,7 +27,7 @@ void InitLLMQSystem(CEvoDB& evoDb, CScheduler* scheduler, bool unitTests) quorumBlockProcessor.reset(new CQuorumBlockProcessor(evoDb)); quorumDKGSessionManager.reset(new CDKGSessionManager(evoDb, *blsWorker)); quorumManager.reset(new CQuorumManager(evoDb, *blsWorker, *quorumDKGSessionManager)); - quorumSigSharesManager = new CSigSharesManager(); + quorumSigSharesManager.reset(new CSigSharesManager()); quorumSigningManager = new CSigningManager(unitTests); chainLocksHandler = new CChainLocksHandler(scheduler); } @@ -38,8 +38,7 @@ void DestroyLLMQSystem() chainLocksHandler = nullptr; delete quorumSigningManager; quorumSigningManager = nullptr; - delete quorumSigSharesManager; - quorumSigSharesManager = nullptr; + quorumSigSharesManager.reset(); quorumDKGSessionManager.reset(); quorumBlockProcessor.reset(); quorumDKGDebugManager.reset(); diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index 3521ba84d4a78..67b7c2bd0829f 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -22,7 +22,7 @@ namespace llmq { -CSigSharesManager* quorumSigSharesManager = nullptr; +std::unique_ptr quorumSigSharesManager{nullptr}; template static std::pair FindBySignHash(const M& m, const uint256& signHash) diff --git a/src/llmq/quorums_signing_shares.h b/src/llmq/quorums_signing_shares.h index 4071194f086c5..0bcae9e8093c4 100644 --- a/src/llmq/quorums_signing_shares.h +++ b/src/llmq/quorums_signing_shares.h @@ -259,7 +259,7 @@ class CSigSharesManager void WorkThreadMain(); }; -extern CSigSharesManager* quorumSigSharesManager; +extern std::unique_ptr quorumSigSharesManager; } // namespace llmq From 4f579eb29177a6e5d269c48dbe94987d27e32d20 Mon Sep 17 00:00:00 2001 From: ale Date: Mon, 25 Mar 2024 19:03:25 +0100 Subject: [PATCH 2/5] promote quorumSigningManager to unique pointer --- src/llmq/quorums_init.cpp | 5 ++--- src/llmq/quorums_signing.cpp | 2 +- src/llmq/quorums_signing.h | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/llmq/quorums_init.cpp b/src/llmq/quorums_init.cpp index d47ffb5e0539b..c178952fb7653 100644 --- a/src/llmq/quorums_init.cpp +++ b/src/llmq/quorums_init.cpp @@ -28,7 +28,7 @@ void InitLLMQSystem(CEvoDB& evoDb, CScheduler* scheduler, bool unitTests) quorumDKGSessionManager.reset(new CDKGSessionManager(evoDb, *blsWorker)); quorumManager.reset(new CQuorumManager(evoDb, *blsWorker, *quorumDKGSessionManager)); quorumSigSharesManager.reset(new CSigSharesManager()); - quorumSigningManager = new CSigningManager(unitTests); + quorumSigningManager.reset(new CSigningManager(unitTests)); chainLocksHandler = new CChainLocksHandler(scheduler); } @@ -36,8 +36,7 @@ void DestroyLLMQSystem() { delete chainLocksHandler; chainLocksHandler = nullptr; - delete quorumSigningManager; - quorumSigningManager = nullptr; + quorumSigningManager.reset(); quorumSigSharesManager.reset(); quorumDKGSessionManager.reset(); quorumBlockProcessor.reset(); diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index 445f3fe5d640d..f240c07e7d36b 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -21,7 +21,7 @@ namespace llmq { -CSigningManager* quorumSigningManager; +std::unique_ptr quorumSigningManager{nullptr}; CRecoveredSigsDb::CRecoveredSigsDb(bool fMemory) : db(fMemory ? "" : (GetDataDir() / "llmq"), 1 << 20, fMemory, false, CLIENT_VERSION | ADDRV2_FORMAT) { diff --git a/src/llmq/quorums_signing.h b/src/llmq/quorums_signing.h index 342fb02b5e056..35db658e965ba 100644 --- a/src/llmq/quorums_signing.h +++ b/src/llmq/quorums_signing.h @@ -156,7 +156,7 @@ class CSigningManager bool VerifyRecoveredSig(Consensus::LLMQType llmqType, int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig); }; -extern CSigningManager* quorumSigningManager; +extern std::unique_ptr quorumSigningManager; } // namespace llmq From 8e85ca21ea94a8a25f4567fea487c695b8700fd1 Mon Sep 17 00:00:00 2001 From: ale Date: Mon, 25 Mar 2024 19:06:08 +0100 Subject: [PATCH 3/5] promote chainLocksHandler to unique pointer --- src/llmq/quorums_chainlocks.cpp | 2 +- src/llmq/quorums_chainlocks.h | 4 +--- src/llmq/quorums_init.cpp | 5 ++--- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp index 3586689e16704..4c0afdda3243f 100644 --- a/src/llmq/quorums_chainlocks.cpp +++ b/src/llmq/quorums_chainlocks.cpp @@ -20,7 +20,7 @@ namespace llmq static const std::string CLSIG_REQUESTID_PREFIX = "clsig"; -CChainLocksHandler* chainLocksHandler; +std::unique_ptr chainLocksHandler{nullptr}; std::string CChainLockSig::ToString() const { diff --git a/src/llmq/quorums_chainlocks.h b/src/llmq/quorums_chainlocks.h index 57a0f4e50b8b9..48e5d9573485e 100644 --- a/src/llmq/quorums_chainlocks.h +++ b/src/llmq/quorums_chainlocks.h @@ -90,9 +90,7 @@ class CChainLocksHandler : public CRecoveredSigsListener void Cleanup(); }; -extern CChainLocksHandler* chainLocksHandler; - - +extern std::unique_ptr chainLocksHandler; } #endif //PIVX_QUORUMS_CHAINLOCKS_H \ No newline at end of file diff --git a/src/llmq/quorums_init.cpp b/src/llmq/quorums_init.cpp index c178952fb7653..72c585f0abef1 100644 --- a/src/llmq/quorums_init.cpp +++ b/src/llmq/quorums_init.cpp @@ -29,13 +29,12 @@ void InitLLMQSystem(CEvoDB& evoDb, CScheduler* scheduler, bool unitTests) quorumManager.reset(new CQuorumManager(evoDb, *blsWorker, *quorumDKGSessionManager)); quorumSigSharesManager.reset(new CSigSharesManager()); quorumSigningManager.reset(new CSigningManager(unitTests)); - chainLocksHandler = new CChainLocksHandler(scheduler); + chainLocksHandler.reset(new CChainLocksHandler(scheduler)); } void DestroyLLMQSystem() { - delete chainLocksHandler; - chainLocksHandler = nullptr; + chainLocksHandler.reset(); quorumSigningManager.reset(); quorumSigSharesManager.reset(); quorumDKGSessionManager.reset(); From 581eabebe97e463543e9e69aa436c0ee853d7664 Mon Sep 17 00:00:00 2001 From: ale Date: Mon, 25 Mar 2024 20:13:58 +0100 Subject: [PATCH 4/5] Add Interrupt step for signing_share thread and do NOT call StopWorkerThread twice Squash where I add interrupt --- src/init.cpp | 1 + src/llmq/quorums_init.cpp | 7 +++++++ src/llmq/quorums_init.h | 1 + src/llmq/quorums_signing_shares.cpp | 22 +++++++++++----------- src/llmq/quorums_signing_shares.h | 3 ++- src/test/test_pivx.cpp | 1 + src/tiertwo/init.cpp | 5 +++++ src/tiertwo/init.h | 3 +++ 8 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 22eb3529d03e0..d78145ef58e19 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -191,6 +191,7 @@ void Interrupt() InterruptREST(); InterruptTorControl(); InterruptMapPort(); + InterruptTierTwo(); if (g_connman) g_connman->Interrupt(); } diff --git a/src/llmq/quorums_init.cpp b/src/llmq/quorums_init.cpp index 72c585f0abef1..220314a779ecf 100644 --- a/src/llmq/quorums_init.cpp +++ b/src/llmq/quorums_init.cpp @@ -71,4 +71,11 @@ void StopLLMQSystem() } } +void InterruptLLMQSystem() +{ + if (quorumSigSharesManager) { + quorumSigSharesManager->Interrupt(); + } +} + } // namespace llmq diff --git a/src/llmq/quorums_init.h b/src/llmq/quorums_init.h index f062113f3f77a..756e1744ff56c 100644 --- a/src/llmq/quorums_init.h +++ b/src/llmq/quorums_init.h @@ -21,6 +21,7 @@ void DestroyLLMQSystem(); // Manage scheduled tasks, threads, listeners etc. void StartLLMQSystem(); void StopLLMQSystem(); +void InterruptLLMQSystem(); } // namespace llmq diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index 67b7c2bd0829f..35ce5221e1c96 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -179,32 +179,30 @@ CSigSharesInv CBatchedSigShares::ToInv() const CSigSharesManager::CSigSharesManager() { + interruptSigningShare.reset(); } CSigSharesManager::~CSigSharesManager() { - StopWorkerThread(); } void CSigSharesManager::StartWorkerThread() { - workThread = std::thread(&TraceThread>, "quorum-sigshares", [this]() { - WorkThreadMain(); - }); + workThread = std::thread(&TraceThread>, "quorum-sigshares", std::function(std::bind(&CSigSharesManager::WorkThreadMain, this))); } void CSigSharesManager::StopWorkerThread() { - if (stopWorkThread) { - return; - } - - stopWorkThread = true; if (workThread.joinable()) { workThread.join(); } } +void CSigSharesManager::Interrupt() +{ + interruptSigningShare(); +} + void CSigSharesManager::ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, CConnman& connman) { // non-masternodes are not interested in sigshares @@ -1105,7 +1103,7 @@ void CSigSharesManager::BanNode(NodeId nodeId) void CSigSharesManager::WorkThreadMain() { int64_t lastProcessTime = GetTimeMillis(); - while (!stopWorkThread && !ShutdownRequested()) { + while (!interruptSigningShare) { RemoveBannedNodeStates(); quorumSigningManager->ProcessPendingRecoveredSigs(*g_connman); ProcessPendingSigShares(*g_connman); @@ -1115,7 +1113,9 @@ void CSigSharesManager::WorkThreadMain() quorumSigningManager->Cleanup(); // TODO Wakeup when pending signing is needed? - MilliSleep(100); + if (!interruptSigningShare.sleep_for(std::chrono::milliseconds(100))) { + return; + } } } diff --git a/src/llmq/quorums_signing_shares.h b/src/llmq/quorums_signing_shares.h index 0bcae9e8093c4..a97a2c394ce81 100644 --- a/src/llmq/quorums_signing_shares.h +++ b/src/llmq/quorums_signing_shares.h @@ -199,7 +199,7 @@ class CSigSharesManager RecursiveMutex cs; std::thread workThread; - std::atomic stopWorkThread{false}; + CThreadInterrupt interruptSigningShare; std::map sigShares; std::map firstSeenForSessions; @@ -221,6 +221,7 @@ class CSigSharesManager void StartWorkerThread(); void StopWorkerThread(); + void Interrupt(); public: void ProcessMessage(CNode* pnode, const std::string& strCommand, CDataStream& vRecv, CConnman& connman); diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp index 4523b7565e568..1b012d0ebef38 100644 --- a/src/test/test_pivx.cpp +++ b/src/test/test_pivx.cpp @@ -141,6 +141,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha TestingSetup::~TestingSetup() { scheduler.stop(); + llmq::InterruptLLMQSystem(); threadGroup.interrupt_all(); threadGroup.join_all(); GetMainSignals().FlushBackgroundCallbacks(); diff --git a/src/tiertwo/init.cpp b/src/tiertwo/init.cpp index bd3fe070e66c1..dfc9174f999a7 100644 --- a/src/tiertwo/init.cpp +++ b/src/tiertwo/init.cpp @@ -328,3 +328,8 @@ void DeleteTierTwo() deterministicMNManager.reset(); evoDb.reset(); } + +void InterruptTierTwo() +{ + llmq::InterruptLLMQSystem(); +} diff --git a/src/tiertwo/init.h b/src/tiertwo/init.h index 521123fb9b861..a80e8fd147857 100644 --- a/src/tiertwo/init.h +++ b/src/tiertwo/init.h @@ -56,5 +56,8 @@ void StopTierTwoThreads(); /** Cleans manager and worker objects pointers */ void DeleteTierTwo(); +/** Interrupt tier two threads */ +void InterruptTierTwo(); + #endif //PIVX_TIERTWO_INIT_H From 44116cf9f0eaf9855d10aeb179990b72dd79b556 Mon Sep 17 00:00:00 2001 From: ale Date: Mon, 25 Mar 2024 21:02:12 +0100 Subject: [PATCH 5/5] Separate Init from Start and Stop from Destroy functions for Chainlock handler --- src/llmq/quorums_chainlocks.cpp | 10 +++++++++- src/llmq/quorums_chainlocks.h | 2 ++ src/llmq/quorums_init.cpp | 10 ++++++++-- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp index 4c0afdda3243f..e907b5bd4b3b0 100644 --- a/src/llmq/quorums_chainlocks.cpp +++ b/src/llmq/quorums_chainlocks.cpp @@ -30,10 +30,18 @@ std::string CChainLockSig::ToString() const CChainLocksHandler::CChainLocksHandler(CScheduler* _scheduler) : scheduler(_scheduler) { - quorumSigningManager->RegisterRecoveredSigsListener(this); } CChainLocksHandler::~CChainLocksHandler() +{ +} + +void CChainLocksHandler::Start() +{ + quorumSigningManager->RegisterRecoveredSigsListener(this); +} + +void CChainLocksHandler::Stop() { quorumSigningManager->UnregisterRecoveredSigsListener(this); } diff --git a/src/llmq/quorums_chainlocks.h b/src/llmq/quorums_chainlocks.h index 48e5d9573485e..237312d427284 100644 --- a/src/llmq/quorums_chainlocks.h +++ b/src/llmq/quorums_chainlocks.h @@ -64,6 +64,8 @@ class CChainLocksHandler : public CRecoveredSigsListener public: CChainLocksHandler(CScheduler* _scheduler); ~CChainLocksHandler(); + void Start(); + void Stop(); public: bool AlreadyHave(const CInv& inv); diff --git a/src/llmq/quorums_init.cpp b/src/llmq/quorums_init.cpp index 220314a779ecf..eb9964e61888c 100644 --- a/src/llmq/quorums_init.cpp +++ b/src/llmq/quorums_init.cpp @@ -56,16 +56,22 @@ void StartLLMQSystem() if (quorumSigSharesManager) { quorumSigSharesManager->StartWorkerThread(); } + if (chainLocksHandler) { + chainLocksHandler->Start(); + } } void StopLLMQSystem() { - if (quorumDKGSessionManager) { - quorumDKGSessionManager->StopThreads(); + if (chainLocksHandler) { + chainLocksHandler->Stop(); } if (quorumSigSharesManager) { quorumSigSharesManager->StopWorkerThread(); } + if (quorumDKGSessionManager) { + quorumDKGSessionManager->StopThreads(); + } if (blsWorker) { blsWorker->Stop(); }