diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 67f377b200e1..ea7d9c1fd526 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -947,10 +947,20 @@ Threads and synchronization - Prefer `Mutex` type to `RecursiveMutex` one. - Consistently use [Clang Thread Safety Analysis](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html) annotations to - get compile-time warnings about potential race conditions in code. Combine annotations in function declarations with - run-time asserts in function definitions (`AssertLockNotHeld()` can be omitted if `LOCK()` is - called unconditionally after it because `LOCK()` does the same check as - `AssertLockNotHeld()` internally, for non-recursive mutexes): + get compile-time warnings about potential race conditions or deadlocks in code. + + - In functions that are declared separately from where they are defined, the + thread safety annotations should be added exclusively to the function + declaration. Annotations on the definition could lead to false positives + (lack of compile failure) at call sites between the two. + + - Prefer locks that are in a class rather than global, and that are + internal to a class (private or protected) rather than public. + + - Combine annotations in function declarations with run-time asserts in + function definitions (`AssertLockNotHeld()` can be omitted if `LOCK()` is + called unconditionally after it because `LOCK()` does the same check as + `AssertLockNotHeld()` internally, for non-recursive mutexes): ```C++ // txmempool.h @@ -975,21 +985,37 @@ void CTxMemPool::UpdateTransactionsFromBlock(...) ```C++ // validation.h -class ChainstateManager +class CChainState { +protected: + ... + Mutex m_chainstate_mutex; + ... public: ... - bool ProcessNewBlock(...) LOCKS_EXCLUDED(::cs_main); + bool ActivateBestChain( + BlockValidationState& state, + std::shared_ptr pblock = nullptr) + EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex) + LOCKS_EXCLUDED(::cs_main); + ... + bool PreciousBlock(BlockValidationState& state, CBlockIndex* pindex) + EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex) + LOCKS_EXCLUDED(::cs_main); ... } // validation.cpp -bool ChainstateManager::ProcessNewBlock(...) +bool CChainState::PreciousBlock(BlockValidationState& state, CBlockIndex* pindex) { + AssertLockNotHeld(m_chainstate_mutex); AssertLockNotHeld(::cs_main); - ... - LOCK(::cs_main); - ... + { + LOCK(cs_main); + ... + } + + return ActivateBestChain(state, std::shared_ptr()); } ``` diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index 79dccfc938dd..cba6903030b3 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -95,16 +95,16 @@ class CoinJoinWalletManager { } } - void Add(const std::shared_ptr& wallet); - void DoMaintenance(CConnman& connman); + void Add(const std::shared_ptr& wallet) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); + void DoMaintenance(CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); - void Remove(const std::string& name); - void Flush(const std::string& name); + void Remove(const std::string& name) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); + void Flush(const std::string& name) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); - CCoinJoinClientManager* Get(const std::string& name) const; + CCoinJoinClientManager* Get(const std::string& name) const EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); template - void ForEachCJClientMan(Callable&& func) + void ForEachCJClientMan(Callable&& func) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map) { LOCK(cs_wallet_manager_map); for (auto&& [_, clientman] : m_wallet_manager_map) { @@ -113,7 +113,7 @@ class CoinJoinWalletManager { }; template - bool ForAnyCJClientMan(Callable&& func) + bool ForAnyCJClientMan(Callable&& func) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map) { LOCK(cs_wallet_manager_map); return ranges::any_of(m_wallet_manager_map, [&](auto& pair) { return func(pair.second); }); @@ -251,9 +251,9 @@ class CCoinJoinClientQueueManager : public CCoinJoinBaseManager m_mn_sync(mn_sync), m_is_masternode{is_masternode} {}; - [[nodiscard]] MessageProcessingResult ProcessMessage(NodeId from, CConnman& connman, PeerManager& peerman, std::string_view msg_type, - CDataStream& vRecv) - EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue); + [[nodiscard]] MessageProcessingResult ProcessMessage(NodeId from, CConnman& connman, PeerManager& peerman, + std::string_view msg_type, CDataStream& vRecv) + EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue, !cs_ProcessDSQueue); void DoMaintenance(); }; diff --git a/src/coinjoin/util.h b/src/coinjoin/util.h index ab0122d1bb64..8f1a26b4fc7e 100644 --- a/src/coinjoin/util.h +++ b/src/coinjoin/util.h @@ -117,7 +117,11 @@ class CTransactionBuilder /// Check if an amounts should be considered as dust bool IsDust(CAmount nAmount) const; /// Get the total number of added outputs - int CountOutputs() const { LOCK(cs_outputs); return vecOutputs.size(); } + int CountOutputs() const EXCLUSIVE_LOCKS_REQUIRED(!cs_outputs) + { + LOCK(cs_outputs); + return vecOutputs.size(); + } /// Create and Commit the transaction to the wallet bool Commit(bilingual_str& strResult) EXCLUSIVE_LOCKS_REQUIRED(!cs_outputs); /// Convert to a string diff --git a/src/evo/creditpool.h b/src/evo/creditpool.h index e337c00baea9..5b2bb8cf43d7 100644 --- a/src/evo/creditpool.h +++ b/src/evo/creditpool.h @@ -132,14 +132,15 @@ class CCreditPoolManager * In case if block is invalid the function GetCreditPool throws an exception * it can happen if there limits of withdrawal (unlock) exceed */ - CCreditPool GetCreditPool(const CBlockIndex* block, const Consensus::Params& consensusParams); + CCreditPool GetCreditPool(const CBlockIndex* block, const Consensus::Params& consensusParams) + EXCLUSIVE_LOCKS_REQUIRED(!cache_mutex); private: - std::optional GetFromCache(const CBlockIndex& block_index); - void AddToCache(const uint256& block_hash, int height, const CCreditPool& pool); + std::optional GetFromCache(const CBlockIndex& block_index) EXCLUSIVE_LOCKS_REQUIRED(!cache_mutex); + void AddToCache(const uint256& block_hash, int height, const CCreditPool& pool) EXCLUSIVE_LOCKS_REQUIRED(!cache_mutex); CCreditPool ConstructCreditPool(const gsl::not_null block_index, CCreditPool prev, - const Consensus::Params& consensusParams); + const Consensus::Params& consensusParams) EXCLUSIVE_LOCKS_REQUIRED(!cache_mutex); }; std::optional GetCreditPoolDiffForBlock(CCreditPoolManager& cpoolman, const node::BlockManager& blockman, const llmq::CQuorumManager& qman, diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index 1c5f53600a13..e195577a5e3e 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -162,7 +162,7 @@ class CDeterministicMNList mutable std::shared_ptr m_cached_sml GUARDED_BY(m_cached_sml_mutex); // Private helper method to invalidate SML cache - void InvalidateSMLCache() + void InvalidateSMLCache() EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex) { LOCK(m_cached_sml_mutex); m_cached_sml = nullptr; @@ -193,6 +193,7 @@ class CDeterministicMNList // Assignment operator CDeterministicMNList& operator=(const CDeterministicMNList& other) + EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex, !other.m_cached_sml_mutex) { if (this != &other) { blockHash = other.blockHash; @@ -229,7 +230,7 @@ class CDeterministicMNList } template - void Unserialize(Stream& s) + void Unserialize(Stream& s) EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex) { Clear(); @@ -240,7 +241,7 @@ class CDeterministicMNList } } - void Clear() + void Clear() EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex) { blockHash = uint256{}; nHeight = -1; @@ -372,7 +373,7 @@ class CDeterministicMNList * Calculates CSimplifiedMNList for current list and cache it * Thread safety: Uses internal mutex for thread-safe cache access */ - gsl::not_null> to_sml() const; + gsl::not_null> to_sml() const EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex); /** * Calculates the maximum penalty which is allowed at the height of this MN list. It is dynamic and might change @@ -393,14 +394,14 @@ class CDeterministicMNList * Penalty scores are only increased when the MN is not already banned, which means that after banning the penalty * might appear lower then the current max penalty, while the MN is still banned. */ - void PoSePunish(const uint256& proTxHash, int penalty, bool debugLogs); + void PoSePunish(const uint256& proTxHash, int penalty, bool debugLogs) EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex); - void DecreaseScores(); + void DecreaseScores() EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex); /** * Decrease penalty score of MN by 1. * Only allowed on non-banned MNs. */ - void PoSeDecrease(const CDeterministicMN& dmn); + void PoSeDecrease(const CDeterministicMN& dmn) EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex); [[nodiscard]] CDeterministicMNListDiff BuildDiff(const CDeterministicMNList& to) const; /** @@ -408,13 +409,17 @@ class CDeterministicMNList * It is more efficient than creating a copy due to heavy copy constructor. * Calculating for old block may require up to {DISK_SNAPSHOT_PERIOD} object copy & destroy. */ - void ApplyDiff(gsl::not_null pindex, const CDeterministicMNListDiff& diff); - - void AddMN(const CDeterministicMNCPtr& dmn, bool fBumpTotalCount = true); - void UpdateMN(const CDeterministicMN& oldDmn, const std::shared_ptr& pdmnState); - void UpdateMN(const uint256& proTxHash, const std::shared_ptr& pdmnState); - void UpdateMN(const CDeterministicMN& oldDmn, const CDeterministicMNStateDiff& stateDiff); - void RemoveMN(const uint256& proTxHash); + void ApplyDiff(gsl::not_null pindex, const CDeterministicMNListDiff& diff) + EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex); + + void AddMN(const CDeterministicMNCPtr& dmn, bool fBumpTotalCount = true) EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex); + void UpdateMN(const CDeterministicMN& oldDmn, const std::shared_ptr& pdmnState) + EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex); + void UpdateMN(const uint256& proTxHash, const std::shared_ptr& pdmnState) + EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex); + void UpdateMN(const CDeterministicMN& oldDmn, const CDeterministicMNStateDiff& stateDiff) + EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex); + void RemoveMN(const uint256& proTxHash) EXCLUSIVE_LOCKS_REQUIRED(!m_cached_sml_mutex); template [[nodiscard]] bool HasUniqueProperty(const T& v) const @@ -672,7 +677,7 @@ class CDeterministicMNManager // Test if given TX is a ProRegTx which also contains the collateral at index n static bool IsProTxWithCollateral(const CTransactionRef& tx, uint32_t n); - void DoMaintenance() EXCLUSIVE_LOCKS_REQUIRED(!cs); + void DoMaintenance() EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_cleanup); // Migration support for nVersion-first CDeterministicMNStateDiff format [[nodiscard]] bool IsMigrationRequired() const EXCLUSIVE_LOCKS_REQUIRED(!cs, ::cs_main); diff --git a/src/evo/mnhftx.h b/src/evo/mnhftx.h index 855607b21698..210598086f7a 100644 --- a/src/evo/mnhftx.h +++ b/src/evo/mnhftx.h @@ -110,7 +110,8 @@ class CMNHFManager : public AbstractEHFManager * @pre Caller must ensure that LLMQContext has been initialized and the llmq::CQuorumManager pointer has been * set by calling ConnectManagers() for this CMNHFManager instance */ - std::optional ProcessBlock(const CBlock& block, const CBlockIndex* const pindex, bool fJustCheck, BlockValidationState& state); + std::optional ProcessBlock(const CBlock& block, const CBlockIndex* const pindex, bool fJustCheck, + BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); /** * Every undo block should be processed when Tip() is updated by calling of CMNHFManager::UndoBlock @@ -120,10 +121,10 @@ class CMNHFManager : public AbstractEHFManager * @pre Caller must ensure that LLMQContext has been initialized and the llmq::CQuorumManager pointer has been * set by calling ConnectManagers() for this CMNHFManager instance */ - bool UndoBlock(const CBlock& block, const CBlockIndex* const pindex); + bool UndoBlock(const CBlock& block, const CBlockIndex* const pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); // Implements interface - Signals GetSignalsStage(const CBlockIndex* const pindexPrev) override; + Signals GetSignalsStage(const CBlockIndex* const pindexPrev) override EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); /** * Helper that used in Unit Test to forcely setup EHF signal for specific block @@ -145,10 +146,10 @@ class CMNHFManager : public AbstractEHFManager */ void DisconnectManagers(); - bool ForceSignalDBUpdate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool ForceSignalDBUpdate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !cs_cache); private: - void AddToCache(const Signals& signals, const CBlockIndex* const pindex); + void AddToCache(const Signals& signals, const CBlockIndex* const pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); /** * This function returns list of signals available on previous block. @@ -156,13 +157,13 @@ class CMNHFManager : public AbstractEHFManager * until state won't be recovered. * NOTE: that some signals could expired between blocks. */ - Signals GetForBlock(const CBlockIndex* const pindex); + Signals GetForBlock(const CBlockIndex* const pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); /** * This function access to in-memory cache or to evo db but does not calculate anything * NOTE: that some signals could expired between blocks. */ - std::optional GetFromCache(const CBlockIndex* const pindex); + std::optional GetFromCache(const CBlockIndex* const pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); }; std::optional extractEHFSignal(const CTransaction& tx); diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 1ff5ddda0e01..dc09f514e50d 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -718,7 +718,7 @@ std::optional CGovernanceManager::CreateGovernanceTrigg } // Nobody submitted a trigger we'd like to see, so let's do it but only if we are the payee - const CBlockIndex *tip = WITH_LOCK(::cs_main, return m_chainman.ActiveChain().Tip()); + const CBlockIndex* tip = m_chainman.ActiveChain().Tip(); const auto mnList = Assert(m_dmnman)->GetListForBlock(tip); const auto mn_payees = mnList.GetProjectedMNPayees(tip); diff --git a/src/httpserver.cpp b/src/httpserver.cpp index fe8c8879ea43..7cdf8cd072cb 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -159,7 +159,7 @@ static std::unique_ptr> g_work_queue{nullptr}; //! List of 'external' RPC users (global variable, used by httprpc) std::vector g_external_usernames; //! Handlers for (sub)paths -static Mutex g_httppathhandlers_mutex; +static GlobalMutex g_httppathhandlers_mutex; static std::vector pathHandlers GUARDED_BY(g_httppathhandlers_mutex); //! Bound listening sockets static std::vector boundSockets; diff --git a/src/init.cpp b/src/init.cpp index 25b2ca72090c..bfa35a053cc1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -800,7 +800,7 @@ void SetupServerArgs(ArgsManager& argsman) } static bool fHaveGenesis = false; -static Mutex g_genesis_wait_mutex; +static GlobalMutex g_genesis_wait_mutex; static std::condition_variable g_genesis_wait_cv; static void BlockNotifyGenesisWait(const CBlockIndex* pBlockIndex) diff --git a/src/instantsend/instantsend.h b/src/instantsend/instantsend.h index ddeb91c9d144..e9b5c809c197 100644 --- a/src/instantsend/instantsend.h +++ b/src/instantsend/instantsend.h @@ -119,7 +119,7 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry); void AddNonLockedTx(const CTransactionRef& tx, const CBlockIndex* pindexMined) - EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks); + EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_timingsTxSeen); void RemoveNonLockedTx(const uint256& txid, bool retryChildren) EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry); void RemoveConflictedTx(const CTransaction& tx) @@ -142,13 +142,14 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent bool IsWaitingForTx(const uint256& txHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks); instantsend::InstantSendLockPtr GetConflictingLock(const CTransaction& tx) const override; - [[nodiscard]] MessageProcessingResult ProcessMessage(NodeId from, std::string_view msg_type, CDataStream& vRecv); + [[nodiscard]] MessageProcessingResult ProcessMessage(NodeId from, std::string_view msg_type, CDataStream& vRecv) + EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks); void TransactionAddedToMempool(const CTransactionRef& tx) - EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry); + EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry, !cs_timingsTxSeen); void TransactionRemovedFromMempool(const CTransactionRef& tx); void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex) - EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry); + EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry, !cs_timingsTxSeen); void BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected); bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks); diff --git a/src/llmq/blockprocessor.h b/src/llmq/blockprocessor.h index f4f19ae067fe..de026946d7ab 100644 --- a/src/llmq/blockprocessor.h +++ b/src/llmq/blockprocessor.h @@ -58,18 +58,26 @@ class CQuorumBlockProcessor CQuorumSnapshotManager& qsnapman); ~CQuorumBlockProcessor(); - [[nodiscard]] MessageProcessingResult ProcessMessage(const CNode& peer, std::string_view msg_type, CDataStream& vRecv); + [[nodiscard]] MessageProcessingResult ProcessMessage(const CNode& peer, std::string_view msg_type, CDataStream& vRecv) + EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs); - bool ProcessBlock(const CBlock& block, gsl::not_null pindex, BlockValidationState& state, bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - bool UndoBlock(const CBlock& block, gsl::not_null pindex) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool ProcessBlock(const CBlock& block, gsl::not_null pindex, BlockValidationState& state, + bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs); + bool UndoBlock(const CBlock& block, gsl::not_null pindex) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs); //! it returns hash of commitment if it should be relay, otherwise nullopt - std::optional AddMineableCommitment(const CFinalCommitment& fqc); - bool HasMineableCommitment(const uint256& hash) const; - bool GetMineableCommitmentByHash(const uint256& commitmentHash, CFinalCommitment& ret) const; - std::optional> GetMineableCommitments(const Consensus::LLMQParams& llmqParams, int nHeight) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - bool GetMineableCommitmentsTx(const Consensus::LLMQParams& llmqParams, int nHeight, std::vector& ret) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - bool HasMinedCommitment(Consensus::LLMQType llmqType, const uint256& quorumHash) const; + std::optional AddMineableCommitment(const CFinalCommitment& fqc) EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs); + bool HasMineableCommitment(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs); + bool GetMineableCommitmentByHash(const uint256& commitmentHash, CFinalCommitment& ret) const + EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs); + std::optional> GetMineableCommitments(const Consensus::LLMQParams& llmqParams, + int nHeight) const + EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs); + bool GetMineableCommitmentsTx(const Consensus::LLMQParams& llmqParams, int nHeight, std::vector& ret) const + EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs); + bool HasMinedCommitment(Consensus::LLMQType llmqType, const uint256& quorumHash) const + EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs); std::pair GetMinedCommitment(Consensus::LLMQType llmqType, const uint256& quorumHash) const; std::vector GetMinedCommitmentsUntilBlock(Consensus::LLMQType llmqType, gsl::not_null pindex, size_t maxCount) const; @@ -82,9 +90,10 @@ class CQuorumBlockProcessor std::optional GetLastMinedCommitmentsByQuorumIndexUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, int quorumIndex, size_t cycle) const; private: static bool GetCommitmentsFromBlock(const CBlock& block, gsl::not_null pindex, std::multimap& ret, BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - bool ProcessCommitment(int nHeight, const uint256& blockHash, const CFinalCommitment& qc, - BlockValidationState& state, bool fJustCheck) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - size_t GetNumCommitmentsRequired(const Consensus::LLMQParams& llmqParams, int nHeight) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool ProcessCommitment(int nHeight, const uint256& blockHash, const CFinalCommitment& qc, BlockValidationState& state, + bool fJustCheck) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs); + size_t GetNumCommitmentsRequired(const Consensus::LLMQParams& llmqParams, int nHeight) const + EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs); static uint256 GetQuorumBlockHash(const Consensus::LLMQParams& llmqParams, const CChain& active_chain, int nHeight, int quorumIndex) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); }; } // namespace llmq diff --git a/src/llmq/debug.h b/src/llmq/debug.h index b97d6bc25cec..61ba107adfd7 100644 --- a/src/llmq/debug.h +++ b/src/llmq/debug.h @@ -103,13 +103,18 @@ class CDKGDebugManager public: CDKGDebugManager(); - void GetLocalDebugStatus(CDKGDebugStatus& ret) const; - - void ResetLocalSessionStatus(Consensus::LLMQType llmqType, int quorumIndex); - void InitLocalSessionStatus(const Consensus::LLMQParams& llmqParams, int quorumIndex, const uint256& quorumHash, int quorumHeight); - - void UpdateLocalSessionStatus(Consensus::LLMQType llmqType, int quorumIndex, std::function&& func); - void UpdateLocalMemberStatus(Consensus::LLMQType llmqType, int quorumIndex, size_t memberIdx, std::function&& func); + void GetLocalDebugStatus(CDKGDebugStatus& ret) const EXCLUSIVE_LOCKS_REQUIRED(!cs_lockStatus); + + void ResetLocalSessionStatus(Consensus::LLMQType llmqType, int quorumIndex) EXCLUSIVE_LOCKS_REQUIRED(!cs_lockStatus); + void InitLocalSessionStatus(const Consensus::LLMQParams& llmqParams, int quorumIndex, const uint256& quorumHash, + int quorumHeight) EXCLUSIVE_LOCKS_REQUIRED(!cs_lockStatus); + + void UpdateLocalSessionStatus(Consensus::LLMQType llmqType, int quorumIndex, + std::function&& func) + EXCLUSIVE_LOCKS_REQUIRED(!cs_lockStatus); + void UpdateLocalMemberStatus(Consensus::LLMQType llmqType, int quorumIndex, size_t memberIdx, + std::function&& func) + EXCLUSIVE_LOCKS_REQUIRED(!cs_lockStatus); }; } // namespace llmq diff --git a/src/llmq/dkgsession.h b/src/llmq/dkgsession.h index 33b11d811a14..2b1b095b93fe 100644 --- a/src/llmq/dkgsession.h +++ b/src/llmq/dkgsession.h @@ -357,30 +357,31 @@ class CDKGSession void Contribute(CDKGPendingMessages& pendingMessages, PeerManager& peerman); void SendContributions(CDKGPendingMessages& pendingMessages, PeerManager& peerman); bool PreVerifyMessage(const CDKGContribution& qc, bool& retBan) const; - std::optional ReceiveMessage(const CDKGContribution& qc); + std::optional ReceiveMessage(const CDKGContribution& qc) EXCLUSIVE_LOCKS_REQUIRED(!invCs, !cs_pending); void VerifyPendingContributions() EXCLUSIVE_LOCKS_REQUIRED(cs_pending); // Phase 2: complaint - void VerifyAndComplain(CConnman& connman, CDKGPendingMessages& pendingMessages, PeerManager& peerman); + void VerifyAndComplain(CConnman& connman, CDKGPendingMessages& pendingMessages, PeerManager& peerman) + EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); void VerifyConnectionAndMinProtoVersions(CConnman& connman) const; void SendComplaint(CDKGPendingMessages& pendingMessages, PeerManager& peerman); bool PreVerifyMessage(const CDKGComplaint& qc, bool& retBan) const; - std::optional ReceiveMessage(const CDKGComplaint& qc); + std::optional ReceiveMessage(const CDKGComplaint& qc) EXCLUSIVE_LOCKS_REQUIRED(!invCs); // Phase 3: justification - void VerifyAndJustify(CDKGPendingMessages& pendingMessages, PeerManager& peerman); + void VerifyAndJustify(CDKGPendingMessages& pendingMessages, PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!invCs); void SendJustification(CDKGPendingMessages& pendingMessages, PeerManager& peerman, const std::set& forMembers); bool PreVerifyMessage(const CDKGJustification& qj, bool& retBan) const; - std::optional ReceiveMessage(const CDKGJustification& qj); + std::optional ReceiveMessage(const CDKGJustification& qj) EXCLUSIVE_LOCKS_REQUIRED(!invCs); // Phase 4: commit void VerifyAndCommit(CDKGPendingMessages& pendingMessages, PeerManager& peerman); void SendCommitment(CDKGPendingMessages& pendingMessages, PeerManager& peerman); bool PreVerifyMessage(const CDKGPrematureCommitment& qc, bool& retBan) const; - std::optional ReceiveMessage(const CDKGPrematureCommitment& qc); + std::optional ReceiveMessage(const CDKGPrematureCommitment& qc) EXCLUSIVE_LOCKS_REQUIRED(!invCs); // Phase 5: aggregate/finalize - std::vector FinalizeCommitments(); + std::vector FinalizeCommitments() EXCLUSIVE_LOCKS_REQUIRED(!invCs); // All Phases 5-in-1 for single-node-quorum CFinalCommitment FinalizeSingleCommitment(); diff --git a/src/llmq/dkgsessionhandler.h b/src/llmq/dkgsessionhandler.h index faab21a69b17..ad241a5efc13 100644 --- a/src/llmq/dkgsessionhandler.h +++ b/src/llmq/dkgsessionhandler.h @@ -76,14 +76,15 @@ class CDKGPendingMessages explicit CDKGPendingMessages(size_t _maxMessagesPerNode, uint32_t _invType) : invType(_invType), maxMessagesPerNode(_maxMessagesPerNode) {}; - [[nodiscard]] MessageProcessingResult PushPendingMessage(NodeId from, CDataStream& vRecv); - std::list PopPendingMessages(size_t maxCount); - bool HasSeen(const uint256& hash) const; + [[nodiscard]] MessageProcessingResult PushPendingMessage(NodeId from, CDataStream& vRecv) + EXCLUSIVE_LOCKS_REQUIRED(!cs_messages); + std::list PopPendingMessages(size_t maxCount) EXCLUSIVE_LOCKS_REQUIRED(!cs_messages); + bool HasSeen(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_messages); void Misbehaving(NodeId from, int score, PeerManager& peerman); - void Clear(); + void Clear() EXCLUSIVE_LOCKS_REQUIRED(!cs_messages); template - void PushPendingMessage(NodeId from, Message& msg, PeerManager& peerman) + void PushPendingMessage(NodeId from, Message& msg, PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!cs_messages) { CDataStream ds(SER_NETWORK, PROTOCOL_VERSION); ds << msg; @@ -91,8 +92,9 @@ class CDKGPendingMessages } // Might return nullptr messages, which indicates that deserialization failed for some reason - template + template std::vector>> PopAndDeserializeMessages(size_t maxCount) + EXCLUSIVE_LOCKS_REQUIRED(!cs_messages) { auto binaryMessages = PopPendingMessages(maxCount); if (binaryMessages.empty()) { @@ -165,7 +167,7 @@ class CDKGSessionHandler const CSporkManager& sporkman, const Consensus::LLMQParams& _params, int _quorumIndex); ~CDKGSessionHandler(); - void UpdatedBlockTip(const CBlockIndex *pindexNew); + void UpdatedBlockTip(const CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); [[nodiscard]] MessageProcessingResult ProcessMessage(NodeId from, std::string_view msg_type, CDataStream& vRecv); void StartThread(CConnman& connman, PeerManager& peerman); @@ -179,7 +181,7 @@ class CDKGSessionHandler private: bool InitNewQuorum(const CBlockIndex* pQuorumBaseBlockIndex); - std::pair GetPhaseAndQuorumHash() const; + std::pair GetPhaseAndQuorumHash() const EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); using StartPhaseFunc = std::function; using WhileWaitFunc = std::function; @@ -189,12 +191,17 @@ class CDKGSessionHandler * @param expectedQuorumHash expected QuorumHash, defaults to null * @param shouldNotWait function that returns bool, defaults to function that returns false. If the function returns false, we will wait in the loop, if true, we don't wait */ - void WaitForNextPhase(std::optional curPhase, QuorumPhase nextPhase, const uint256& expectedQuorumHash=uint256(), const WhileWaitFunc& shouldNotWait=[]{return false;}) const; - void WaitForNewQuorum(const uint256& oldQuorumHash) const; - void SleepBeforePhase(QuorumPhase curPhase, const uint256& expectedQuorumHash, double randomSleepFactor, const WhileWaitFunc& runWhileWaiting) const; - void HandlePhase(QuorumPhase curPhase, QuorumPhase nextPhase, const uint256& expectedQuorumHash, double randomSleepFactor, const StartPhaseFunc& startPhaseFunc, const WhileWaitFunc& runWhileWaiting); - void HandleDKGRound(CConnman& connman, PeerManager& peerman); - void PhaseHandlerThread(CConnman& connman, PeerManager& peerman); + void WaitForNextPhase( + std::optional curPhase, QuorumPhase nextPhase, const uint256& expectedQuorumHash = uint256(), + const WhileWaitFunc& shouldNotWait = [] { return false; }) const EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); + void WaitForNewQuorum(const uint256& oldQuorumHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); + void SleepBeforePhase(QuorumPhase curPhase, const uint256& expectedQuorumHash, double randomSleepFactor, + const WhileWaitFunc& runWhileWaiting) const EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); + void HandlePhase(QuorumPhase curPhase, QuorumPhase nextPhase, const uint256& expectedQuorumHash, + double randomSleepFactor, const StartPhaseFunc& startPhaseFunc, + const WhileWaitFunc& runWhileWaiting) EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); + void HandleDKGRound(CConnman& connman, PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); + void PhaseHandlerThread(CConnman& connman, PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); }; } // namespace llmq diff --git a/src/llmq/dkgsessionmgr.h b/src/llmq/dkgsessionmgr.h index 51fb2fdbe8e9..dba7948ec2f7 100644 --- a/src/llmq/dkgsessionmgr.h +++ b/src/llmq/dkgsessionmgr.h @@ -87,7 +87,8 @@ class CDKGSessionManager void StartThreads(CConnman& connman, PeerManager& peerman); void StopThreads(); - void UpdatedBlockTip(const CBlockIndex *pindexNew, bool fInitialDownload); + void UpdatedBlockTip(const CBlockIndex* pindexNew, bool fInitialDownload) + EXCLUSIVE_LOCKS_REQUIRED(!contributionsCacheCs); [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& pfrom, bool is_masternode, std::string_view msg_type, CDataStream& vRecv); @@ -100,7 +101,11 @@ class CDKGSessionManager // Contributions are written while in the DKG void WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const BLSVerificationVectorPtr& vvec); void WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution); - bool GetVerifiedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const std::vector& validMembers, std::vector& memberIndexesRet, std::vector& vvecsRet, std::vector& skContributionsRet) const; + bool GetVerifiedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, + const std::vector& validMembers, std::vector& memberIndexesRet, + std::vector& vvecsRet, + std::vector& skContributionsRet) const + EXCLUSIVE_LOCKS_REQUIRED(!contributionsCacheCs); /// Write encrypted (unverified) DKG contributions for the member with the given proTxHash to the llmqDb void WriteEncryptedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSIESMultiRecipientObjects& contributions); /// Read encrypted (unverified) DKG contributions for the member with the given proTxHash from the llmqDb @@ -109,7 +114,7 @@ class CDKGSessionManager void CleanupOldContributions() const; private: - void CleanupCache() const; + void CleanupCache() const EXCLUSIVE_LOCKS_REQUIRED(!contributionsCacheCs); }; } // namespace llmq diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index 33acc6f3bbeb..1ba398103739 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -197,25 +197,27 @@ class CQuorum ~CQuorum() = default; void Init(CFinalCommitmentPtr _qc, const CBlockIndex* _pQuorumBaseBlockIndex, const uint256& _minedBlockHash, Span _members); - bool SetVerificationVector(const std::vector& quorumVecIn); - void SetVerificationVector(BLSVerificationVectorPtr vvec_in) { + bool SetVerificationVector(const std::vector& quorumVecIn) EXCLUSIVE_LOCKS_REQUIRED(!cs_vvec_shShare); + void SetVerificationVector(BLSVerificationVectorPtr vvec_in) EXCLUSIVE_LOCKS_REQUIRED(!cs_vvec_shShare) + { LOCK(cs_vvec_shShare); quorumVvec = std::move(vvec_in); } - bool SetSecretKeyShare(const CBLSSecretKey& secretKeyShare, const CActiveMasternodeManager& mn_activeman); + bool SetSecretKeyShare(const CBLSSecretKey& secretKeyShare, const CActiveMasternodeManager& mn_activeman) + EXCLUSIVE_LOCKS_REQUIRED(!cs_vvec_shShare); - bool HasVerificationVector() const LOCKS_EXCLUDED(cs_vvec_shShare); + bool HasVerificationVector() const EXCLUSIVE_LOCKS_REQUIRED(!cs_vvec_shShare); bool IsMember(const uint256& proTxHash) const; bool IsValidMember(const uint256& proTxHash) const; int GetMemberIndex(const uint256& proTxHash) const; - CBLSPublicKey GetPubKeyShare(size_t memberIdx) const; - CBLSSecretKey GetSkShare() const; + CBLSPublicKey GetPubKeyShare(size_t memberIdx) const EXCLUSIVE_LOCKS_REQUIRED(!cs_vvec_shShare); + CBLSSecretKey GetSkShare() const EXCLUSIVE_LOCKS_REQUIRED(!cs_vvec_shShare); private: bool HasVerificationVectorInternal() const EXCLUSIVE_LOCKS_REQUIRED(cs_vvec_shShare); - void WriteContributions(CDBWrapper& db) const; - bool ReadContributions(const CDBWrapper& db); + void WriteContributions(CDBWrapper& db) const EXCLUSIVE_LOCKS_REQUIRED(!cs_vvec_shShare); + bool ReadContributions(const CDBWrapper& db) EXCLUSIVE_LOCKS_REQUIRED(!cs_vvec_shShare); }; /** @@ -242,7 +244,7 @@ class CQuorumManager mutable Mutex cs_map_quorums; mutable std::map> mapQuorumsCache GUARDED_BY(cs_map_quorums); - mutable Mutex cs_scan_quorums; + mutable Mutex cs_scan_quorums; // TODO: merge cs_map_quorums, cs_scan_quorums mutexes mutable std::map>> scanQuorumsCache GUARDED_BY(cs_scan_quorums); mutable Mutex cs_cleanup; @@ -266,11 +268,15 @@ class CQuorumManager void Start(); void Stop(); - void TriggerQuorumDataRecoveryThreads(CConnman& connman, const CBlockIndex* pIndex) const; + void TriggerQuorumDataRecoveryThreads(CConnman& connman, const CBlockIndex* pIndex) const + EXCLUSIVE_LOCKS_REQUIRED(!cs_db, !cs_scan_quorums, !cs_map_quorums); - void UpdatedBlockTip(const CBlockIndex* pindexNew, CConnman& connman, bool fInitialDownload) const; + void UpdatedBlockTip(const CBlockIndex* pindexNew, CConnman& connman, bool fInitialDownload) const + EXCLUSIVE_LOCKS_REQUIRED(!cs_db, !cs_scan_quorums, !cs_map_quorums); - [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& pfrom, CConnman& connman, std::string_view msg_type, CDataStream& vRecv); + [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& pfrom, CConnman& connman, std::string_view msg_type, + CDataStream& vRecv) + EXCLUSIVE_LOCKS_REQUIRED(!cs_map_quorums, !cs_db); static bool HasQuorum(Consensus::LLMQType llmqType, const CQuorumBlockProcessor& quorum_block_processor, const uint256& quorumHash); @@ -278,21 +284,28 @@ class CQuorumManager const uint256& proTxHash = uint256()) const; // all these methods will lock cs_main for a short period of time - CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash) const; - std::vector ScanQuorums(Consensus::LLMQType llmqType, size_t nCountRequested) const; + CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash) const + EXCLUSIVE_LOCKS_REQUIRED(!cs_db, !cs_map_quorums); + std::vector ScanQuorums(Consensus::LLMQType llmqType, size_t nCountRequested) const + EXCLUSIVE_LOCKS_REQUIRED(!cs_db, !cs_map_quorums, !cs_scan_quorums); // this one is cs_main-free - std::vector ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t nCountRequested) const; + std::vector ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, + size_t nCountRequested) const + EXCLUSIVE_LOCKS_REQUIRED(!cs_db, !cs_map_quorums, !cs_scan_quorums); private: // all private methods here are cs_main-free - void CheckQuorumConnections(CConnman& connman, const Consensus::LLMQParams& llmqParams, - const CBlockIndex* pindexNew) const; + void CheckQuorumConnections(CConnman& connman, const Consensus::LLMQParams& llmqParams, const CBlockIndex* pindexNew) const + EXCLUSIVE_LOCKS_REQUIRED(!cs_db, !cs_scan_quorums, !cs_map_quorums); - CQuorumPtr BuildQuorumFromCommitment(Consensus::LLMQType llmqType, gsl::not_null pQuorumBaseBlockIndex, bool populate_cache) const; + CQuorumPtr BuildQuorumFromCommitment(Consensus::LLMQType llmqType, + gsl::not_null pQuorumBaseBlockIndex, + bool populate_cache) const EXCLUSIVE_LOCKS_REQUIRED(!cs_db, !cs_map_quorums); bool BuildQuorumContributions(const CFinalCommitmentPtr& fqc, const std::shared_ptr& quorum) const; - CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, gsl::not_null pindex, bool populate_cache = true) const; + CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, gsl::not_null pindex, + bool populate_cache = true) const EXCLUSIVE_LOCKS_REQUIRED(!cs_db, !cs_map_quorums); /// Returns the start offset for the masternode with the given proTxHash. This offset is applied when picking data recovery members of a quorum's /// memberlist and is calculated based on a list of all member of all active quorums for the given llmqType in a way that each member /// should receive the same number of request if all active llmqType members requests data from one llmqType quorum. @@ -303,7 +316,7 @@ class CQuorumManager uint16_t nDataMask) const; void StartCleanupOldQuorumDataThread(const CBlockIndex* pIndex) const; - void MigrateOldQuorumDB(CEvoDB& evoDb) const; + void MigrateOldQuorumDB(CEvoDB& evoDb) const EXCLUSIVE_LOCKS_REQUIRED(!cs_db); }; // when selecting a quorum for signing and verification, we use CQuorumManager::SelectQuorum with this offset as diff --git a/src/llmq/signing.h b/src/llmq/signing.h index 2425de093604..e49e76a14329 100644 --- a/src/llmq/signing.h +++ b/src/llmq/signing.h @@ -127,15 +127,15 @@ class CRecoveredSigsDb ~CRecoveredSigsDb(); bool HasRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) const; - bool HasRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id) const; - bool HasRecoveredSigForSession(const uint256& signHash) const; - bool HasRecoveredSigForHash(const uint256& hash) const; + bool HasRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id) const EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); + bool HasRecoveredSigForSession(const uint256& signHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); + bool HasRecoveredSigForHash(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); bool GetRecoveredSigByHash(const uint256& hash, CRecoveredSig& ret) const; bool GetRecoveredSigById(Consensus::LLMQType llmqType, const uint256& id, CRecoveredSig& ret) const; - void WriteRecoveredSig(const CRecoveredSig& recSig); - void TruncateRecoveredSig(Consensus::LLMQType llmqType, const uint256& id); + void WriteRecoveredSig(const CRecoveredSig& recSig) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); + void TruncateRecoveredSig(Consensus::LLMQType llmqType, const uint256& id) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); - void CleanupOldRecoveredSigs(int64_t maxAge); + void CleanupOldRecoveredSigs(int64_t maxAge) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); // votes are removed when the recovered sig is written to the db bool HasVotedOnId(Consensus::LLMQType llmqType, const uint256& id) const; @@ -146,7 +146,8 @@ class CRecoveredSigsDb private: bool ReadRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, CRecoveredSig& ret) const; - void RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType llmqType, const uint256& id, bool deleteHashKey, bool deleteTimeKey); + void RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType llmqType, const uint256& id, bool deleteHashKey, + bool deleteTimeKey) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); }; class CRecoveredSigsListener @@ -182,14 +183,16 @@ class CSigningManager CSigningManager(const CActiveMasternodeManager* const mn_activeman, const CChainState& chainstate, const CQuorumManager& _qman, bool fMemory, bool fWipe); - bool AlreadyHave(const CInv& inv) const; + bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); bool GetRecoveredSigForGetData(const uint256& hash, CRecoveredSig& ret) const; - [[nodiscard]] MessageProcessingResult ProcessMessage(NodeId from, std::string_view msg_type, CDataStream& vRecv); + [[nodiscard]] MessageProcessingResult ProcessMessage(NodeId from, std::string_view msg_type, CDataStream& vRecv) + EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); // This is called when a recovered signature was was reconstructed from another P2P message and is known to be valid // This is the case for example when a signature appears as part of InstantSend or ChainLocks - void PushReconstructedRecoveredSig(const std::shared_ptr& recoveredSig); + void PushReconstructedRecoveredSig(const std::shared_ptr& recoveredSig) + EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); // This is called when a recovered signature can be safely removed from the DB. This is only safe when some other // mechanism prevents possible conflicts. As an example, ChainLocks prevent conflicts in confirmed TXs InstantSend votes @@ -198,22 +201,26 @@ class CSigningManager void TruncateRecoveredSig(Consensus::LLMQType llmqType, const uint256& id); private: - void CollectPendingRecoveredSigsToVerify(size_t maxUniqueSessions, - std::unordered_map>>& retSigShares, - std::unordered_map, CQuorumCPtr, StaticSaltedHasher>& retQuorums); - void ProcessPendingReconstructedRecoveredSigs(PeerManager& peerman); - bool ProcessPendingRecoveredSigs(PeerManager& peerman); // called from the worker thread of CSigSharesManager + void CollectPendingRecoveredSigsToVerify( + size_t maxUniqueSessions, std::unordered_map>>& retSigShares, + std::unordered_map, CQuorumCPtr, StaticSaltedHasher>& retQuorums) + EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); + void ProcessPendingReconstructedRecoveredSigs(PeerManager& peerman) + EXCLUSIVE_LOCKS_REQUIRED(!cs_pending, !cs_listeners); + bool ProcessPendingRecoveredSigs(PeerManager& peerman) + EXCLUSIVE_LOCKS_REQUIRED(!cs_pending, !cs_listeners); // called from the worker thread of CSigSharesManager public: // TODO - should not be public! - void ProcessRecoveredSig(const std::shared_ptr& recoveredSig, PeerManager& peerman); + void ProcessRecoveredSig(const std::shared_ptr& recoveredSig, PeerManager& peerman) + EXCLUSIVE_LOCKS_REQUIRED(!cs_pending, !cs_listeners); private: void Cleanup(); // called from the worker thread of CSigSharesManager public: // public interface - void RegisterRecoveredSigsListener(CRecoveredSigsListener* l); - void UnregisterRecoveredSigsListener(CRecoveredSigsListener* l); + void RegisterRecoveredSigsListener(CRecoveredSigsListener* l) EXCLUSIVE_LOCKS_REQUIRED(!cs_listeners); + void UnregisterRecoveredSigsListener(CRecoveredSigsListener* l) EXCLUSIVE_LOCKS_REQUIRED(!cs_listeners); bool AsyncSignIfMember(Consensus::LLMQType llmqType, CSigSharesManager& shareman, const uint256& id, const uint256& msgHash, const uint256& quorumHash = uint256(), bool allowReSign = false, @@ -229,7 +236,8 @@ class CSigningManager private: std::thread workThread; CThreadInterrupt workInterrupt; - void WorkThreadMain(PeerManager& peerman); + void WorkThreadMain(PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!cs_pending, !cs_listeners); + ; public: void StartWorkerThread(PeerManager& peerman); diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index b47a41c16a1d..258a6f2417a6 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -433,7 +433,8 @@ class CSigSharesManager : public CRecoveredSigsListener void ProcessMessage(const CNode& pnode, PeerManager& peerman, const CSporkManager& sporkman, const std::string& msg_type, CDataStream& vRecv); - void AsyncSign(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash); + void AsyncSign(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) + EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns); std::optional CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const; void ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash); @@ -485,8 +486,8 @@ class CSigSharesManager : public CRecoveredSigsListener void CollectSigSharesToAnnounce(const CConnman& connman, std::unordered_map>& sigSharesToAnnounce) EXCLUSIVE_LOCKS_REQUIRED(cs); - void SignPendingSigShares(const CConnman& connman, PeerManager& peerman); - void WorkThreadMain(CConnman& connman, PeerManager& peerman); + void SignPendingSigShares(const CConnman& connman, PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns); + void WorkThreadMain(CConnman& connman, PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns); }; } // namespace llmq diff --git a/src/masternode/meta.cpp b/src/masternode/meta.cpp index 93dda0796f13..adb2aaa30259 100644 --- a/src/masternode/meta.cpp +++ b/src/masternode/meta.cpp @@ -66,7 +66,7 @@ void CMasternodeMetaInfo::RemoveGovernanceObject(const uint256& nGovernanceObjec mapGovernanceObjectsVotedOn.erase(nGovernanceObjectHash); } -CMasternodeMetaInfoPtr CMasternodeMetaMan::GetMetaInfo(const uint256& proTxHash, bool fCreate) EXCLUSIVE_LOCKS_REQUIRED(!cs) +CMasternodeMetaInfoPtr CMasternodeMetaMan::GetMetaInfo(const uint256& proTxHash, bool fCreate) { LOCK(cs); auto it = metaInfos.find(proTxHash); @@ -115,7 +115,7 @@ bool CMasternodeMetaMan::AddGovernanceVote(const uint256& proTxHash, const uint2 return true; } -void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObjectHash) EXCLUSIVE_LOCKS_REQUIRED(!cs) +void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObjectHash) { LOCK(cs); for(const auto& p : metaInfos) { @@ -123,20 +123,20 @@ void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObject } } -std::vector CMasternodeMetaMan::GetAndClearDirtyGovernanceObjectHashes() EXCLUSIVE_LOCKS_REQUIRED(!cs) +std::vector CMasternodeMetaMan::GetAndClearDirtyGovernanceObjectHashes() { std::vector vecTmp; WITH_LOCK(cs, vecTmp.swap(vecDirtyGovernanceObjectHashes)); return vecTmp; } -bool CMasternodeMetaMan::AlreadyHavePlatformBan(const uint256& inv_hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs) +bool CMasternodeMetaMan::AlreadyHavePlatformBan(const uint256& inv_hash) const { LOCK(cs); return m_seen_platform_bans.exists(inv_hash); } -std::optional CMasternodeMetaMan::GetPlatformBan(const uint256& inv_hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs) +std::optional CMasternodeMetaMan::GetPlatformBan(const uint256& inv_hash) const { LOCK(cs); PlatformBanMessage ret; @@ -147,13 +147,13 @@ std::optional CMasternodeMetaMan::GetPlatformBan(const uint2 return ret; } -void CMasternodeMetaMan::RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg) EXCLUSIVE_LOCKS_REQUIRED(!cs) +void CMasternodeMetaMan::RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg) { LOCK(cs); m_seen_platform_bans.insert(inv_hash, std::move(msg)); } -std::string MasternodeMetaStore::ToString() const EXCLUSIVE_LOCKS_REQUIRED(!cs) +std::string MasternodeMetaStore::ToString() const { LOCK(cs); return strprintf("Masternodes: meta infos object count: %d, nDsqCount: %d", metaInfos.size(), nDsqCount); diff --git a/src/masternode/meta.h b/src/masternode/meta.h index 8d8df8d0df92..217b9d800287 100644 --- a/src/masternode/meta.h +++ b/src/masternode/meta.h @@ -70,18 +70,26 @@ class CMasternodeMetaInfo { } - SERIALIZE_METHODS(CMasternodeMetaInfo, obj) + template + void Serialize(Stream& s) const EXCLUSIVE_LOCKS_REQUIRED(!cs) { - LOCK(obj.cs); - READWRITE(obj.proTxHash, obj.nLastDsq, obj.nMixingTxCount, obj.mapGovernanceObjectsVotedOn, - obj.outboundAttemptCount, obj.lastOutboundAttempt, obj.lastOutboundSuccess, obj.m_platform_ban, - obj.m_platform_ban_updated); + LOCK(cs); + s << proTxHash << nLastDsq << nMixingTxCount << mapGovernanceObjectsVotedOn << outboundAttemptCount + << lastOutboundAttempt << lastOutboundSuccess << m_platform_ban << m_platform_ban_updated; + } + + template + void Unserialize(Stream& s) EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + LOCK(cs); + s >> proTxHash >> nLastDsq >> nMixingTxCount >> mapGovernanceObjectsVotedOn >> outboundAttemptCount >> + lastOutboundAttempt >> lastOutboundSuccess >> m_platform_ban >> m_platform_ban_updated; } - UniValue ToJson() const; + UniValue ToJson() const EXCLUSIVE_LOCKS_REQUIRED(!cs); public: - const uint256 GetProTxHash() const + const uint256 GetProTxHash() const EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); return proTxHash; @@ -92,16 +100,16 @@ class CMasternodeMetaInfo bool IsValidForMixingTxes() const { return GetMixingTxCount() <= MASTERNODE_MAX_MIXING_TXES; } // KEEP TRACK OF EACH GOVERNANCE ITEM IN CASE THIS NODE GOES OFFLINE, SO WE CAN RECALCULATE THEIR STATUS - void AddGovernanceVote(const uint256& nGovernanceObjectHash); + void AddGovernanceVote(const uint256& nGovernanceObjectHash) EXCLUSIVE_LOCKS_REQUIRED(!cs); - void RemoveGovernanceObject(const uint256& nGovernanceObjectHash); + void RemoveGovernanceObject(const uint256& nGovernanceObjectHash) EXCLUSIVE_LOCKS_REQUIRED(!cs); bool OutboundFailedTooManyTimes() const { return outboundAttemptCount > MASTERNODE_MAX_FAILED_OUTBOUND_ATTEMPTS; } void SetLastOutboundAttempt(int64_t t) { lastOutboundAttempt = t; ++outboundAttemptCount; } int64_t GetLastOutboundAttempt() const { return lastOutboundAttempt; } void SetLastOutboundSuccess(int64_t t) { lastOutboundSuccess = t; outboundAttemptCount = 0; } int64_t GetLastOutboundSuccess() const { return lastOutboundSuccess; } - bool SetPlatformBan(bool is_banned, int height) + bool SetPlatformBan(bool is_banned, int height) EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); if (height < m_platform_ban_updated) { @@ -114,7 +122,7 @@ class CMasternodeMetaInfo m_platform_ban_updated = height; return true; } - bool IsPlatformBanned() const + bool IsPlatformBanned() const EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); return m_platform_ban; diff --git a/src/net.cpp b/src/net.cpp index b339dd451460..ea96b1162a40 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -128,7 +128,7 @@ static const uint64_t RANDOMIZER_ID_ADDRCACHE = 0x1cf2e4ddd306dda9ULL; // SHA256 // bool fDiscover = true; bool fListen = true; -Mutex g_maplocalhost_mutex; +GlobalMutex g_maplocalhost_mutex; std::map mapLocalHost GUARDED_BY(g_maplocalhost_mutex); std::string strSubVersion; diff --git a/src/net.h b/src/net.h index 1f86bf5a0199..bbbc260a1e11 100644 --- a/src/net.h +++ b/src/net.h @@ -199,7 +199,7 @@ struct LocalServiceInfo { uint16_t nPort; }; -extern Mutex g_maplocalhost_mutex; +extern GlobalMutex g_maplocalhost_mutex; extern std::map mapLocalHost GUARDED_BY(g_maplocalhost_mutex); extern const std::string NET_MESSAGE_TYPE_OTHER; @@ -1027,7 +1027,7 @@ class CNode void CloseSocketDisconnect(CConnman* connman) EXCLUSIVE_LOCKS_REQUIRED(!m_sock_mutex); - void CopyStats(CNodeStats& stats) EXCLUSIVE_LOCKS_REQUIRED(!m_subver_mutex, !m_addr_local_mutex, !cs_vSend, !cs_vRecv); + void CopyStats(CNodeStats& stats) EXCLUSIVE_LOCKS_REQUIRED(!m_subver_mutex, !m_addr_local_mutex, !cs_vSend, !cs_vRecv, !cs_mnauth); std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); } @@ -1041,42 +1041,42 @@ class CNode bool CanRelay() const { return !m_masternode_connection || m_masternode_iqr_connection; } - uint256 GetSentMNAuthChallenge() const { + uint256 GetSentMNAuthChallenge() const EXCLUSIVE_LOCKS_REQUIRED(!cs_mnauth) { LOCK(cs_mnauth); return sentMNAuthChallenge; } - uint256 GetReceivedMNAuthChallenge() const { + uint256 GetReceivedMNAuthChallenge() const EXCLUSIVE_LOCKS_REQUIRED(!cs_mnauth) { LOCK(cs_mnauth); return receivedMNAuthChallenge; } - uint256 GetVerifiedProRegTxHash() const { + uint256 GetVerifiedProRegTxHash() const EXCLUSIVE_LOCKS_REQUIRED(!cs_mnauth) { LOCK(cs_mnauth); return verifiedProRegTxHash; } - uint256 GetVerifiedPubKeyHash() const { + uint256 GetVerifiedPubKeyHash() const EXCLUSIVE_LOCKS_REQUIRED(!cs_mnauth) { LOCK(cs_mnauth); return verifiedPubKeyHash; } - void SetSentMNAuthChallenge(const uint256& newSentMNAuthChallenge) { + void SetSentMNAuthChallenge(const uint256& newSentMNAuthChallenge) EXCLUSIVE_LOCKS_REQUIRED(!cs_mnauth) { LOCK(cs_mnauth); sentMNAuthChallenge = newSentMNAuthChallenge; } - void SetReceivedMNAuthChallenge(const uint256& newReceivedMNAuthChallenge) { + void SetReceivedMNAuthChallenge(const uint256& newReceivedMNAuthChallenge) EXCLUSIVE_LOCKS_REQUIRED(!cs_mnauth) { LOCK(cs_mnauth); receivedMNAuthChallenge = newReceivedMNAuthChallenge; } - void SetVerifiedProRegTxHash(const uint256& newVerifiedProRegTxHash) { + void SetVerifiedProRegTxHash(const uint256& newVerifiedProRegTxHash) EXCLUSIVE_LOCKS_REQUIRED(!cs_mnauth) { LOCK(cs_mnauth); verifiedProRegTxHash = newVerifiedProRegTxHash; } - void SetVerifiedPubKeyHash(const uint256& newVerifiedPubKeyHash) { + void SetVerifiedPubKeyHash(const uint256& newVerifiedPubKeyHash) EXCLUSIVE_LOCKS_REQUIRED(!cs_mnauth) { LOCK(cs_mnauth); verifiedPubKeyHash = newVerifiedPubKeyHash; } @@ -1244,8 +1244,8 @@ friend class CNode; EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !m_added_nodes_mutex, !m_addr_fetches_mutex, !mutexMsgProc); void StopThreads(); - void StopNodes(); - void Stop() + void StopNodes() EXCLUSIVE_LOCKS_REQUIRED(!cs_mapSocketToNode, !cs_sendable_receivable_nodes); + void Stop() EXCLUSIVE_LOCKS_REQUIRED(!cs_mapSocketToNode, !cs_sendable_receivable_nodes) { StopThreads(); StopNodes(); @@ -1274,9 +1274,9 @@ friend class CNode; const char* strDest, ConnectionType conn_type, bool use_v2transport, MasternodeConn masternode_connection = MasternodeConn::IsNotConnection, MasternodeProbeConn masternode_probe_connection = MasternodeProbeConn::IsNotConnection) - EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !mutexMsgProc, !cs_mapSocketToNode); void OpenMasternodeConnection(const CAddress& addrConnect, bool use_v2transport, MasternodeProbeConn probe = MasternodeProbeConn::IsConnection) - EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !mutexMsgProc, !cs_mapSocketToNode); bool CheckIncomingNonce(uint64_t nonce); // alias for thread safety annotations only, not defined @@ -1477,7 +1477,7 @@ friend class CNode; * - Max connection capacity for type is filled */ bool AddConnection(const std::string& address, ConnectionType conn_type, bool use_v2transport) - EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !mutexMsgProc, !cs_mapSocketToNode); bool AddPendingMasternode(const uint256& proTxHash); void SetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash, const Uint256HashSet& proTxHashes); @@ -1579,16 +1579,16 @@ friend class CNode; bool InitBinds(const Options& options); void ThreadOpenAddedConnections() - EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex, !mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex, !mutexMsgProc, !cs_mapSocketToNode); void AddAddrFetch(const std::string& strDest) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex); void ProcessAddrFetch() - EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_unused_i2p_sessions_mutex, !mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_unused_i2p_sessions_mutex, !mutexMsgProc, !cs_mapSocketToNode); void ThreadOpenConnections(const std::vector connect, CDeterministicMNManager& dmnman) - EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex, !mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex, !mutexMsgProc, !cs_mapSocketToNode); void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); - void ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + void ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !cs_mapSocketToNode); void AcceptConnection(const ListenSocket& hListenSocket, CMasternodeSync& mn_sync) - EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !cs_mapSocketToNode); /** * Create a `CNode` object from a socket that has just been accepted and add the node to @@ -1602,7 +1602,7 @@ friend class CNode; NetPermissionFlags permission_flags, const CAddress& addr_bind, const CAddress& addr, - CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !cs_mapSocketToNode); void DisconnectNodes() EXCLUSIVE_LOCKS_REQUIRED(!m_reconnections_mutex, !m_nodes_mutex); void NotifyNumConnectionsChanged(CMasternodeSync& mn_sync); @@ -1620,28 +1620,28 @@ friend class CNode; /** * Check connected and listening sockets for IO readiness and process them accordingly. */ - void SocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc); + void SocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !cs_mapSocketToNode, !cs_sendable_receivable_nodes); /** * Do the read/write for connected sockets that are ready for IO. * @param[in] events_per_sock Sockets that are ready for IO. */ void SocketHandlerConnected(const Sock::EventsPerSock& events_per_sock) - EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !cs_sendable_receivable_nodes, !cs_mapSocketToNode, !cs_sendable_receivable_nodes); /** * Accept incoming connections, one from each read-ready listening socket. * @param[in] events_per_sock Sockets that are ready for IO. */ void SocketHandlerListening(const Sock::EventsPerSock& events_per_sock, CMasternodeSync& mn_sync) - EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !cs_mapSocketToNode); void ThreadSocketHandler(CMasternodeSync& mn_sync) - EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex, !m_reconnections_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex, !m_reconnections_mutex, !cs_mapSocketToNode, !cs_sendable_receivable_nodes); void ThreadDNSAddressSeed() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex); void ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync) - EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex, !m_unused_i2p_sessions_mutex, !mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex, !m_unused_i2p_sessions_mutex, !mutexMsgProc, !cs_mapSocketToNode); uint64_t CalculateKeyedNetGroup(const CAddress& ad) const; @@ -1934,7 +1934,7 @@ friend class CNode; std::list m_reconnections GUARDED_BY(m_reconnections_mutex); /** Attempt reconnections, if m_reconnections non-empty. */ - void PerformReconnections() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !m_reconnections_mutex, !m_unused_i2p_sessions_mutex); + void PerformReconnections() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !m_reconnections_mutex, !m_unused_i2p_sessions_mutex, !cs_mapSocketToNode); /** * Cap on the size of `m_unused_i2p_sessions`, to ensure it does not diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6e104ac8ab5b..284b7942ef75 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -324,7 +324,7 @@ struct Peer { /** * (Bitcoin) Initializes a TxRelay struct for this peer. Can be called at most once for a peer. * (Dash) Enables the flag that allows GetTxRelay() to return m_tx_relay */ - TxRelay* SetTxRelay() LOCKS_EXCLUDED(m_tx_relay_mutex) + TxRelay* SetTxRelay() EXCLUSIVE_LOCKS_REQUIRED(!m_tx_relay_mutex) { LOCK(m_tx_relay_mutex); Assume(!m_can_tx_relay); @@ -332,17 +332,17 @@ struct Peer { return m_tx_relay.get(); }; - TxRelay* GetInvRelay() LOCKS_EXCLUDED(m_tx_relay_mutex) + TxRelay* GetInvRelay() EXCLUSIVE_LOCKS_REQUIRED(!m_tx_relay_mutex) { return WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get()); } - TxRelay* GetTxRelay() LOCKS_EXCLUDED(m_tx_relay_mutex) + TxRelay* GetTxRelay() EXCLUSIVE_LOCKS_REQUIRED(!m_tx_relay_mutex) { LOCK(m_tx_relay_mutex); return m_can_tx_relay ? m_tx_relay.get() : nullptr; }; - const TxRelay* GetTxRelay() const LOCKS_EXCLUDED(m_tx_relay_mutex) + const TxRelay* GetTxRelay() const EXCLUSIVE_LOCKS_REQUIRED(!m_tx_relay_mutex) { LOCK(m_tx_relay_mutex); return m_can_tx_relay ? m_tx_relay.get() : nullptr; @@ -611,15 +611,15 @@ class PeerManagerImpl final : public PeerManager EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void BlockChecked(const CBlock& block, const BlockValidationState& state) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); - void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& pblock) override; + void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& pblock) override EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex); /** Implement NetEventsInterface */ void InitializeNode(CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool ProcessMessages(CNode* pfrom, std::atomic& interrupt) override - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, g_msgproc_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, g_msgproc_mutex, !m_most_recent_block_mutex); bool SendMessages(CNode* pto) override - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, g_msgproc_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, g_msgproc_mutex, !m_most_recent_block_mutex); /** Implement PeerManager */ void StartScheduledTasks(CScheduler& scheduler) override; @@ -639,12 +639,12 @@ class PeerManagerImpl final : public PeerManager void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message = "") override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) override - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, g_msgproc_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, g_msgproc_mutex, !m_most_recent_block_mutex); void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override; bool IsBanned(NodeId pnode) override EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_peer_mutex); size_t GetRequestedObjectCount(NodeId nodeid) const override EXCLUSIVE_LOCKS_REQUIRED(::cs_main); private: - void _RelayTransaction(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void _RelayTransaction(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_peer_mutex); /** Ask peers that have a transaction in their inventory to relay it to us. */ void AskPeersForTransaction(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); @@ -852,7 +852,7 @@ class PeerManagerImpl final : public PeerManager */ bool BlockRequestAllowed(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool AlreadyHaveBlock(const uint256& block_hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& inv, llmq::CInstantSendManager& isman); + void ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& inv, llmq::CInstantSendManager& isman) EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex); /** * Validation logic for compact filters request handling. @@ -1034,7 +1034,7 @@ class PeerManagerImpl final : public PeerManager /** Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). */ CTransactionRef FindTxForGetData(const CNode* peer, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) LOCKS_EXCLUDED(cs_main); - void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(peer.m_getdata_requests_mutex) LOCKS_EXCLUDED(::cs_main); + void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(peer.m_getdata_requests_mutex, !m_most_recent_block_mutex) LOCKS_EXCLUDED(::cs_main); /** Process a new block. Perform any post-processing housekeeping */ void ProcessBlock(CNode& from, const std::shared_ptr& pblock, bool force_processing); diff --git a/src/netbase.cpp b/src/netbase.cpp index 64034b919b50..7ac84ad4e3d0 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -29,7 +29,7 @@ #endif // Settings -static Mutex g_proxyinfo_mutex; +static GlobalMutex g_proxyinfo_mutex; static Proxy proxyInfo[NET_MAX] GUARDED_BY(g_proxyinfo_mutex); static Proxy nameProxy GUARDED_BY(g_proxyinfo_mutex); int nConnectTimeout = DEFAULT_CONNECT_TIMEOUT; diff --git a/src/qt/clientmodel.h b/src/qt/clientmodel.h index f4f39a4f61f1..241639e2cd14 100644 --- a/src/qt/clientmodel.h +++ b/src/qt/clientmodel.h @@ -123,7 +123,7 @@ class ClientModel : public QObject std::unique_ptr mnListCached GUARDED_BY(cs_mnlist){}; const CBlockIndex* mnListTip{nullptr}; - void TipChanged(SynchronizationState sync_state, interfaces::BlockTip tip, double verification_progress, bool header); + void TipChanged(SynchronizationState sync_state, interfaces::BlockTip tip, double verification_progress, bool header) EXCLUSIVE_LOCKS_REQUIRED(!m_cached_tip_mutex); void subscribeToCoreSignals(); void unsubscribeFromCoreSignals(); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 35e4d54c1f25..ac07d6a5a029 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -77,7 +77,7 @@ struct CUpdatedBlock int height; }; -static Mutex cs_blockchange; +static GlobalMutex cs_blockchange; static std::condition_variable cond_blockchange; static CUpdatedBlock latestblock GUARDED_BY(cs_blockchange); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 80e07ad9b1d5..1932268917ae 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -23,14 +23,14 @@ #include #include -static Mutex g_rpc_warmup_mutex; +static GlobalMutex g_rpc_warmup_mutex; static std::atomic g_rpc_running{false}; static bool fRPCInWarmup GUARDED_BY(g_rpc_warmup_mutex) = true; static std::string rpcWarmupStatus GUARDED_BY(g_rpc_warmup_mutex) = "RPC server started"; /* Timer-creating functions */ static RPCTimerInterface* timerInterface = nullptr; /* Map of name to timer. */ -static Mutex g_deadline_timers_mutex; +static GlobalMutex g_deadline_timers_mutex; static std::map > deadlineTimers GUARDED_BY(g_deadline_timers_mutex); static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler); diff --git a/src/spork.cpp b/src/spork.cpp index 021ab690e139..0b66222d6911 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -31,7 +31,7 @@ std::optional CSporkManager::SporkValueIfActive(SporkId nSporkID) co if (!mapSporksActive.count(nSporkID)) return std::nullopt; { - LOCK(cs_mapSporksCachedValues); + LOCK(cs_cache); if (auto it = mapSporksCachedValues.find(nSporkID); it != mapSporksCachedValues.end()) { return {it->second}; } @@ -45,7 +45,7 @@ std::optional CSporkManager::SporkValueIfActive(SporkId nSporkID) co // nMinSporkKeys is always more than the half of the max spork keys number, // so there is only one such value and we can stop here { - LOCK(cs_mapSporksCachedValues); + LOCK(cs_cache); mapSporksCachedValues[nSporkID] = spork.nValue; } return {spork.nValue}; @@ -184,8 +184,11 @@ MessageProcessingResult CSporkManager::ProcessSpork(NodeId from, CDataStream& vR mapSporksByHash[hash] = spork; mapSporksActive[spork.nSporkID][keyIDSigner] = spork; // Clear cached values on new spork being processed - WITH_LOCK(cs_mapSporksCachedActive, mapSporksCachedActive.erase(spork.nSporkID)); - WITH_LOCK(cs_mapSporksCachedValues, mapSporksCachedValues.erase(spork.nSporkID)); + { + LOCK(cs_cache); + mapSporksCachedActive.erase(spork.nSporkID); + mapSporksCachedValues.erase(spork.nSporkID); + } } ret.m_inventory.emplace_back(MSG_SPORK, hash); @@ -226,8 +229,10 @@ bool CSporkManager::UpdateSpork(PeerManager& peerman, SporkId nSporkID, SporkVal mapSporksByHash[spork.GetHash()] = spork; mapSporksActive[nSporkID][*opt_keyIDSigner] = spork; // Clear cached values on new spork being processed - WITH_LOCK(cs_mapSporksCachedActive, mapSporksCachedActive.erase(spork.nSporkID)); - WITH_LOCK(cs_mapSporksCachedValues, mapSporksCachedValues.erase(spork.nSporkID)); + + LOCK(cs_cache); + mapSporksCachedActive.erase(spork.nSporkID); + mapSporksCachedValues.erase(spork.nSporkID); } spork.Relay(peerman); @@ -238,7 +243,7 @@ bool CSporkManager::IsSporkActive(SporkId nSporkID) const { // If nSporkID is cached, and the cached value is true, then return early true { - LOCK(cs_mapSporksCachedActive); + LOCK(cs_cache); if (auto it = mapSporksCachedActive.find(nSporkID); it != mapSporksCachedActive.end() && it->second) { return true; } @@ -249,7 +254,7 @@ bool CSporkManager::IsSporkActive(SporkId nSporkID) const bool ret = nSporkValue < GetAdjustedTime(); // Only cache true values if (ret) { - LOCK(cs_mapSporksCachedActive); + LOCK(cs_cache); mapSporksCachedActive[nSporkID] = ret; } return ret; diff --git a/src/spork.h b/src/spork.h index 6f348a50a93e..5dde49cb4dbe 100644 --- a/src/spork.h +++ b/src/spork.h @@ -220,11 +220,11 @@ class CSporkManager : public SporkStore const std::unique_ptr m_db; bool is_valid{false}; - mutable Mutex cs_mapSporksCachedActive; - mutable std::unordered_map mapSporksCachedActive GUARDED_BY(cs_mapSporksCachedActive); - - mutable Mutex cs_mapSporksCachedValues; - mutable std::unordered_map mapSporksCachedValues GUARDED_BY(cs_mapSporksCachedValues); + // TODO: drop mutex cs_cache completely so far as sporks are used on testnet only + // and simplify IsSporkActive to avoid any mutex for better mainnet performance + mutable Mutex cs_cache; + mutable std::unordered_map mapSporksCachedActive GUARDED_BY(cs_cache); + mutable std::unordered_map mapSporksCachedValues GUARDED_BY(cs_cache); std::set setSporkPubKeyIDs GUARDED_BY(cs); int nMinSporkKeys GUARDED_BY(cs) {std::numeric_limits::max()}; @@ -234,7 +234,7 @@ class CSporkManager : public SporkStore * SporkValueIfActive is used to get the value agreed upon by the majority * of signed spork messages for a given Spork ID. */ - std::optional SporkValueIfActive(SporkId nSporkID) const EXCLUSIVE_LOCKS_REQUIRED(cs); + std::optional SporkValueIfActive(SporkId nSporkID) const EXCLUSIVE_LOCKS_REQUIRED(cs, !cs_cache); public: CSporkManager(); @@ -257,7 +257,8 @@ class CSporkManager : public SporkStore /** * ProcessMessage is used to call ProcessSpork and ProcessGetSporks. See below */ - [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv); + [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, + CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); /** * ProcessSpork is used to handle the 'spork' p2p message. @@ -265,7 +266,8 @@ class CSporkManager : public SporkStore * For 'spork', it validates the spork and adds it to the internal spork storage and * performs any necessary processing. */ - [[nodiscard]] MessageProcessingResult ProcessSpork(NodeId from, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs); + [[nodiscard]] MessageProcessingResult ProcessSpork(NodeId from, CDataStream& vRecv) + EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_cache); /** * ProcessGetSporks is used to handle the 'getsporks' p2p message. @@ -278,7 +280,7 @@ class CSporkManager : public SporkStore * UpdateSpork is used by the spork RPC command to set a new spork value, sign * and broadcast the spork message. */ - bool UpdateSpork(PeerManager& peerman, SporkId nSporkID, SporkValue nValue) EXCLUSIVE_LOCKS_REQUIRED(!cs); + bool UpdateSpork(PeerManager& peerman, SporkId nSporkID, SporkValue nValue) EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_cache); /** * IsSporkActive returns a bool for time-based sporks, and should be used @@ -288,13 +290,13 @@ class CSporkManager : public SporkStore * instead, and therefore this method doesn't make sense and should not be * used. */ - bool IsSporkActive(SporkId nSporkID) const; + bool IsSporkActive(SporkId nSporkID) const EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); /** * GetSporkValue returns the spork value given a Spork ID. If no active spork * message has yet been received by the node, it returns the default value. */ - SporkValue GetSporkValue(SporkId nSporkID) const EXCLUSIVE_LOCKS_REQUIRED(!cs); + SporkValue GetSporkValue(SporkId nSporkID) const EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_cache); /** * GetSporkIDByName returns the internal Spork ID given the spork name. diff --git a/src/stats/client.cpp b/src/stats/client.cpp index d0e3a938e1e1..973fcb266941 100644 --- a/src/stats/client.cpp +++ b/src/stats/client.cpp @@ -48,24 +48,63 @@ class StatsdClientImpl final : public StatsdClient ~StatsdClientImpl() = default; public: - bool dec(std::string_view key, float sample_rate) override { return count(key, -1, sample_rate); } - bool inc(std::string_view key, float sample_rate) override { return count(key, 1, sample_rate); } - bool count(std::string_view key, int64_t delta, float sample_rate) override { return _send(key, delta, STATSD_METRIC_COUNT, sample_rate); } - bool gauge(std::string_view key, int64_t value, float sample_rate) override { return _send(key, value, STATSD_METRIC_GAUGE, sample_rate); } - bool gaugeDouble(std::string_view key, double value, float sample_rate) override { return _send(key, value, STATSD_METRIC_GAUGE, sample_rate); } - bool timing(std::string_view key, uint64_t ms, float sample_rate) override { return _send(key, ms, STATSD_METRIC_TIMING, sample_rate); } - - bool send(std::string_view key, double value, std::string_view type, float sample_rate) override { return _send(key, value, type, sample_rate); } - bool send(std::string_view key, int32_t value, std::string_view type, float sample_rate) override { return _send(key, value, type, sample_rate); } - bool send(std::string_view key, int64_t value, std::string_view type, float sample_rate) override { return _send(key, value, type, sample_rate); } - bool send(std::string_view key, uint32_t value, std::string_view type, float sample_rate) override { return _send(key, value, type, sample_rate); } - bool send(std::string_view key, uint64_t value, std::string_view type, float sample_rate) override { return _send(key, value, type, sample_rate); } + bool dec(std::string_view key, float sample_rate) override EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return count(key, -1, sample_rate); + } + bool inc(std::string_view key, float sample_rate) override EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return count(key, 1, sample_rate); + } + bool count(std::string_view key, int64_t delta, float sample_rate) override EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return _send(key, delta, STATSD_METRIC_COUNT, sample_rate); + } + bool gauge(std::string_view key, int64_t value, float sample_rate) override EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return _send(key, value, STATSD_METRIC_GAUGE, sample_rate); + } + bool gaugeDouble(std::string_view key, double value, float sample_rate) override EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return _send(key, value, STATSD_METRIC_GAUGE, sample_rate); + } + bool timing(std::string_view key, uint64_t ms, float sample_rate) override EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return _send(key, ms, STATSD_METRIC_TIMING, sample_rate); + } + + bool send(std::string_view key, double value, std::string_view type, float sample_rate) override + EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return _send(key, value, type, sample_rate); + } + bool send(std::string_view key, int32_t value, std::string_view type, float sample_rate) override + EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return _send(key, value, type, sample_rate); + } + bool send(std::string_view key, int64_t value, std::string_view type, float sample_rate) override + EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return _send(key, value, type, sample_rate); + } + bool send(std::string_view key, uint32_t value, std::string_view type, float sample_rate) override + EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return _send(key, value, type, sample_rate); + } + bool send(std::string_view key, uint64_t value, std::string_view type, float sample_rate) override + EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return _send(key, value, type, sample_rate); + } bool active() const override { return m_sender != nullptr; } private: template - inline bool _send(std::string_view key, T1 value, std::string_view type, float sample_rate); + inline bool _send(std::string_view key, T1 value, std::string_view type, float sample_rate) + EXCLUSIVE_LOCKS_REQUIRED(!cs); private: /* Mutex to protect PRNG */ diff --git a/src/sync.h b/src/sync.h index 6d10e10637ce..8ca945cca76f 100644 --- a/src/sync.h +++ b/src/sync.h @@ -144,6 +144,17 @@ using RecursiveMutex = AnnotatedMixin; /** Wrapped mutex: supports waiting but not recursive locking */ using Mutex = AnnotatedMixin; +/** Different type to mark Mutex at global scope + * + * Thread safety analysis can't handle negative assertions about mutexes + * with global scope well, so mark them with a separate type, and + * eventually move all the mutexes into classes so they are not globally + * visible. + * + * See: https://github.com/bitcoin/bitcoin/pull/20272#issuecomment-720755781 + */ +class GlobalMutex : public Mutex { }; + /** Wrapped shared mutex: supports read locking via .shared_lock, exclusive locking via .lock; * does not support recursive locking */ using SharedMutex = SharedAnnotatedMixin; @@ -152,6 +163,7 @@ using SharedMutex = SharedAnnotatedMixin; inline void AssertLockNotHeldInline(const char* name, const char* file, int line, Mutex* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) { AssertLockNotHeldInternal(name, file, line, cs); } inline void AssertLockNotHeldInline(const char* name, const char* file, int line, RecursiveMutex* cs) LOCKS_EXCLUDED(cs) { AssertLockNotHeldInternal(name, file, line, cs); } +inline void AssertLockNotHeldInline(const char* name, const char* file, int line, GlobalMutex* cs) LOCKS_EXCLUDED(cs) { AssertLockNotHeldInternal(name, file, line, cs); } inline void AssertLockNotHeldInline(const char* name, const char* file, int line, SharedMutex* cs) LOCKS_EXCLUDED(cs) { AssertLockNotHeldInternal(name, file, line, cs); } #define AssertLockNotHeld(cs) AssertLockNotHeldInline(#cs, __FILE__, __LINE__, &cs) @@ -308,14 +320,33 @@ using DebugLock = UniqueLock using ReadLock = SharedLock::type>::type>; -#define LOCK(cs) DebugLock UNIQUE_NAME(criticalblock)(cs, #cs, __FILE__, __LINE__) -#define READ_LOCK(cs) ReadLock UNIQUE_NAME(criticalblock)(cs, #cs, __FILE__, __LINE__) +// When locking a Mutex, require negative capability to ensure the lock +// is not already held +inline Mutex& MaybeCheckNotHeld(Mutex& cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) LOCK_RETURNED(cs) { return cs; } +inline Mutex* MaybeCheckNotHeld(Mutex* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) LOCK_RETURNED(cs) { return cs; } + +// When locking a GlobalMutex, just check it is not locked in the surrounding scope +inline GlobalMutex& MaybeCheckNotHeld(GlobalMutex& cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; } +inline GlobalMutex* MaybeCheckNotHeld(GlobalMutex* cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; } + +// When locking a RecursiveMutex, it's okay to already hold the lock +// but check that it is not known to be locked in the surrounding scope anyway +inline RecursiveMutex& MaybeCheckNotHeld(RecursiveMutex& cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; } +inline RecursiveMutex* MaybeCheckNotHeld(RecursiveMutex* cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; } + +// When locking a SharedMutex, it's okay to already hold the lock +// but check that it is not known to be locked in the surrounding scope anyway +inline SharedMutex& MaybeCheckNotHeld(SharedMutex& cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; } +inline SharedMutex* MaybeCheckNotHeld(SharedMutex* cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; } + +#define LOCK(cs) DebugLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__) +#define READ_LOCK(cs) ReadLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__) #define LOCK2(cs1, cs2) \ - DebugLock criticalblock1(cs1, #cs1, __FILE__, __LINE__); \ - DebugLock criticalblock2(cs2, #cs2, __FILE__, __LINE__) -#define TRY_LOCK(cs, name) DebugLock name(cs, #cs, __FILE__, __LINE__, true) -#define TRY_READ_LOCK(cs, name) ReadLock name(cs, #cs, __FILE__, __LINE__, true) -#define WAIT_LOCK(cs, name) DebugLock name(cs, #cs, __FILE__, __LINE__) + DebugLock criticalblock1(MaybeCheckNotHeld(cs1), #cs1, __FILE__, __LINE__); \ + DebugLock criticalblock2(MaybeCheckNotHeld(cs2), #cs2, __FILE__, __LINE__) +#define TRY_LOCK(cs, name) DebugLock name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__, true) +#define TRY_READ_LOCK(cs, name) ReadLock name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__, true) +#define WAIT_LOCK(cs, name) DebugLock name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__) #define ENTER_CRITICAL_SECTION(cs) \ { \ @@ -354,8 +385,8 @@ using ReadLock = SharedLock decltype(auto) { LOCK(cs); code; }() -#define WITH_READ_LOCK(cs, code) [&]() -> decltype(auto) { READ_LOCK(cs); code; }() +#define WITH_LOCK(cs, code) (MaybeCheckNotHeld(cs), [&]() -> decltype(auto) { LOCK(cs); code; }()) +#define WITH_READ_LOCK(cs, code) (MaybeCheckNotHeld(cs), [&]() -> decltype(auto) { READ_LOCK(cs); code; }()) /** An implementation of a semaphore. * diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index 708c4686e7e6..62eea17e34a0 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -17,7 +17,10 @@ struct Dersig100Setup : public TestChain100Setup { : TestChain100Setup{CBaseChainParams::REGTEST, {"-testactivationheight=dersig@102"}} {} }; -bool CheckInputScripts(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks); +bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, + const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore, + bool cacheFullScriptStore, PrecomputedTransactionData& txdata, + std::vector* pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main); BOOST_AUTO_TEST_SUITE(txvalidationcache_tests) diff --git a/src/timedata.cpp b/src/timedata.cpp index 8debca706562..ceee08e68c4e 100644 --- a/src/timedata.cpp +++ b/src/timedata.cpp @@ -16,7 +16,7 @@ #include #include -static Mutex g_timeoffset_mutex; +static GlobalMutex g_timeoffset_mutex; static int64_t nTimeOffset GUARDED_BY(g_timeoffset_mutex) = 0; /** diff --git a/src/util/system.cpp b/src/util/system.cpp index e92324817c23..efa58bbd22cf 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -97,7 +97,7 @@ const char * const BITCOIN_SETTINGS_FILENAME = "settings.json"; ArgsManager gArgs; /** Mutex to protect dir_locks. */ -static Mutex cs_dir_locks; +static GlobalMutex cs_dir_locks; /** A map that contains all the currently held directory locks. After * successful locking, these will be held here until the global destructor diff --git a/src/validation.cpp b/src/validation.cpp index 63f7109b83f9..5354f8909c88 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -125,7 +125,7 @@ static constexpr int PRUNE_LOCK_BUFFER{10}; */ RecursiveMutex cs_main; -Mutex g_best_block_mutex; +GlobalMutex g_best_block_mutex; std::condition_variable g_best_block_cv; uint256 g_best_block; bool g_parallel_script_checks{false}; @@ -161,7 +161,11 @@ const CBlockIndex* CChainState::FindForkInGlobalIndex(const CBlockLocator& locat return m_chain.Genesis(); } -bool CheckInputScripts(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks = nullptr); +bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, + const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore, + bool cacheFullScriptStore, PrecomputedTransactionData& txdata, + std::vector* pvChecks = nullptr) + EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx) { @@ -1793,7 +1797,10 @@ void InitScriptExecutionCache() { * * Non-static (and re-declared) in src/test/txvalidationcache_tests.cpp */ -bool CheckInputScripts(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, + const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore, + bool cacheFullScriptStore, PrecomputedTransactionData& txdata, + std::vector* pvChecks) { auto start = Now(); if (tx.IsCoinBase()) return true; diff --git a/src/validation.h b/src/validation.h index 5da4e69f203f..31318029cf8e 100644 --- a/src/validation.h +++ b/src/validation.h @@ -111,7 +111,7 @@ enum class SynchronizationState { }; extern RecursiveMutex cs_main; -extern Mutex g_best_block_mutex; +extern GlobalMutex g_best_block_mutex; extern std::condition_variable g_best_block_cv; /** Used to notify getblocktemplate RPC of new tips. */ extern uint256 g_best_block; diff --git a/src/versionbits.h b/src/versionbits.h index 19232ba61977..036fba26b423 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -85,7 +85,7 @@ class VersionBitsCache public: /** Get the numerical statistics for a given deployment for the signalling period that includes the block after pindexPrev. */ - BIP9Stats Statistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); + BIP9Stats Statistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); static uint32_t Mask(const Consensus::Params& params, Consensus::DeploymentPos pos); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e9c09e8186e0..1caf30747a3b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -197,8 +197,8 @@ void NotifyWalletLoaded(WalletContext& context, const std::shared_ptr& } } -static Mutex g_loading_wallet_mutex; -static Mutex g_wallet_release_mutex; +static GlobalMutex g_loading_wallet_mutex; +static GlobalMutex g_wallet_release_mutex; static std::condition_variable g_wallet_release_cv; static std::set g_loading_wallet_set GUARDED_BY(g_loading_wallet_mutex); static std::set g_unloading_wallet_set GUARDED_BY(g_wallet_release_mutex); diff --git a/src/warnings.cpp b/src/warnings.cpp index a4ba98333c0f..85719d2dcdb1 100644 --- a/src/warnings.cpp +++ b/src/warnings.cpp @@ -13,7 +13,7 @@ #include -static Mutex g_warnings_mutex; +static GlobalMutex g_warnings_mutex; static bilingual_str g_misc_warnings GUARDED_BY(g_warnings_mutex); static bool fLargeWorkInvalidChainFound GUARDED_BY(g_warnings_mutex) = false;