trivial: use std::atomic to protect connected signer pointers

kwvg · kwvg · commit b7700b3d473b · 2025-09-09T19:02:47.000Z
diff --git a/src/chainlock/chainlock.cpp b/src/chainlock/chainlock.cpp
@@ -64,17 +64,18 @@ CChainLocksHandler::~CChainLocksHandler()
 
 void CChainLocksHandler::Start(const llmq::CInstantSendManager& isman)
 {
-    if (m_signer) {
-        m_signer->Start();
+    if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+        signer->Start();
     }
     scheduler->scheduleEvery(
         [&]() {
+            auto signer = m_signer.load(std::memory_order_acquire);
             CheckActiveState();
             EnforceBestChainLock();
             Cleanup();
             // regularly retry signing the current chaintip as it might have failed before due to missing islocks
-            if (m_signer) {
-                m_signer->TrySignChainTip(isman);
+            if (signer) {
+                signer->TrySignChainTip(isman);
             }
         },
         std::chrono::seconds{5});
@@ -83,8 +84,8 @@ void CChainLocksHandler::Start(const llmq::CInstantSendManager& isman)
 void CChainLocksHandler::Stop()
 {
     scheduler->stop();
-    if (m_signer) {
-        m_signer->Stop();
+    if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+        signer->Stop();
     }
 }
 
@@ -218,11 +219,12 @@ void CChainLocksHandler::UpdatedBlockTip(const llmq::CInstantSendManager& isman)
     if (bool expected = false; tryLockChainTipScheduled.compare_exchange_strong(expected, true)) {
         scheduler->scheduleFromNow(
             [&]() {
+                auto signer = m_signer.load(std::memory_order_acquire);
                 CheckActiveState();
                 EnforceBestChainLock();
                 Cleanup();
-                if (m_signer) {
-                    m_signer->TrySignChainTip(isman);
+                if (signer) {
+                    signer->TrySignChainTip(isman);
                 }
                 tryLockChainTipScheduled = false;
             },
@@ -274,16 +276,16 @@ void CChainLocksHandler::BlockConnected(const std::shared_ptr<const CBlock>& pbl
     }
 
     // We need this information later when we try to sign a new tip, so that we can determine if all included TXs are safe.
-    if (m_signer) {
-        m_signer->UpdateBlockHashTxidMap(pindex->GetBlockHash(), pblock->vtx);
+    if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+        signer->UpdateBlockHashTxidMap(pindex->GetBlockHash(), pblock->vtx);
     }
 }
 
 void CChainLocksHandler::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock,
                                            gsl::not_null<const CBlockIndex*> pindexDisconnected)
 {
-    if (m_signer) {
-        m_signer->EraseFromBlockHashTxidMap(pindexDisconnected->GetBlockHash());
+    if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+        signer->EraseFromBlockHashTxidMap(pindexDisconnected->GetBlockHash());
     }
 }
 
@@ -451,8 +453,8 @@ void CChainLocksHandler::Cleanup()
         }
     }
 
-    if (m_signer) {
-        const auto cleanup_txes{m_signer->Cleanup()};
+    if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+        const auto cleanup_txes{signer->Cleanup()};
         LOCK(cs);
         for (const auto& tx : cleanup_txes) {
             for (const auto& txid : *tx) {
diff --git a/src/chainlock/chainlock.h b/src/chainlock/chainlock.h
@@ -46,7 +46,7 @@ class CChainLocksHandler final : public chainlock::ChainLockSignerParent
     std::unique_ptr<CScheduler> scheduler;
     std::unique_ptr<std::thread> scheduler_thread;
 
-    chainlock::ChainLockSigner* m_signer{nullptr};
+    std::atomic<chainlock::ChainLockSigner*> m_signer{nullptr};
 
     mutable Mutex cs;
     std::atomic<bool> tryLockChainTipScheduled{false};
@@ -73,10 +73,10 @@ class CChainLocksHandler final : public chainlock::ChainLockSignerParent
     void ConnectSigner(gsl::not_null<chainlock::ChainLockSigner*> signer)
     {
         // Prohibit double initialization
-        assert(m_signer == nullptr);
-        m_signer = signer;
+        assert(m_signer.load(std::memory_order_acquire) == nullptr);
+        m_signer.store(signer, std::memory_order_release);
     }
-    void DisconnectSigner() { m_signer = nullptr; }
+    void DisconnectSigner() { m_signer.store(nullptr, std::memory_order_release); }
 
     void Start(const llmq::CInstantSendManager& isman);
     void Stop();
diff --git a/src/instantsend/instantsend.cpp b/src/instantsend/instantsend.cpp
@@ -75,15 +75,15 @@ void CInstantSendManager::Start(PeerManager& peerman)
 
     workThread = std::thread(&util::TraceThread, "isman", [this, &peerman] { WorkThreadMain(peerman); });
 
-    if (m_signer) {
-        m_signer->Start();
+    if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+        signer->Start();
     }
 }
 
 void CInstantSendManager::Stop()
 {
-    if (m_signer) {
-        m_signer->Stop();
+    if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+        signer->Stop();
     }
 
     // make sure to call InterruptWorkerThread() first
@@ -348,8 +348,8 @@ MessageProcessingResult CInstantSendManager::ProcessInstantSendLock(NodeId from,
     LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s, islock=%s: processing islock, peer=%d\n",
              __func__, islock->txid.ToString(), hash.ToString(), from);
 
-    if (m_signer) {
-        m_signer->ClearLockFromQueue(islock);
+    if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+        signer->ClearLockFromQueue(islock);
     }
     if (db.KnownInstantSendLock(hash)) {
         return {};
@@ -449,8 +449,8 @@ void CInstantSendManager::TransactionAddedToMempool(const CTransactionRef& tx)
     }
 
     if (islock == nullptr) {
-        if (m_signer) {
-            m_signer->ProcessTx(*tx, false, Params().GetConsensus());
+        if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+            signer->ProcessTx(*tx, false, Params().GetConsensus());
         }
         // TX is not locked, so make sure it is tracked
         AddNonLockedTx(tx, nullptr);
@@ -491,8 +491,8 @@ void CInstantSendManager::BlockConnected(const std::shared_ptr<const CBlock>& pb
             }
 
             if (!IsLocked(tx->GetHash()) && !has_chainlock) {
-                if (m_signer) {
-                    m_signer->ProcessTx(*tx, true, Params().GetConsensus());
+                if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+                    signer->ProcessTx(*tx, true, Params().GetConsensus());
                 }
                 // TX is not locked, so make sure it is tracked
                 AddNonLockedTx(tx, pindex);
@@ -597,16 +597,16 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChild
 void CInstantSendManager::RemoveConflictedTx(const CTransaction& tx)
 {
     RemoveNonLockedTx(tx.GetHash(), false);
-    if (m_signer) {
-        m_signer->ClearInputsFromQueue(GetIdsFromLockable(tx.vin));
+    if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+        signer->ClearInputsFromQueue(GetIdsFromLockable(tx.vin));
     }
 }
 
 void CInstantSendManager::TruncateRecoveredSigsForInputs(const instantsend::InstantSendLock& islock)
 {
     auto ids = GetIdsFromLockable(islock.inputs);
-    if (m_signer) {
-        m_signer->ClearInputsFromQueue(ids);
+    if (auto signer = m_signer.load(std::memory_order_acquire); signer) {
+        signer->ClearInputsFromQueue(ids);
     }
     for (const auto& id : ids) {
         sigman.TruncateRecoveredSig(Params().GetConsensus().llmqTypeDIP0024InstantSend, id);
@@ -925,7 +925,8 @@ void CInstantSendManager::WorkThreadMain(PeerManager& peerman)
             for (auto& [node_id, mpr] : peer_activity) {
                 peerman.PostProcessMessage(std::move(mpr), node_id);
             }
-            if (!m_signer) return more_work;
+            auto signer = m_signer.load(std::memory_order_acquire);
+            if (!signer) return more_work;
             // Construct set of non-locked transactions that are pending to retry
             std::vector<CTransactionRef> txns{};
             {
@@ -942,7 +943,7 @@ void CInstantSendManager::WorkThreadMain(PeerManager& peerman)
                 }
             }
             // Retry processing them
-            m_signer->ProcessPendingRetryLockTxs(txns);
+            signer->ProcessPendingRetryLockTxs(txns);
             return more_work;
         }();
 
diff --git a/src/instantsend/instantsend.h b/src/instantsend/instantsend.h
@@ -59,7 +59,7 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent
     CTxMemPool& mempool;
     const CMasternodeSync& m_mn_sync;
 
-    instantsend::InstantSendSigner* m_signer{nullptr};
+    std::atomic<instantsend::InstantSendSigner*> m_signer{nullptr};
 
     std::thread workThread;
     CThreadInterrupt workInterrupt;
@@ -97,10 +97,10 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent
     void ConnectSigner(gsl::not_null<instantsend::InstantSendSigner*> signer)
     {
         // Prohibit double initialization
-        assert(m_signer == nullptr);
-        m_signer = signer;
+        assert(m_signer.load(std::memory_order_acquire) == nullptr);
+        m_signer.store(signer, std::memory_order_release);
     }
-    void DisconnectSigner() { m_signer = nullptr; }
+    void DisconnectSigner() { m_signer.store(nullptr, std::memory_order_release); }
 
     void Start(PeerManager& peerman);
     void Stop();

Original file line number	Diff line number	Diff line change
`@@ -64,17 +64,18 @@ CChainLocksHandler::~CChainLocksHandler()`
`64`	`64`
`65`	`65`	`void CChainLocksHandler::Start(const llmq::CInstantSendManager& isman)`
`66`	`66`	`{`
`67`		`- if (m_signer) {`
`68`		`- m_signer->Start();`
	`67`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`68`	`+ signer->Start();`
`69`	`69`	`}`
`70`	`70`	`scheduler->scheduleEvery(`
`71`	`71`	`[&]() {`
	`72`	`+ auto signer = m_signer.load(std::memory_order_acquire);`
`72`	`73`	`CheckActiveState();`
`73`	`74`	`EnforceBestChainLock();`
`74`	`75`	`Cleanup();`
`75`	`76`	`// regularly retry signing the current chaintip as it might have failed before due to missing islocks`
`76`		`- if (m_signer) {`
`77`		`- m_signer->TrySignChainTip(isman);`
	`77`	`+ if (signer) {`
	`78`	`+ signer->TrySignChainTip(isman);`
`78`	`79`	`}`
`79`	`80`	`},`
`80`	`81`	`std::chrono::seconds{5});`
`@@ -83,8 +84,8 @@ void CChainLocksHandler::Start(const llmq::CInstantSendManager& isman)`
`83`	`84`	`void CChainLocksHandler::Stop()`
`84`	`85`	`{`
`85`	`86`	`scheduler->stop();`
`86`		`- if (m_signer) {`
`87`		`- m_signer->Stop();`
	`87`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`88`	`+ signer->Stop();`
`88`	`89`	`}`
`89`	`90`	`}`
`90`	`91`
`@@ -218,11 +219,12 @@ void CChainLocksHandler::UpdatedBlockTip(const llmq::CInstantSendManager& isman)`
`218`	`219`	`if (bool expected = false; tryLockChainTipScheduled.compare_exchange_strong(expected, true)) {`
`219`	`220`	`scheduler->scheduleFromNow(`
`220`	`221`	`[&]() {`
	`222`	`+ auto signer = m_signer.load(std::memory_order_acquire);`
`221`	`223`	`CheckActiveState();`
`222`	`224`	`EnforceBestChainLock();`
`223`	`225`	`Cleanup();`
`224`		`- if (m_signer) {`
`225`		`- m_signer->TrySignChainTip(isman);`
	`226`	`+ if (signer) {`
	`227`	`+ signer->TrySignChainTip(isman);`
`226`	`228`	`}`
`227`	`229`	`tryLockChainTipScheduled = false;`
`228`	`230`	`},`
`@@ -274,16 +276,16 @@ void CChainLocksHandler::BlockConnected(const std::shared_ptr<const CBlock>& pbl`
`274`	`276`	`}`
`275`	`277`
`276`	`278`	`// We need this information later when we try to sign a new tip, so that we can determine if all included TXs are safe.`
`277`		`- if (m_signer) {`
`278`		`- m_signer->UpdateBlockHashTxidMap(pindex->GetBlockHash(), pblock->vtx);`
	`279`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`280`	`+ signer->UpdateBlockHashTxidMap(pindex->GetBlockHash(), pblock->vtx);`
`279`	`281`	`}`
`280`	`282`	`}`
`281`	`283`
`282`	`284`	`void CChainLocksHandler::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock,`
`283`	`285`	`gsl::not_null<const CBlockIndex*> pindexDisconnected)`
`284`	`286`	`{`
`285`		`- if (m_signer) {`
`286`		`- m_signer->EraseFromBlockHashTxidMap(pindexDisconnected->GetBlockHash());`
	`287`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`288`	`+ signer->EraseFromBlockHashTxidMap(pindexDisconnected->GetBlockHash());`
`287`	`289`	`}`
`288`	`290`	`}`
`289`	`291`
`@@ -451,8 +453,8 @@ void CChainLocksHandler::Cleanup()`
`451`	`453`	`}`
`452`	`454`	`}`
`453`	`455`
`454`		`- if (m_signer) {`
`455`		`- const auto cleanup_txes{m_signer->Cleanup()};`
	`456`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`457`	`+ const auto cleanup_txes{signer->Cleanup()};`
`456`	`458`	`LOCK(cs);`
`457`	`459`	`for (const auto& tx : cleanup_txes) {`
`458`	`460`	`for (const auto& txid : *tx) {`
Original file line number	Diff line number	Diff line change
`@@ -75,15 +75,15 @@ void CInstantSendManager::Start(PeerManager& peerman)`
`75`	`75`
`76`	`76`	`workThread = std::thread(&util::TraceThread, "isman", [this, &peerman] { WorkThreadMain(peerman); });`
`77`	`77`
`78`		`- if (m_signer) {`
`79`		`- m_signer->Start();`
	`78`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`79`	`+ signer->Start();`
`80`	`80`	`}`
`81`	`81`	`}`
`82`	`82`
`83`	`83`	`void CInstantSendManager::Stop()`
`84`	`84`	`{`
`85`		`- if (m_signer) {`
`86`		`- m_signer->Stop();`
	`85`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`86`	`+ signer->Stop();`
`87`	`87`	`}`
`88`	`88`
`89`	`89`	`// make sure to call InterruptWorkerThread() first`
`@@ -348,8 +348,8 @@ MessageProcessingResult CInstantSendManager::ProcessInstantSendLock(NodeId from,`
`348`	`348`	`LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s, islock=%s: processing islock, peer=%d\n",`
`349`	`349`	`__func__, islock->txid.ToString(), hash.ToString(), from);`
`350`	`350`
`351`		`- if (m_signer) {`
`352`		`- m_signer->ClearLockFromQueue(islock);`
	`351`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`352`	`+ signer->ClearLockFromQueue(islock);`
`353`	`353`	`}`
`354`	`354`	`if (db.KnownInstantSendLock(hash)) {`
`355`	`355`	`return {};`
`@@ -449,8 +449,8 @@ void CInstantSendManager::TransactionAddedToMempool(const CTransactionRef& tx)`
`449`	`449`	`}`
`450`	`450`
`451`	`451`	`if (islock == nullptr) {`
`452`		`- if (m_signer) {`
`453`		`- m_signer->ProcessTx(*tx, false, Params().GetConsensus());`
	`452`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`453`	`+ signer->ProcessTx(*tx, false, Params().GetConsensus());`
`454`	`454`	`}`
`455`	`455`	`// TX is not locked, so make sure it is tracked`
`456`	`456`	`AddNonLockedTx(tx, nullptr);`
`@@ -491,8 +491,8 @@ void CInstantSendManager::BlockConnected(const std::shared_ptr<const CBlock>& pb`
`491`	`491`	`}`
`492`	`492`
`493`	`493`	`if (!IsLocked(tx->GetHash()) && !has_chainlock) {`
`494`		`- if (m_signer) {`
`495`		`- m_signer->ProcessTx(*tx, true, Params().GetConsensus());`
	`494`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`495`	`+ signer->ProcessTx(*tx, true, Params().GetConsensus());`
`496`	`496`	`}`
`497`	`497`	`// TX is not locked, so make sure it is tracked`
`498`	`498`	`AddNonLockedTx(tx, pindex);`
`@@ -597,16 +597,16 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChild`
`597`	`597`	`void CInstantSendManager::RemoveConflictedTx(const CTransaction& tx)`
`598`	`598`	`{`
`599`	`599`	`RemoveNonLockedTx(tx.GetHash(), false);`
`600`		`- if (m_signer) {`
`601`		`- m_signer->ClearInputsFromQueue(GetIdsFromLockable(tx.vin));`
	`600`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`601`	`+ signer->ClearInputsFromQueue(GetIdsFromLockable(tx.vin));`
`602`	`602`	`}`
`603`	`603`	`}`
`604`	`604`
`605`	`605`	`void CInstantSendManager::TruncateRecoveredSigsForInputs(const instantsend::InstantSendLock& islock)`
`606`	`606`	`{`
`607`	`607`	`auto ids = GetIdsFromLockable(islock.inputs);`
`608`		`- if (m_signer) {`
`609`		`- m_signer->ClearInputsFromQueue(ids);`
	`608`	`+ if (auto signer = m_signer.load(std::memory_order_acquire); signer) {`
	`609`	`+ signer->ClearInputsFromQueue(ids);`
`610`	`610`	`}`
`611`	`611`	`for (const auto& id : ids) {`
`612`	`612`	`sigman.TruncateRecoveredSig(Params().GetConsensus().llmqTypeDIP0024InstantSend, id);`
`@@ -925,7 +925,8 @@ void CInstantSendManager::WorkThreadMain(PeerManager& peerman)`
`925`	`925`	`for (auto& [node_id, mpr] : peer_activity) {`
`926`	`926`	`peerman.PostProcessMessage(std::move(mpr), node_id);`
`927`	`927`	`}`
`928`		`- if (!m_signer) return more_work;`
	`928`	`+ auto signer = m_signer.load(std::memory_order_acquire);`
	`929`	`+ if (!signer) return more_work;`
`929`	`930`	`// Construct set of non-locked transactions that are pending to retry`
`930`	`931`	`std::vector<CTransactionRef> txns{};`
`931`	`932`	`{`
`@@ -942,7 +943,7 @@ void CInstantSendManager::WorkThreadMain(PeerManager& peerman)`
`942`	`943`	`}`
`943`	`944`	`}`
`944`	`945`	`// Retry processing them`
`945`		`- m_signer->ProcessPendingRetryLockTxs(txns);`
	`946`	`+ signer->ProcessPendingRetryLockTxs(txns);`
`946`	`947`	`return more_work;`
`947`	`948`	`}();`
`948`	`949`