Skip to content

Commit 0444e59

Browse files
kwvgkwvg
committed
trivial: use std::atomic to protect connected manager pointers
1 parent b7700b3 commit 0444e59

File tree

4 files changed

+60
-41
lines changed

4 files changed

+60
-41
lines changed

‎src/evo/mnhftx.cpp‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -201,11 +201,12 @@ static bool extractSignals(const ChainstateManager& chainman, const llmq::CQuoru
201201

202202
std::optional<CMNHFManager::Signals> CMNHFManager::ProcessBlock(const CBlock& block, const CBlockIndex* const pindex, bool fJustCheck, BlockValidationState& state)
203203
{
204-
assert(m_chainman && m_qman);
204+
auto chainman = Assert(m_chainman.load(std::memory_order_acquire));
205+
auto qman = Assert(m_qman.load(std::memory_order_acquire));
205206

206207
try {
207208
std::vector<uint8_t> new_signals;
208-
if (!extractSignals(*m_chainman, *m_qman, block, pindex, new_signals, state)) {
209+
if (!extractSignals(*chainman, *qman, block, pindex, new_signals, state)) {
209210
// state is set inside extractSignals
210211
return std::nullopt;
211212
}
@@ -252,11 +253,12 @@ std::optional<CMNHFManager::Signals> CMNHFManager::ProcessBlock(const CBlock& bl
252253

253254
bool CMNHFManager::UndoBlock(const CBlock& block, const CBlockIndex* const pindex)
254255
{
255-
assert(m_chainman && m_qman);
256+
auto chainman = Assert(m_chainman.load(std::memory_order_acquire));
257+
auto qman = Assert(m_qman.load(std::memory_order_acquire));
256258

257259
std::vector<uint8_t> excluded_signals;
258260
BlockValidationState state;
259-
if (!extractSignals(*m_chainman, *m_qman, block, pindex, excluded_signals, state)) {
261+
if (!extractSignals(*chainman, *qman, block, pindex, excluded_signals, state)) {
260262
LogPrintf("CMNHFManager::%s: failed to extract signals\n", __func__);
261263
return false;
262264
}
@@ -372,19 +374,28 @@ void CMNHFManager::AddSignal(const CBlockIndex* const pindex, int bit)
372374
void CMNHFManager::ConnectManagers(gsl::not_null<ChainstateManager*> chainman, gsl::not_null<llmq::CQuorumManager*> qman)
373375
{
374376
// Do not allow double-initialization
375-
assert(m_chainman == nullptr && m_qman == nullptr);
376-
m_chainman = chainman;
377-
m_qman = qman;
377+
assert(m_chainman.load(std::memory_order_acquire) == nullptr);
378+
m_chainman.store(chainman, std::memory_order_release);
379+
assert(m_qman.load(std::memory_order_acquire) == nullptr);
380+
m_qman.store(qman, std::memory_order_release);
381+
}
382+
383+
void CMNHFManager::DisconnectManagers()
384+
{
385+
m_chainman.store(nullptr, std::memory_order_release);
386+
m_qman.store(nullptr, std::memory_order_release);
378387
}
379388

380389
bool CMNHFManager::ForceSignalDBUpdate()
381390
{
391+
auto chainman = Assert(m_chainman.load(std::memory_order_acquire));
392+
382393
// force ehf signals db update
383394
auto dbTx = m_evoDb.BeginTransaction();
384395

385396
const bool last_legacy = bls::bls_legacy_scheme.load();
386397
bls::bls_legacy_scheme.store(false);
387-
GetSignalsStage(m_chainman->ActiveChainstate().m_chain.Tip());
398+
GetSignalsStage(chainman->ActiveChainstate().m_chain.Tip());
388399
bls::bls_legacy_scheme.store(last_legacy);
389400

390401
dbTx->Commit();

‎src/evo/mnhftx.h‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,19 @@
55
#ifndef BITCOIN_EVO_MNHFTX_H
66
#define BITCOIN_EVO_MNHFTX_H
77

8-
#include <bls/bls.h>
9-
#include <gsl/pointers.h>
8+
#include <saltedhasher.h>
109
#include <sync.h>
1110
#include <threadsafety.h>
11+
#include <versionbits.h>
12+
13+
#include <bls/bls.h>
14+
#include <unordered_lru_cache.h>
15+
16+
#include <gsl/pointers.h>
1217
#include <univalue.h>
1318

19+
#include <atomic>
1420
#include <optional>
15-
#include <saltedhasher.h>
16-
#include <unordered_lru_cache.h>
17-
#include <versionbits.h>
1821

1922
class BlockValidationState;
2023
class CBlock;
@@ -91,8 +94,8 @@ class CMNHFManager : public AbstractEHFManager
9194
{
9295
private:
9396
CEvoDB& m_evoDb;
94-
ChainstateManager* m_chainman{nullptr};
95-
llmq::CQuorumManager* m_qman{nullptr};
97+
std::atomic<ChainstateManager*> m_chainman{nullptr};
98+
std::atomic<llmq::CQuorumManager*> m_qman{nullptr};
9699

97100
static constexpr size_t MNHFCacheSize = 1000;
98101
Mutex cs_cache;
@@ -144,7 +147,7 @@ class CMNHFManager : public AbstractEHFManager
144147
*
145148
* @pre Must be called before LLMQContext (containing llmq::CQuorumManager) is destroyed.
146149
*/
147-
void DisconnectManagers() { m_chainman = nullptr; m_qman = nullptr; };
150+
void DisconnectManagers();
148151

149152
bool ForceSignalDBUpdate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
150153

‎src/txmempool.cpp‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -440,10 +440,16 @@ CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator, int check_ratio)
440440
void CTxMemPool::ConnectManagers(gsl::not_null<CDeterministicMNManager*> dmnman, gsl::not_null<llmq::CInstantSendManager*> isman)
441441
{
442442
// Do not allow double-initialization
443-
assert(m_dmnman == nullptr);
444-
m_dmnman = dmnman;
445-
assert(m_isman == nullptr);
446-
m_isman = isman;
443+
assert(m_dmnman.load(std::memory_order_acquire) == nullptr);
444+
m_dmnman.store(dmnman, std::memory_order_release);
445+
assert(m_isman.load(std::memory_order_acquire) == nullptr);
446+
m_isman.store(isman, std::memory_order_release);
447+
}
448+
449+
void CTxMemPool::DisconnectManagers()
450+
{
451+
m_dmnman.store(nullptr, std::memory_order_release);
452+
m_isman.store(nullptr, std::memory_order_release);
447453
}
448454

449455
bool CTxMemPool::isSpent(const COutPoint& outpoint) const
@@ -516,8 +522,8 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
516522
// Invalid ProTxes should never get this far because transactions should be
517523
// fully checked by AcceptToMemoryPool() at this point, so we just assume that
518524
// everything is fine here.
519-
if (m_dmnman) {
520-
addUncheckedProTx(newit, tx);
525+
if (auto dmnman = m_dmnman.load(std::memory_order_acquire); dmnman) {
526+
addUncheckedProTx(*dmnman, newit, tx);
521527
}
522528
}
523529

@@ -641,10 +647,9 @@ void CTxMemPool::removeSpentIndex(const uint256 txhash)
641647
}
642648
}
643649

644-
void CTxMemPool::addUncheckedProTx(indexed_transaction_set::iterator& newit, const CTransaction& tx)
650+
void CTxMemPool::addUncheckedProTx(CDeterministicMNManager& dmnman, indexed_transaction_set::iterator& newit,
651+
const CTransaction& tx)
645652
{
646-
assert(m_dmnman);
647-
648653
const uint256 tx_hash{tx.GetHash()};
649654
if (tx.nType == TRANSACTION_PROVIDER_REGISTER) {
650655
auto proTx = *Assert(GetTxPayload<CProRegTx>(tx));
@@ -671,15 +676,15 @@ void CTxMemPool::addUncheckedProTx(indexed_transaction_set::iterator& newit, con
671676
auto proTx = *Assert(GetTxPayload<CProUpRegTx>(tx));
672677
mapProTxRefs.emplace(proTx.proTxHash, tx_hash);
673678
mapProTxBlsPubKeyHashes.emplace(proTx.pubKeyOperator.GetHash(), tx_hash);
674-
auto dmn = Assert(m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash));
679+
auto dmn = Assert(dmnman.GetListAtChainTip().GetMN(proTx.proTxHash));
675680
newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator);
676681
if (dmn->pdmnState->pubKeyOperator != proTx.pubKeyOperator) {
677682
newit->isKeyChangeProTx = true;
678683
}
679684
} else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) {
680685
auto proTx = *Assert(GetTxPayload<CProUpRevTx>(tx));
681686
mapProTxRefs.emplace(proTx.proTxHash, tx_hash);
682-
auto dmn = Assert(m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash));
687+
auto dmn = Assert(dmnman.GetListAtChainTip().GetMN(proTx.proTxHash));
683688
newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator);
684689
if (dmn->pdmnState->pubKeyOperator.Get() != CBLSPublicKey()) {
685690
newit->isKeyChangeProTx = true;
@@ -721,7 +726,7 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
721726
} else
722727
vTxHashes.clear();
723728

724-
if (m_dmnman) {
729+
if (m_dmnman.load(std::memory_order_acquire)) {
725730
removeUncheckedProTx(it->GetTx());
726731
}
727732

@@ -926,7 +931,7 @@ void CTxMemPool::removeProTxCollateralConflicts(const CTransaction &tx, const CO
926931

927932
void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx)
928933
{
929-
assert(m_dmnman);
934+
auto dmnman = Assert(m_dmnman.load(std::memory_order_acquire));
930935

931936
// Remove TXs that refer to a MN for which the collateral was spent
932937
auto removeSpentCollateralConflict = [&](const uint256& proTxHash) EXCLUSIVE_LOCKS_REQUIRED(cs) {
@@ -948,7 +953,7 @@ void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx)
948953
}
949954
}
950955
};
951-
auto mnList = m_dmnman->GetListAtChainTip();
956+
auto mnList = dmnman->GetListAtChainTip();
952957
for (const auto& in : tx.vin) {
953958
auto collateralIt = mapProTxCollaterals.find(in.prevout);
954959
if (collateralIt != mapProTxCollaterals.end()) {
@@ -1070,7 +1075,7 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
10701075
RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK);
10711076
}
10721077
removeConflicts(*tx);
1073-
if (m_dmnman) {
1078+
if (m_dmnman.load(std::memory_order_acquire)) {
10741079
removeProTxConflicts(*tx);
10751080
}
10761081
ClearPrioritisation(tx->GetHash());
@@ -1328,7 +1333,7 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const
13281333
}
13291334

13301335
bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const {
1331-
assert(m_dmnman);
1336+
auto dmnman = Assert(m_dmnman.load(std::memory_order_acquire));
13321337

13331338
LOCK(cs);
13341339

@@ -1394,7 +1399,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const {
13941399
auto& proTx = *opt_proTx;
13951400

13961401
// this method should only be called with validated ProTxs
1397-
auto dmn = m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash);
1402+
auto dmn = dmnman->GetListAtChainTip().GetMN(proTx.proTxHash);
13981403
if (!dmn) {
13991404
LogPrint(BCLog::MEMPOOL, "%s: ERROR: Masternode is not in the list, proTxHash: %s\n", __func__, proTx.proTxHash.ToString());
14001405
return true; // i.e. failed to find validated ProTx == conflict
@@ -1416,7 +1421,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const {
14161421
}
14171422
auto& proTx = *opt_proTx;
14181423
// this method should only be called with validated ProTxs
1419-
auto dmn = m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash);
1424+
auto dmn = dmnman->GetListAtChainTip().GetMN(proTx.proTxHash);
14201425
if (!dmn) {
14211426
LogPrint(BCLog::MEMPOOL, "%s: ERROR: Masternode is not in the list, proTxHash: %s\n", __func__, proTx.proTxHash.ToString());
14221427
return true; // i.e. failed to find validated ProTx == conflict
@@ -1566,12 +1571,12 @@ void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPool
15661571
int CTxMemPool::Expire(std::chrono::seconds time)
15671572
{
15681573
AssertLockHeld(cs);
1569-
assert(m_isman);
1574+
auto isman = Assert(m_isman.load(std::memory_order_acquire));
15701575
indexed_transaction_set::index<entry_time>::type::iterator it = mapTx.get<entry_time>().begin();
15711576
setEntries toremove;
15721577
while (it != mapTx.get<entry_time>().end() && it->GetTime() < time) {
15731578
// locked txes do not expire until mined and have sufficient confirmations
1574-
if (m_isman->IsLocked(it->GetTx().GetHash())) {
1579+
if (isman->IsLocked(it->GetTx().GetHash())) {
15751580
it++;
15761581
continue;
15771582
}

‎src/txmempool.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -435,8 +435,8 @@ class CTxMemPool
435435
const int m_check_ratio; //!< Value n means that 1 times in n we check.
436436
std::atomic<unsigned int> nTransactionsUpdated{0}; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation
437437
CBlockPolicyEstimator* const minerPolicyEstimator;
438-
CDeterministicMNManager* m_dmnman{nullptr};
439-
llmq::CInstantSendManager* m_isman{nullptr};
438+
std::atomic<CDeterministicMNManager*> m_dmnman{nullptr};
439+
std::atomic<llmq::CInstantSendManager*> m_isman{nullptr};
440440

441441
uint64_t totalTxSize GUARDED_BY(cs); //!< sum of all mempool tx' byte sizes
442442
CAmount m_total_fee GUARDED_BY(cs); //!< sum of all mempool tx's fees (NOT modified fee)
@@ -590,7 +590,7 @@ class CTxMemPool
590590
*
591591
* @pre Must be called before CDeterministicMNManager and CInstantSendManager are destroyed.
592592
*/
593-
void DisconnectManagers() { m_dmnman = nullptr; m_isman = nullptr; }
593+
void DisconnectManagers();
594594

595595
/**
596596
* If sanity-checking is turned on, check makes sure the pool is
@@ -886,9 +886,9 @@ class CTxMemPool
886886

887887
/**
888888
* addUnchecked extension for Dash-specific transactions (ProTx).
889-
* Depends on CDeterministicMNManager.
890889
*/
891-
void addUncheckedProTx(indexed_transaction_set::iterator& newit, const CTransaction& tx);
890+
void addUncheckedProTx(CDeterministicMNManager& dmnman, indexed_transaction_set::iterator& newit,
891+
const CTransaction& tx);
892892

893893
/** Before calling removeUnchecked for a given transaction,
894894
* UpdateForRemoveFromMempool must be called on the entire (dependent) set

0 commit comments

Comments
 (0)