From fb561ef58415865f2311eef69cf20ade3f29b118 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 9 Dec 2024 13:55:33 -0600 Subject: [PATCH] perf: convert m_nodes_mutex to a non-recursive mutex --- src/net.cpp | 4 +- src/net.h | 112 ++++++++++++++++++++++++++-------------------------- 2 files changed, 57 insertions(+), 59 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 7c2804ed5e..55c7a8d826 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -536,7 +536,6 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo } // It is possible that we already have a connection to the IP/port pszDest resolved to. // In that case, drop the connection that was just created. - LOCK(m_nodes_mutex); CNode* pnode = FindNode(static_cast(addrConnect)); if (pnode) { LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort()); @@ -3810,7 +3809,7 @@ void CConnman::ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, auto getConnectToDmn = [&]() -> CDeterministicMNCPtr { // don't hold lock while calling OpenMasternodeConnection as cs_main is locked deep inside - LOCK2(m_nodes_mutex, cs_vPendingMasternodes); + LOCK(cs_vPendingMasternodes); if (!vPendingMasternodes.empty()) { auto dmn = mnList.GetValidMN(vPendingMasternodes.front()); @@ -4797,7 +4796,6 @@ void CConnman::GetNodeStats(std::vector& vstats) const bool CConnman::DisconnectNode(const std::string& strNode) { - LOCK(m_nodes_mutex); if (CNode* pnode = FindNode(strNode)) { LogPrint(BCLog::NET_NETCONN, "disconnect by address%s matched peer=%d; disconnecting\n", (fLogIPs ? strprintf("=%s", strNode) : ""), pnode->GetId()); pnode->fDisconnect = true; diff --git a/src/net.h b/src/net.h index 9085771b53..7d2e112323 100644 --- a/src/net.h +++ b/src/net.h @@ -1263,8 +1263,8 @@ public: 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(!m_nodes_mutex); + void Stop() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { StopThreads(); StopNodes(); @@ -1292,10 +1292,10 @@ public: 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, !m_nodes_mutex); void OpenMasternodeConnection(const CAddress& addrConnect, bool use_v2transport, MasternodeProbeConn probe = MasternodeProbeConn::IsConnection) - EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !mutexMsgProc); - bool CheckIncomingNonce(uint64_t nonce); + EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !mutexMsgProc, !m_nodes_mutex); + bool CheckIncomingNonce(uint64_t nonce) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); // alias for thread safety annotations only, not defined RecursiveMutex& GetNodesMutex() const LOCK_RETURNED(m_nodes_mutex); @@ -1314,37 +1314,37 @@ public: constexpr static const CAllNodes AllNodes{}; - bool ForNode(NodeId id, std::function cond, std::function func); - bool ForNode(const CService& addr, std::function cond, std::function func); + bool ForNode(NodeId id, std::function cond, std::function func) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + bool ForNode(const CService& addr, std::function cond, std::function func) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); template - bool ForNode(const CService& addr, Callable&& func) + bool ForNode(const CService& addr, Callable&& func) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { return ForNode(addr, FullyConnectedOnly, func); } template - bool ForNode(NodeId id, Callable&& func) + bool ForNode(NodeId id, Callable&& func) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { return ForNode(id, FullyConnectedOnly, func); } using NodeFn = std::function; - bool IsConnected(const CService& addr, std::function cond) + bool IsConnected(const CService& addr, std::function cond) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { return ForNode(addr, cond, [](CNode* pnode){ return true; }); } - bool IsMasternodeOrDisconnectRequested(const CService& addr); + bool IsMasternodeOrDisconnectRequested(const CService& addr) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); void PushMessage(CNode* pnode, CSerializedNetMsg&& msg) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !m_total_bytes_sent_mutex); template - bool ForEachNodeContinueIf(const Condition& cond, Callable&& func) + bool ForEachNodeContinueIf(const Condition& cond, Callable&& func) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { LOCK(m_nodes_mutex); for (auto&& node : m_nodes) @@ -1361,7 +1361,7 @@ public: } template - bool ForEachNodeContinueIf(const Condition& cond, Callable&& func) const + bool ForEachNodeContinueIf(const Condition& cond, Callable&& func) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { LOCK(m_nodes_mutex); for (const auto& node : m_nodes) @@ -1378,7 +1378,7 @@ public: } template - void ForEachNode(const Condition& cond, Callable&& func) + void ForEachNode(const Condition& cond, Callable&& func) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { LOCK(m_nodes_mutex); for (auto&& node : m_nodes) { @@ -1387,13 +1387,13 @@ public: } }; - void ForEachNode(const NodeFn& fn) + void ForEachNode(const NodeFn& fn) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { ForEachNode(FullyConnectedOnly, fn); } template - void ForEachNode(const Condition& cond, Callable&& func) const + void ForEachNode(const Condition& cond, Callable&& func) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { LOCK(m_nodes_mutex); for (auto&& node : m_nodes) { @@ -1402,13 +1402,13 @@ public: } }; - void ForEachNode(const NodeFn& fn) const + void ForEachNode(const NodeFn& fn) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { ForEachNode(FullyConnectedOnly, fn); } template - void ForEachNodeThen(const Condition& cond, Callable&& pre, CallableAfter&& post) + void ForEachNodeThen(const Condition& cond, Callable&& pre, CallableAfter&& post) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { LOCK(m_nodes_mutex); for (auto&& node : m_nodes) { @@ -1419,13 +1419,13 @@ public: }; template - void ForEachNodeThen(Callable&& pre, CallableAfter&& post) + void ForEachNodeThen(Callable&& pre, CallableAfter&& post) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { ForEachNodeThen(FullyConnectedOnly, pre, post); } template - void ForEachNodeThen(const Condition& cond, Callable&& pre, CallableAfter&& post) const + void ForEachNodeThen(const Condition& cond, Callable&& pre, CallableAfter&& post) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { LOCK(m_nodes_mutex); for (auto&& node : m_nodes) { @@ -1475,14 +1475,14 @@ public: // return a value less than (num_outbound_connections - num_outbound_slots) // in cases where some outbound connections are not yet fully connected, or // not yet fully disconnected. - int GetExtraFullOutboundCount() const; + int GetExtraFullOutboundCount() const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); // Count the number of block-relay-only peers we have over our limit. - int GetExtraBlockRelayCount() const; + int GetExtraBlockRelayCount() const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); bool AddNode(const AddedNodeParams& add) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); bool RemoveAddedNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); bool AddedNodesContain(const CAddress& addr) const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); - std::vector GetAddedNodeInfo(bool include_connected) const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); + std::vector GetAddedNodeInfo(bool include_connected) const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_nodes_mutex); /** * Attempts to open a connection. Currently only used from tests. @@ -1498,28 +1498,28 @@ public: * - 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, !m_nodes_mutex); bool AddPendingMasternode(const uint256& proTxHash); void SetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash, const std::set& proTxHashes); - void SetMasternodeQuorumRelayMembers(Consensus::LLMQType llmqType, const uint256& quorumHash, const std::set& proTxHashes); + void SetMasternodeQuorumRelayMembers(Consensus::LLMQType llmqType, const uint256& quorumHash, const std::set& proTxHashes) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); bool HasMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const; std::set GetMasternodeQuorums(Consensus::LLMQType llmqType) const; // also returns QWATCH nodes - std::set GetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const; + std::set GetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); void RemoveMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash); bool IsMasternodeQuorumNode(const CNode* pnode, const CDeterministicMNList& tip_mn_list) const; bool IsMasternodeQuorumRelayMember(const uint256& protxHash); void AddPendingProbeConnections(const std::set& proTxHashes); - size_t GetNodeCount(ConnectionDirection) const; + size_t GetNodeCount(ConnectionDirection) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); size_t GetMaxOutboundNodeCount(); size_t GetMaxOutboundOnionNodeCount(); - void GetNodeStats(std::vector& vstats) const; - bool DisconnectNode(const std::string& node); - bool DisconnectNode(const CSubNet& subnet); - bool DisconnectNode(const CNetAddr& addr); - bool DisconnectNode(NodeId id); + void GetNodeStats(std::vector& vstats) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + bool DisconnectNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + bool DisconnectNode(const CSubNet& subnet) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + bool DisconnectNode(const CNetAddr& addr) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + bool DisconnectNode(NodeId id) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); //! Used to convey which local services we are offering peers during node //! connection. @@ -1564,7 +1564,7 @@ public: { public: explicit NodesSnapshot(const CConnman& connman, std::function cond = AllNodes, - bool shuffle = false); + bool shuffle = false) EXCLUSIVE_LOCKS_REQUIRED(!connman.m_nodes_mutex); ~NodesSnapshot(); const std::vector& Nodes() const @@ -1599,16 +1599,16 @@ private: 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, !m_nodes_mutex); 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, !m_nodes_mutex); 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); - void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); - void ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !m_nodes_mutex); + void ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !m_nodes_mutex); void AcceptConnection(const ListenSocket& hListenSocket, CMasternodeSync& mn_sync) - EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !m_nodes_mutex); /** * Create a `CNode` object from a socket that has just been accepted and add the node to @@ -1622,11 +1622,11 @@ private: NetPermissionFlags permission_flags, const CAddress& addr_bind, const CAddress& addr, - CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !m_nodes_mutex); void DisconnectNodes() EXCLUSIVE_LOCKS_REQUIRED(!m_reconnections_mutex, !m_nodes_mutex); - void NotifyNumConnectionsChanged(CMasternodeSync& mn_sync); - void CalculateNumConnectionsChangedStats(); + void NotifyNumConnectionsChanged(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + void CalculateNumConnectionsChangedStats() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); /** Return true if the peer is inactive and should be disconnected. */ bool InactivityCheck(const CNode& node) const; @@ -1686,7 +1686,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, !mutexMsgProc); + void SocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex); /** * Do the read/write for connected sockets that are ready for IO. @@ -1697,14 +1697,14 @@ 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, !mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex); /** * 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) - EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !m_nodes_mutex); void ThreadSocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex, !m_reconnections_mutex); @@ -1715,19 +1715,19 @@ private: uint64_t CalculateKeyedNetGroup(const CAddress& ad) const; - CNode* FindNode(const CNetAddr& ip, bool fExcludeDisconnecting = true); - CNode* FindNode(const CSubNet& subNet, bool fExcludeDisconnecting = true); - CNode* FindNode(const std::string& addrName, bool fExcludeDisconnecting = true); - CNode* FindNode(const CService& addr, bool fExcludeDisconnecting = true); + CNode* FindNode(const CNetAddr& ip, bool fExcludeDisconnecting = true) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + CNode* FindNode(const CSubNet& subNet, bool fExcludeDisconnecting = true) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + CNode* FindNode(const std::string& addrName, bool fExcludeDisconnecting = true) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + CNode* FindNode(const CService& addr, bool fExcludeDisconnecting = true) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); /** * Determine whether we're already connected to a given address, in order to * avoid initiating duplicate connections. */ - bool AlreadyConnectedToAddress(const CAddress& addr); + bool AlreadyConnectedToAddress(const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); - bool AttemptToEvictConnection(); - CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); + bool AttemptToEvictConnection() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !m_nodes_mutex); void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const; void DeleteNode(CNode* pnode); @@ -1737,7 +1737,7 @@ private: /** (Try to) send data from node's vSendMsg. Returns (bytes_sent, data_left). */ std::pair SocketSendData(CNode& node) const EXCLUSIVE_LOCKS_REQUIRED(node.cs_vSend); - size_t SocketRecvData(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); + size_t SocketRecvData(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !m_nodes_mutex); void DumpAddresses(); @@ -1754,7 +1754,7 @@ private: /** * Return vector of current BLOCK_RELAY peers. */ - std::vector GetCurrentBlockRelayOnlyConns() const; + std::vector GetCurrentBlockRelayOnlyConns() const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); /** * Search for a "preferred" network, a reachable network to which we @@ -1766,7 +1766,7 @@ private: * * @return bool Whether a preferred network was found. */ - bool MaybePickPreferredNetwork(std::optional& network); + bool MaybePickPreferredNetwork(std::optional& network) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); // Whether the node should be passed out in ForEach* callbacks static bool NodeFullyConnected(const CNode* pnode); @@ -1806,7 +1806,7 @@ private: mutable Mutex m_added_nodes_mutex; std::vector m_nodes GUARDED_BY(m_nodes_mutex); std::list m_nodes_disconnected; - mutable RecursiveMutex m_nodes_mutex; + mutable Mutex m_nodes_mutex; std::atomic nLastNodeId{0}; unsigned int nPrevNodeCount{0}; @@ -1993,7 +1993,7 @@ private: 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, !m_nodes_mutex); /** * Cap on the size of `m_unused_i2p_sessions`, to ensure it does not