From 5dde8e7b33d91c8d285e261494fa213e12f669b7 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 5 May 2024 07:21:48 +0000 Subject: [PATCH] merge bitcoin#25109: Strengthen AssertLockNotHeld assertions --- src/checkqueue.h | 10 ++--- src/coinjoin/coinjoin.h | 17 +++++--- src/httpserver.cpp | 6 +-- src/i2p.h | 6 +-- src/index/blockfilterindex.h | 2 +- src/net.cpp | 1 - src/net.h | 75 +++++++++++++++++++++--------------- src/net_processing.cpp | 74 ++++++++++++++++++++--------------- src/qt/clientmodel.h | 2 +- src/random.cpp | 6 +-- src/scheduler.h | 29 +++++++------- src/sync.h | 14 +++++-- src/threadinterrupt.h | 2 +- src/validationinterface.cpp | 8 ++-- src/versionbits.h | 8 ++-- 15 files changed, 148 insertions(+), 112 deletions(-) diff --git a/src/checkqueue.h b/src/checkqueue.h index 68357c2177..e94103f416 100644 --- a/src/checkqueue.h +++ b/src/checkqueue.h @@ -65,7 +65,7 @@ private: bool m_request_stop GUARDED_BY(m_mutex){false}; /** Internal function that does bulk of the verification work. */ - bool Loop(bool fMaster) + bool Loop(bool fMaster) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { std::condition_variable& cond = fMaster ? m_master_cv : m_worker_cv; std::vector vChecks; @@ -139,7 +139,7 @@ public: } //! Create a pool of new worker threads. - void StartWorkerThreads(const int threads_num) + void StartWorkerThreads(const int threads_num) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { { LOCK(m_mutex); @@ -157,13 +157,13 @@ public: } //! Wait until execution finishes, and return whether all evaluations were successful. - bool Wait() + bool Wait() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { return Loop(true /* master thread */); } //! Add a batch of checks to the queue - void Add(std::vector& vChecks) + void Add(std::vector& vChecks) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { if (vChecks.empty()) { return; @@ -186,7 +186,7 @@ public: } //! Stop all of the worker threads. - void StopWorkerThreads() + void StopWorkerThreads() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { WITH_LOCK(m_mutex, m_request_stop = true); m_worker_cv.notify_all(); diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index bb42917d32..06dd934a83 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -368,15 +368,22 @@ public: void AddDSTX(const CCoinJoinBroadcastTx& dstx) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx); CCoinJoinBroadcastTx GetDSTX(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx); - void UpdatedBlockTip(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler, const CMasternodeSync& mn_sync); - void NotifyChainLock(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler, const CMasternodeSync& mn_sync); + void UpdatedBlockTip(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler, + const CMasternodeSync& mn_sync) + EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx); + void NotifyChainLock(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler, + const CMasternodeSync& mn_sync) + EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx); void TransactionAddedToMempool(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx); - void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx); - void BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex*) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx); + void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex) + EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx); + void BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex*) + EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx); private: - void CheckDSTXes(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler); + void CheckDSTXes(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler) + EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx); void UpdateDSTXConfirmedHeight(const CTransactionRef& tx, std::optional nHeight); }; diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 2572df0cf3..a749a02699 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -83,7 +83,7 @@ public: { } /** Enqueue a work item */ - bool Enqueue(WorkItem* item) + bool Enqueue(WorkItem* item) EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); if (!running || queue.size() >= maxDepth) { @@ -94,7 +94,7 @@ public: return true; } /** Thread function */ - void Run() + void Run() EXCLUSIVE_LOCKS_REQUIRED(!cs) { while (true) { std::unique_ptr i; @@ -111,7 +111,7 @@ public: } } /** Interrupt and exit loops */ - void Interrupt() + void Interrupt() EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); running = false; diff --git a/src/i2p.h b/src/i2p.h index cb2efedba8..3899dbf81f 100644 --- a/src/i2p.h +++ b/src/i2p.h @@ -84,7 +84,7 @@ public: * to the listening socket and address. * @return true on success */ - bool Listen(Connection& conn); + bool Listen(Connection& conn) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** * Wait for and accept a new incoming connection. @@ -103,7 +103,7 @@ public: * it is set to `false`. Only set if `false` is returned. * @return true on success */ - bool Connect(const CService& to, Connection& conn, bool& proxy_error); + bool Connect(const CService& to, Connection& conn, bool& proxy_error) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); private: /** @@ -172,7 +172,7 @@ private: /** * Check the control socket for errors and possibly disconnect. */ - void CheckControlSock(); + void CheckControlSock() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** * Generate a new destination with the SAM proxy and set `m_private_key` to it. diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index 96c4936f8d..760a712349 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -63,7 +63,7 @@ public: bool LookupFilter(const CBlockIndex* block_index, BlockFilter& filter_out) const; /** Get a single filter header by block. */ - bool LookupFilterHeader(const CBlockIndex* block_index, uint256& header_out); + bool LookupFilterHeader(const CBlockIndex* block_index, uint256& header_out) EXCLUSIVE_LOCKS_REQUIRED(!m_cs_headers_cache); /** Get a range of filters between two heights on a chain. */ bool LookupFilterRange(int start_height, const CBlockIndex* stop_index, diff --git a/src/net.cpp b/src/net.cpp index cdd20cafb7..90957d5f53 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2986,7 +2986,6 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai } void CConnman::OpenMasternodeConnection(const CAddress &addrConnect, MasternodeProbeConn probe) { - OpenNetworkConnection(addrConnect, false, nullptr, nullptr, ConnectionType::OUTBOUND_FULL_RELAY, MasternodeConn::IsConnection, probe); } diff --git a/src/net.h b/src/net.h index ef2def3869..a8b6505099 100644 --- a/src/net.h +++ b/src/net.h @@ -652,7 +652,7 @@ public: * @return True if the peer should stay connected, * False if the peer should be disconnected from. */ - bool ReceiveMsgBytes(Span msg_bytes, bool& complete); + bool ReceiveMsgBytes(Span msg_bytes, bool& complete) EXCLUSIVE_LOCKS_REQUIRED(!cs_vRecv); void SetCommonVersion(int greatest_common_version) { @@ -664,9 +664,9 @@ public: return m_greatest_common_version; } - CService GetAddrLocal() const LOCKS_EXCLUDED(m_addr_local_mutex); + CService GetAddrLocal() const EXCLUSIVE_LOCKS_REQUIRED(!m_addr_local_mutex); //! May not be called more than once - void SetAddrLocal(const CService& addrLocalIn) LOCKS_EXCLUDED(m_addr_local_mutex); + void SetAddrLocal(const CService& addrLocalIn) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_local_mutex); CNode* AddRef() { @@ -679,9 +679,9 @@ public: nRefCount--; } - void CloseSocketDisconnect(CConnman* connman); + void CloseSocketDisconnect(CConnman* connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_hSocket); - void copyStats(CNodeStats &stats, const std::vector &m_asmap); + void copyStats(CNodeStats &stats, const std::vector &m_asmap) EXCLUSIVE_LOCKS_REQUIRED(!m_subver_mutex, !m_addr_local_mutex, !cs_vSend, !cs_vRecv); ServiceFlags GetLocalServices() const { @@ -857,7 +857,7 @@ public: bool m_i2p_accept_incoming; }; - void Init(const Options& connOptions) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex) + void Init(const Options& connOptions) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_total_bytes_sent_mutex) { AssertLockNotHeld(m_total_bytes_sent_mutex); @@ -891,7 +891,8 @@ public: CConnman(uint64_t seed0, uint64_t seed1, CAddrMan& addrman, bool network_active = true); ~CConnman(); bool Start(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync, - CScheduler& scheduler, const Options& options) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); + CScheduler& scheduler, const Options& options) + EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !m_added_nodes_mutex, !m_addr_fetches_mutex, !mutexMsgProc); void StopThreads(); void StopNodes(); @@ -901,7 +902,7 @@ public: StopNodes(); }; - void Interrupt(); + void Interrupt() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; void SetNetworkActive(bool active, CMasternodeSync* const mn_sync); @@ -917,8 +918,13 @@ public: IsConnection, }; - void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound, const char* strDest, ConnectionType conn_type, MasternodeConn masternode_connection = MasternodeConn::IsNotConnection, MasternodeProbeConn masternode_probe_connection = MasternodeProbeConn::IsNotConnection); - void OpenMasternodeConnection(const CAddress& addrConnect, MasternodeProbeConn probe = MasternodeProbeConn::IsConnection); + void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound, + const char* strDest, ConnectionType conn_type, + MasternodeConn masternode_connection = MasternodeConn::IsNotConnection, + MasternodeProbeConn masternode_probe_connection = MasternodeProbeConn::IsNotConnection) + EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + void OpenMasternodeConnection(const CAddress& addrConnect, MasternodeProbeConn probe = MasternodeProbeConn::IsConnection) + EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); bool CheckIncomingNonce(uint64_t nonce); struct CFullyConnectedOnly { @@ -959,7 +965,8 @@ public: bool IsMasternodeOrDisconnectRequested(const CService& addr); - void PushMessage(CNode* pnode, CSerializedNetMsg&& msg) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); + void PushMessage(CNode* pnode, CSerializedNetMsg&& msg) + EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !m_total_bytes_sent_mutex); template bool ForEachNodeContinueIf(const Condition& cond, Callable&& func) @@ -1099,9 +1106,9 @@ public: // Count the number of block-relay-only peers we have over our limit. int GetExtraBlockRelayCount() const; - bool AddNode(const std::string& node); - bool RemoveAddedNode(const std::string& node); - std::vector GetAddedNodeInfo() const; + bool AddNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); + bool RemoveAddedNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); + std::vector GetAddedNodeInfo() const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); /** * Attempts to open a connection. Currently only used from tests. @@ -1114,7 +1121,7 @@ public: * - Max total outbound connection capacity filled * - Max connection capacity for type is filled */ - bool AddConnection(const std::string& address, ConnectionType conn_type); + bool AddConnection(const std::string& address, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); bool AddPendingMasternode(const uint256& proTxHash); void SetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash, const std::set& proTxHashes); @@ -1166,8 +1173,8 @@ public: unsigned int GetReceiveFloodSize() const; - void WakeMessageHandler(); - void WakeSelect(); + void WakeMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + void WakeSelect() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); /** Attempts to obfuscate tx time through exponentially distributed emitting. Works assuming that a single interval is used. @@ -1221,13 +1228,15 @@ private: const std::vector& whiteBinds, const std::vector& onion_binds); - void ThreadOpenAddedConnections(); - void AddAddrFetch(const std::string& strDest); - void ProcessAddrFetch(); - void ThreadOpenConnections(const std::vector connect, CDeterministicMNManager& dmnman); - void ThreadMessageHandler(); - void ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync); - void AcceptConnection(const ListenSocket& hListenSocket, CMasternodeSync& mn_sync); + void ThreadOpenAddedConnections() EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !mutexMsgProc); + void AddAddrFetch(const std::string& strDest) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex); + void ProcessAddrFetch() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !mutexMsgProc); + void ThreadOpenConnections(const std::vector connect, CDeterministicMNManager& dmnman) + EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !mutexMsgProc); + void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + void ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + void AcceptConnection(const ListenSocket& hListenSocket, CMasternodeSync& mn_sync) + EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); /** * Create a `CNode` object from a socket that has just been accepted and add the node to @@ -1241,7 +1250,7 @@ private: NetPermissionFlags permissionFlags, const CAddress& addr_bind, const CAddress& addr, - CMasternodeSync& mn_sync); + CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); void DisconnectNodes(); void NotifyNumConnectionsChanged(CMasternodeSync& mn_sync); @@ -1305,7 +1314,7 @@ private: /** * 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); + void SocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc); /** * Do the read/write for connected sockets that are ready for IO. @@ -1316,18 +1325,20 @@ private: void SocketHandlerConnected(const std::set& recv_set, const std::set& send_set, const std::set& error_set) - EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc); /** * Accept incoming connections, one from each read-ready listening socket. * @param[in] recv_set Sockets that are ready for read. */ - void SocketHandlerListening(const std::set& recv_set, CMasternodeSync& mn_sync); + void SocketHandlerListening(const std::set& recv_set, CMasternodeSync& mn_sync) + EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); - void ThreadSocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); - void ThreadDNSAddressSeed(); + void ThreadSocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc); + void ThreadDNSAddressSeed() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex); void ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, - CMasternodeSync& mn_sync); + CMasternodeSync& mn_sync) + EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex, !mutexMsgProc); uint64_t CalculateKeyedNetGroup(const CAddress& ad) const; @@ -1351,7 +1362,7 @@ private: NodeId GetNewNodeId(); size_t SocketSendData(CNode& node) EXCLUSIVE_LOCKS_REQUIRED(node.cs_vSend); - size_t SocketRecvData(CNode* pnode); + size_t SocketRecvData(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); void DumpAddresses(); // Network stats diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 72ef8b3ea2..3a33be97c7 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -374,38 +374,45 @@ public: bool ignore_incoming_txs); /** Overridden from CValidationInterface. */ - void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected) override; - void BlockDisconnected(const std::shared_ptr &block, const CBlockIndex* pindex) override; - void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; - void BlockChecked(const CBlock& block, const BlockValidationState& state) override; + void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected) override + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex); + void BlockDisconnected(const std::shared_ptr &block, const CBlockIndex* pindex) override + EXCLUSIVE_LOCKS_REQUIRED(!m_recent_confirmed_transactions_mutex); + void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override + 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; /** Implement NetEventsInterface */ - void InitializeNode(CNode* pnode) override; - void FinalizeNode(const CNode& node) override; - bool ProcessMessages(CNode* pfrom, std::atomic& interrupt) override; - bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing); + void InitializeNode(CNode* pnode) 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); + bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex); /** Implement PeerManager */ void CheckForStaleTipAndEvictPeers() override; - bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override; + bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool IgnoresIncomingTxs() override { return m_ignore_incoming_txs; } - void SendPings() override; - void PushInventory(NodeId nodeid, const CInv& inv) override; - void RelayInv(CInv &inv, const int minProtoVersion) override; - void RelayInvFiltered(CInv &inv, const CTransaction &relatedTx, const int minProtoVersion) override; - void RelayInvFiltered(CInv &inv, const uint256 &relatedTxHash, const int minProtoVersion) override; - void RelayTransaction(const uint256& txid) override; + void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);; + void PushInventory(NodeId nodeid, const CInv& inv) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void RelayInv(CInv &inv, const int minProtoVersion) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void RelayInvFiltered(CInv &inv, const CTransaction &relatedTx, const int minProtoVersion) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void RelayInvFiltered(CInv &inv, const uint256 &relatedTxHash, const int minProtoVersion) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void RelayTransaction(const uint256& txid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void SetBestHeight(int height) override { m_best_height = height; }; - void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message = "") override; + 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; - bool IsBanned(NodeId pnode) override EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool IsInvInFilter(NodeId nodeid, const uint256& hash) const override; + const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) override + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex); + bool IsBanned(NodeId pnode) override EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_peer_mutex); + bool IsInvInFilter(NodeId nodeid, const uint256& hash) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); private: /** Helper to process result of external handlers of message */ - void ProcessPeerMsgRet(const PeerMsgRet& ret, CNode& pfrom); + void ProcessPeerMsgRet(const PeerMsgRet& ret, CNode& pfrom) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */ void ConsiderEviction(CNode& pto, int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -414,15 +421,15 @@ private: void EvictExtraOutboundPeers(int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Retrieve unbroadcast transactions from the mempool and reattempt sending to peers */ - void ReattemptInitialBroadcast(CScheduler& scheduler); + void ReattemptInitialBroadcast(CScheduler& scheduler) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Get a shared pointer to the Peer object. * May return an empty shared_ptr if the Peer object can't be found. */ - PeerRef GetPeerRef(NodeId id) const; + PeerRef GetPeerRef(NodeId id) const EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Get a shared pointer to the Peer object and remove it from m_peer_map. * May return an empty shared_ptr if the Peer object can't be found. */ - PeerRef RemovePeer(NodeId id); + PeerRef RemovePeer(NodeId id) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** * Potentially mark a node discouraged based on the contents of a BlockValidationState object @@ -435,7 +442,8 @@ private: * @return Returns true if the peer was punished (probably disconnected) */ bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& state, - bool via_compact_block, const std::string& message = ""); + bool via_compact_block, const std::string& message = "") + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** * Potentially ban a node based on the contents of a TxValidationState object @@ -444,7 +452,8 @@ private: * * Changes here may need to be reflected in TxRelayMayResultInDisconnect(). */ - bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, const std::string& message = ""); + bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, const std::string& message = "") + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Maybe disconnect a peer and discourage future connections from its address. * @@ -454,14 +463,16 @@ private: */ bool MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer); - void ProcessOrphanTx(std::set& orphan_work_set) - EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans); + void ProcessOrphanTx(std::set& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Process a single headers message from a peer. */ void ProcessHeadersMessage(CNode& pfrom, const Peer& peer, const std::vector& headers, - bool via_compact_block); + bool via_compact_block) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); - void SendBlockTransactions(CNode& pfrom, const CBlock& block, const BlockTransactionsRequest& req); + void SendBlockTransactions(CNode& pfrom, const CBlock& block, const BlockTransactionsRequest& req) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Send a version message to a peer */ void PushNodeVersion(CNode& pnode, const Peer& peer); @@ -482,7 +493,7 @@ private: * @param[in] fReachable Whether the address' network is reachable. We relay unreachable * addresses less. */ - void RelayAddress(NodeId originator, const CAddress& addr, bool fReachable); + void RelayAddress(NodeId originator, const CAddress& addr, bool fReachable) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); const CChainParams& m_chainparams; CConnman& m_connman; @@ -608,7 +619,8 @@ private: /** Number of outbound peers with m_chain_sync.m_protect. */ int m_outbound_peers_with_protect_from_disconnect GUARDED_BY(cs_main) = 0; - bool AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool AlreadyHave(const CInv& inv) + EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_recent_confirmed_transactions_mutex); /** * Filter for transactions that were recently rejected by diff --git a/src/qt/clientmodel.h b/src/qt/clientmodel.h index f328ed7519..46068ca264 100644 --- a/src/qt/clientmodel.h +++ b/src/qt/clientmodel.h @@ -63,7 +63,7 @@ public: //! Return number of connections, default is in- and outbound (total) int getNumConnections(unsigned int flags = CONNECTIONS_ALL) const; int getNumBlocks() const; - uint256 getBestBlockHash(); + uint256 getBestBlockHash() EXCLUSIVE_LOCKS_REQUIRED(!m_cached_tip_mutex); int getHeaderTipHeight() const; int64_t getHeaderTipTime() const; diff --git a/src/random.cpp b/src/random.cpp index 0c6129ea25..92ea4976e1 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -377,7 +377,7 @@ public: { } - void AddEvent(uint32_t event_info) noexcept + void AddEvent(uint32_t event_info) noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_events_mutex) { LOCK(m_events_mutex); @@ -391,7 +391,7 @@ public: /** * Feed (the hash of) all events added through AddEvent() to hasher. */ - void SeedEvents(CSHA512& hasher) noexcept + void SeedEvents(CSHA512& hasher) noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_events_mutex) { // We use only SHA256 for the events hashing to get the ASM speedups we have for SHA256, // since we want it to be fast as network peers may be able to trigger it repeatedly. @@ -410,7 +410,7 @@ public: * * If this function has never been called with strong_seed = true, false is returned. */ - bool MixExtract(unsigned char* out, size_t num, CSHA512&& hasher, bool strong_seed) noexcept + bool MixExtract(unsigned char* out, size_t num, CSHA512&& hasher, bool strong_seed) noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { assert(num <= 32); unsigned char buf[64]; diff --git a/src/scheduler.h b/src/scheduler.h index c2396da742..438fb8c126 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -46,10 +46,10 @@ public: typedef std::function Function; /** Call func at/after time t */ - void schedule(Function f, std::chrono::system_clock::time_point t); + void schedule(Function f, std::chrono::system_clock::time_point t) EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); /** Call f once after the delta has passed */ - void scheduleFromNow(Function f, std::chrono::milliseconds delta) + void scheduleFromNow(Function f, std::chrono::milliseconds delta) EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex) { schedule(std::move(f), std::chrono::system_clock::now() + delta); } @@ -60,29 +60,29 @@ public: * The timing is not exact: Every time f is finished, it is rescheduled to run again after delta. If you need more * accurate scheduling, don't use this method. */ - void scheduleEvery(Function f, std::chrono::milliseconds delta); + void scheduleEvery(Function f, std::chrono::milliseconds delta) EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); /** * Mock the scheduler to fast forward in time. * Iterates through items on taskQueue and reschedules them * to be delta_seconds sooner. */ - void MockForward(std::chrono::seconds delta_seconds); + void MockForward(std::chrono::seconds delta_seconds) EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); /** * Services the queue 'forever'. Should be run in a thread. */ - void serviceQueue(); + void serviceQueue() EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); /** Tell any threads running serviceQueue to stop as soon as the current task is done */ - void stop() + void stop() EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex) { WITH_LOCK(newTaskMutex, stopRequested = true); newTaskScheduled.notify_all(); if (m_service_thread.joinable()) m_service_thread.join(); } /** Tell any threads running serviceQueue to stop when there is no work left to be done */ - void StopWhenDrained() + void StopWhenDrained() EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex) { WITH_LOCK(newTaskMutex, stopWhenEmpty = true); newTaskScheduled.notify_all(); @@ -94,10 +94,11 @@ public: * and first and last task times */ size_t getQueueInfo(std::chrono::system_clock::time_point& first, - std::chrono::system_clock::time_point& last) const; + std::chrono::system_clock::time_point& last) const + EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); /** Returns true if there are threads actively running in serviceQueue() */ - bool AreThreadsServicingQueue() const; + bool AreThreadsServicingQueue() const EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); private: mutable Mutex newTaskMutex; @@ -128,8 +129,8 @@ private: std::list> m_callbacks_pending GUARDED_BY(m_callbacks_mutex); bool m_are_callbacks_running GUARDED_BY(m_callbacks_mutex) = false; - void MaybeScheduleProcessQueue(); - void ProcessQueue(); + void MaybeScheduleProcessQueue() EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex); + void ProcessQueue() EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex); public: explicit SingleThreadedSchedulerClient(CScheduler& scheduler LIFETIMEBOUND) : m_scheduler{scheduler} {} @@ -140,15 +141,15 @@ public: * Practically, this means that callbacks can behave as if they are executed * in order by a single thread. */ - void AddToProcessQueue(std::function func); + void AddToProcessQueue(std::function func) EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex); /** * Processes all remaining queue members on the calling thread, blocking until queue is empty * Must be called after the CScheduler has no remaining processing threads! */ - void EmptyQueue(); + void EmptyQueue() EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex); - size_t CallbacksPending(); + size_t CallbacksPending() EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex); }; #endif diff --git a/src/sync.h b/src/sync.h index 2a046a78d8..c75b6554cf 100644 --- a/src/sync.h +++ b/src/sync.h @@ -77,8 +77,6 @@ inline void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, inline void DeleteLock(void* cs) {} inline bool LockStackEmpty() { return true; } #endif -#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) -#define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs) /** * Template mixin that adds -Wthread-safety locking annotations and lock order @@ -138,10 +136,18 @@ public: using RecursiveMutex = AnnotatedMixin; /** Wrapped mutex: supports waiting but not recursive locking */ -typedef AnnotatedMixin Mutex; +using Mutex = AnnotatedMixin; + /** Wrapped shared mutex: supports read locking via .shared_lock, exlusive locking via .lock; * does not support recursive locking */ -typedef SharedAnnotatedMixin SharedMutex; +using SharedMutex = SharedAnnotatedMixin; + +#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) + +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, SharedMutex* cs) LOCKS_EXCLUDED(cs) { AssertLockNotHeldInternal(name, file, line, cs); } +#define AssertLockNotHeld(cs) AssertLockNotHeldInline(#cs, __FILE__, __LINE__, &cs) /** Prints a lock contention to the log */ void LockContention(const char* pszName, const char* pszFile, int nLine); diff --git a/src/threadinterrupt.h b/src/threadinterrupt.h index c60ed3b336..c053c01499 100644 --- a/src/threadinterrupt.h +++ b/src/threadinterrupt.h @@ -22,7 +22,7 @@ public: using Clock = std::chrono::steady_clock; CThreadInterrupt(); explicit operator bool() const; - void operator()(); + void operator()() EXCLUSIVE_LOCKS_REQUIRED(!mut); void reset(); bool sleep_for(Clock::duration rel_time) EXCLUSIVE_LOCKS_REQUIRED(!mut); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index cbee7d7cb8..ac8b19dcdc 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -47,7 +47,7 @@ public: explicit MainSignalsInstance(CScheduler& scheduler LIFETIMEBOUND) : m_schedulerClient(scheduler) {} - void Register(std::shared_ptr callbacks) + void Register(std::shared_ptr callbacks) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { LOCK(m_mutex); auto inserted = m_map.emplace(callbacks.get(), m_list.end()); @@ -55,7 +55,7 @@ public: inserted.first->second->callbacks = std::move(callbacks); } - void Unregister(CValidationInterface* callbacks) + void Unregister(CValidationInterface* callbacks) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { LOCK(m_mutex); auto it = m_map.find(callbacks); @@ -69,7 +69,7 @@ public: //! map entry. After this call, the list may still contain callbacks that //! are currently executing, but it will be cleared when they are done //! executing. - void Clear() + void Clear() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { LOCK(m_mutex); for (const auto& entry : m_map) { @@ -78,7 +78,7 @@ public: m_map.clear(); } - template void Iterate(F&& f) + template void Iterate(F&& f) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { WAIT_LOCK(m_mutex, lock); for (auto it = m_list.begin(); it != m_list.end();) { diff --git a/src/versionbits.h b/src/versionbits.h index c02769809e..a02fc8cce5 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -90,16 +90,16 @@ public: static uint32_t Mask(const Consensus::Params& params, Consensus::DeploymentPos pos); /** Get the BIP9 state for a given deployment for the block after pindexPrev. */ - ThresholdState State(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); + ThresholdState State(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Get the block height at which the BIP9 deployment switched into the state for the block after pindexPrev. */ - int StateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); + int StateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Determine what nVersion a new block should use */ - int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params); + int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); - void Clear(); + void Clear() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); }; class AbstractEHFManager