From 574cc4271ab09a4c8f8d076cb1a3a2d5b3924b73 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 28 Aug 2021 19:51:08 +0200 Subject: [PATCH 1/4] refactor: remove RecursiveMutex cs_totalBytesRecv, use std::atomic instead The RecursiveMutex cs_totalBytesRecv is only used at two places: in CConnman::RecordBytesRecv() to increment the nTotalBytesRecv member, and in CConnman::GetTotalBytesRecv() to read it. For this simple use-case, we can make the member std::atomic instead to achieve the same result. --- src/net.cpp | 2 -- src/net.h | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 1b6d16ad9b..5406e7e49d 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2879,7 +2879,6 @@ bool CConnman::DisconnectNode(NodeId id) void CConnman::RecordBytesRecv(uint64_t bytes) { - LOCK(cs_totalBytesRecv); nTotalBytesRecv += bytes; } @@ -2956,7 +2955,6 @@ uint64_t CConnman::GetOutboundTargetBytesLeft() const uint64_t CConnman::GetTotalBytesRecv() const { - LOCK(cs_totalBytesRecv); return nTotalBytesRecv; } diff --git a/src/net.h b/src/net.h index bb024f35f0..7092a6524a 100644 --- a/src/net.h +++ b/src/net.h @@ -1074,9 +1074,8 @@ private: static bool NodeFullyConnected(const CNode* pnode); // Network usage totals - mutable RecursiveMutex cs_totalBytesRecv; mutable RecursiveMutex cs_totalBytesSent; - uint64_t nTotalBytesRecv GUARDED_BY(cs_totalBytesRecv) {0}; + std::atomic nTotalBytesRecv{0}; uint64_t nTotalBytesSent GUARDED_BY(cs_totalBytesSent) {0}; // outbound limit & stats From d51d2a3bb5b0011efa1b4f1e2c9512a16ce9b347 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 28 Aug 2021 20:57:52 +0200 Subject: [PATCH 2/4] scripted-diff: rename node vector/mutex members in CConnman -BEGIN VERIFY SCRIPT- ren() { sed -i "s/$1/$2/g" $3 $4 $5; } ren cs_vAddedNodes m_added_nodes_mutex src/net.h src/net.cpp ren vAddedNodes m_added_nodes src/net.h src/net.cpp ren cs_vNodes m_nodes_mutex src/net.h src/net.cpp src/test/util/net.h ren vNodesDisconnectedCopy nodes_disconnected_copy src/net.cpp ren vNodesDisconnected m_nodes_disconnected src/net.h src/net.cpp ren vNodesCopy nodes_copy src/net.cpp ren vNodesSize nodes_size src/net.cpp ren vNodes m_nodes src/net.h src/net.cpp src/test/util/net.h -END VERIFY SCRIPT- --- src/net.cpp | 158 ++++++++++++++++++++++---------------------- src/net.h | 30 ++++----- src/test/util/net.h | 10 +-- 3 files changed, 99 insertions(+), 99 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 5406e7e49d..db496c2185 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -326,8 +326,8 @@ bool IsLocal(const CService& addr) CNode* CConnman::FindNode(const CNetAddr& ip) { - LOCK(cs_vNodes); - for (CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (CNode* pnode : m_nodes) { if (static_cast(pnode->addr) == ip) { return pnode; } @@ -337,8 +337,8 @@ CNode* CConnman::FindNode(const CNetAddr& ip) CNode* CConnman::FindNode(const CSubNet& subNet) { - LOCK(cs_vNodes); - for (CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (CNode* pnode : m_nodes) { if (subNet.Match(static_cast(pnode->addr))) { return pnode; } @@ -348,8 +348,8 @@ CNode* CConnman::FindNode(const CSubNet& subNet) CNode* CConnman::FindNode(const std::string& addrName) { - LOCK(cs_vNodes); - for (CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (CNode* pnode : m_nodes) { if (pnode->m_addr_name == addrName) { return pnode; } @@ -359,8 +359,8 @@ CNode* CConnman::FindNode(const std::string& addrName) CNode* CConnman::FindNode(const CService& addr) { - LOCK(cs_vNodes); - for (CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (CNode* pnode : m_nodes) { if (static_cast(pnode->addr) == addr) { return pnode; } @@ -375,8 +375,8 @@ bool CConnman::AlreadyConnectedToAddress(const CAddress& addr) bool CConnman::CheckIncomingNonce(uint64_t nonce) { - LOCK(cs_vNodes); - for (const CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (const CNode* pnode : m_nodes) { if (!pnode->fSuccessfullyConnected && !pnode->IsInboundConn() && pnode->GetLocalNonce() == nonce) return false; } @@ -435,7 +435,7 @@ 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(cs_vNodes); + LOCK(m_nodes_mutex); CNode* pnode = FindNode(static_cast(addrConnect)); if (pnode) { LogPrintf("Failed to open new connection, already connected\n"); @@ -1056,8 +1056,8 @@ bool CConnman::AttemptToEvictConnection() std::vector vEvictionCandidates; { - LOCK(cs_vNodes); - for (const CNode* node : vNodes) { + LOCK(m_nodes_mutex); + for (const CNode* node : m_nodes) { if (node->HasPermission(NetPermissionFlags::NoBan)) continue; if (!node->IsInboundConn()) @@ -1084,8 +1084,8 @@ bool CConnman::AttemptToEvictConnection() if (!node_id_to_evict) { return false; } - LOCK(cs_vNodes); - for (CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (CNode* pnode : m_nodes) { if (pnode->GetId() == *node_id_to_evict) { LogPrint(BCLog::NET, "selected %s connection for eviction peer=%d; disconnecting\n", pnode->ConnectionTypeAsString(), pnode->GetId()); pnode->fDisconnect = true; @@ -1141,8 +1141,8 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket, } { - LOCK(cs_vNodes); - for (const CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (const CNode* pnode : m_nodes) { if (pnode->IsInboundConn()) nInbound++; } } @@ -1210,8 +1210,8 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket, LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString()); { - LOCK(cs_vNodes); - vNodes.push_back(pnode); + LOCK(m_nodes_mutex); + m_nodes.push_back(pnode); } // We received a new connection, harvest entropy from the time (and our peer count) @@ -1238,8 +1238,8 @@ bool CConnman::AddConnection(const std::string& address, ConnectionType conn_typ } // no default case, so the compiler can warn about missing cases // Count existing connections - int existing_connections = WITH_LOCK(cs_vNodes, - return std::count_if(vNodes.begin(), vNodes.end(), [conn_type](CNode* node) { return node->m_conn_type == conn_type; });); + int existing_connections = WITH_LOCK(m_nodes_mutex, + return std::count_if(m_nodes.begin(), m_nodes.end(), [conn_type](CNode* node) { return node->m_conn_type == conn_type; });); // Max connections of specified type already exist if (max_connections != std::nullopt && existing_connections >= max_connections) return false; @@ -1255,11 +1255,11 @@ bool CConnman::AddConnection(const std::string& address, ConnectionType conn_typ void CConnman::DisconnectNodes() { { - LOCK(cs_vNodes); + LOCK(m_nodes_mutex); if (!fNetworkActive) { // Disconnect any connected nodes - for (CNode* pnode : vNodes) { + for (CNode* pnode : m_nodes) { if (!pnode->fDisconnect) { LogPrint(BCLog::NET, "Network not active, dropping peer=%d\n", pnode->GetId()); pnode->fDisconnect = true; @@ -1268,13 +1268,13 @@ void CConnman::DisconnectNodes() } // Disconnect unused nodes - std::vector vNodesCopy = vNodes; - for (CNode* pnode : vNodesCopy) + std::vector nodes_copy = m_nodes; + for (CNode* pnode : nodes_copy) { if (pnode->fDisconnect) { - // remove from vNodes - vNodes.erase(remove(vNodes.begin(), vNodes.end(), pnode), vNodes.end()); + // remove from m_nodes + m_nodes.erase(remove(m_nodes.begin(), m_nodes.end(), pnode), m_nodes.end()); // release outbound grant (if any) pnode->grantOutbound.Release(); @@ -1284,18 +1284,18 @@ void CConnman::DisconnectNodes() // hold in disconnected pool until all refs are released pnode->Release(); - vNodesDisconnected.push_back(pnode); + m_nodes_disconnected.push_back(pnode); } } } { // Delete disconnected nodes - std::list vNodesDisconnectedCopy = vNodesDisconnected; - for (CNode* pnode : vNodesDisconnectedCopy) + std::list nodes_disconnected_copy = m_nodes_disconnected; + for (CNode* pnode : nodes_disconnected_copy) { // Destroy the object only after other threads have stopped using it. if (pnode->GetRefCount() <= 0) { - vNodesDisconnected.remove(pnode); + m_nodes_disconnected.remove(pnode); DeleteNode(pnode); } } @@ -1304,15 +1304,15 @@ void CConnman::DisconnectNodes() void CConnman::NotifyNumConnectionsChanged() { - size_t vNodesSize; + size_t nodes_size; { - LOCK(cs_vNodes); - vNodesSize = vNodes.size(); + LOCK(m_nodes_mutex); + nodes_size = m_nodes.size(); } - if(vNodesSize != nPrevNodeCount) { - nPrevNodeCount = vNodesSize; + if(nodes_size != nPrevNodeCount) { + nPrevNodeCount = nodes_size; if (m_client_interface) { - m_client_interface->NotifyNumConnectionsChanged(vNodesSize); + m_client_interface->NotifyNumConnectionsChanged(nodes_size); } } } @@ -1716,8 +1716,8 @@ void CConnman::ThreadDNSAddressSeed() int nRelevant = 0; { - LOCK(cs_vNodes); - for (const CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (const CNode* pnode : m_nodes) { if (pnode->fSuccessfullyConnected && pnode->IsFullOutboundConn()) ++nRelevant; } } @@ -1825,8 +1825,8 @@ int CConnman::GetExtraFullOutboundCount() const { int full_outbound_peers = 0; { - LOCK(cs_vNodes); - for (const CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (const CNode* pnode : m_nodes) { if (pnode->fSuccessfullyConnected && !pnode->fDisconnect && pnode->IsFullOutboundConn()) { ++full_outbound_peers; } @@ -1839,8 +1839,8 @@ int CConnman::GetExtraBlockRelayCount() const { int block_relay_peers = 0; { - LOCK(cs_vNodes); - for (const CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (const CNode* pnode : m_nodes) { if (pnode->fSuccessfullyConnected && !pnode->fDisconnect && pnode->IsBlockOnlyConn()) { ++block_relay_peers; } @@ -1911,8 +1911,8 @@ void CConnman::ThreadOpenConnections(const std::vector connect) // Checking !dnsseed is cheaper before locking 2 mutexes. if (!add_fixed_seeds_now && !dnsseed) { - LOCK2(m_addr_fetches_mutex, cs_vAddedNodes); - if (m_addr_fetches.empty() && vAddedNodes.empty()) { + LOCK2(m_addr_fetches_mutex, m_added_nodes_mutex); + if (m_addr_fetches.empty() && m_added_nodes.empty()) { add_fixed_seeds_now = true; LogPrintf("Adding fixed seeds as -dnsseed=0, -addnode is not provided and all -seednode(s) attempted\n"); } @@ -1937,8 +1937,8 @@ void CConnman::ThreadOpenConnections(const std::vector connect) std::set > setConnected; { - LOCK(cs_vNodes); - for (const CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (const CNode* pnode : m_nodes) { if (pnode->IsFullOutboundConn()) nOutboundFullRelay++; if (pnode->IsBlockOnlyConn()) nOutboundBlockRelay++; @@ -2126,8 +2126,8 @@ void CConnman::ThreadOpenConnections(const std::vector connect) std::vector CConnman::GetCurrentBlockRelayOnlyConns() const { std::vector ret; - LOCK(cs_vNodes); - for (const CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (const CNode* pnode : m_nodes) { if (pnode->IsBlockOnlyConn()) { ret.push_back(pnode->addr); } @@ -2142,9 +2142,9 @@ std::vector CConnman::GetAddedNodeInfo() const std::list lAddresses(0); { - LOCK(cs_vAddedNodes); - ret.reserve(vAddedNodes.size()); - std::copy(vAddedNodes.cbegin(), vAddedNodes.cend(), std::back_inserter(lAddresses)); + LOCK(m_added_nodes_mutex); + ret.reserve(m_added_nodes.size()); + std::copy(m_added_nodes.cbegin(), m_added_nodes.cend(), std::back_inserter(lAddresses)); } @@ -2152,8 +2152,8 @@ std::vector CConnman::GetAddedNodeInfo() const std::map mapConnected; std::map> mapConnectedByName; { - LOCK(cs_vNodes); - for (const CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (const CNode* pnode : m_nodes) { if (pnode->addr.IsValid()) { mapConnected[pnode->addr] = pnode->IsInboundConn(); } @@ -2249,8 +2249,8 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai m_msgproc->InitializeNode(pnode); { - LOCK(cs_vNodes); - vNodes.push_back(pnode); + LOCK(m_nodes_mutex); + m_nodes.push_back(pnode); } } @@ -2264,7 +2264,7 @@ void CConnman::ThreadMessageHandler() { // Randomize the order in which we process messages from/to our peers. // This prevents attacks in which an attacker exploits having multiple - // consecutive connections in the vNodes list. + // consecutive connections in the m_nodes list. const NodesSnapshot snap{*this, /*shuffle=*/true}; for (CNode* pnode : snap.Nodes()) { @@ -2694,7 +2694,7 @@ void CConnman::StopNodes() // Delete peer connections. std::vector nodes; - WITH_LOCK(cs_vNodes, nodes.swap(vNodes)); + WITH_LOCK(m_nodes_mutex, nodes.swap(m_nodes)); for (CNode* pnode : nodes) { pnode->CloseSocketDisconnect(); DeleteNode(pnode); @@ -2709,10 +2709,10 @@ void CConnman::StopNodes() } } - for (CNode* pnode : vNodesDisconnected) { + for (CNode* pnode : m_nodes_disconnected) { DeleteNode(pnode); } - vNodesDisconnected.clear(); + m_nodes_disconnected.clear(); vhListenSocket.clear(); semOutbound.reset(); semAddnode.reset(); @@ -2785,21 +2785,21 @@ std::vector CConnman::GetAddresses(CNode& requestor, size_t max_addres bool CConnman::AddNode(const std::string& strNode) { - LOCK(cs_vAddedNodes); - for (const std::string& it : vAddedNodes) { + LOCK(m_added_nodes_mutex); + for (const std::string& it : m_added_nodes) { if (strNode == it) return false; } - vAddedNodes.push_back(strNode); + m_added_nodes.push_back(strNode); return true; } bool CConnman::RemoveAddedNode(const std::string& strNode) { - LOCK(cs_vAddedNodes); - for(std::vector::iterator it = vAddedNodes.begin(); it != vAddedNodes.end(); ++it) { + LOCK(m_added_nodes_mutex); + for(std::vector::iterator it = m_added_nodes.begin(); it != m_added_nodes.end(); ++it) { if (strNode == *it) { - vAddedNodes.erase(it); + m_added_nodes.erase(it); return true; } } @@ -2808,12 +2808,12 @@ bool CConnman::RemoveAddedNode(const std::string& strNode) size_t CConnman::GetNodeCount(ConnectionDirection flags) const { - LOCK(cs_vNodes); + LOCK(m_nodes_mutex); if (flags == ConnectionDirection::Both) // Shortcut if we want total - return vNodes.size(); + return m_nodes.size(); int nNum = 0; - for (const auto& pnode : vNodes) { + for (const auto& pnode : m_nodes) { if (flags & (pnode->IsInboundConn() ? ConnectionDirection::In : ConnectionDirection::Out)) { nNum++; } @@ -2825,9 +2825,9 @@ size_t CConnman::GetNodeCount(ConnectionDirection flags) const void CConnman::GetNodeStats(std::vector& vstats) const { vstats.clear(); - LOCK(cs_vNodes); - vstats.reserve(vNodes.size()); - for (CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + vstats.reserve(m_nodes.size()); + for (CNode* pnode : m_nodes) { vstats.emplace_back(); pnode->CopyStats(vstats.back()); vstats.back().m_mapped_as = pnode->addr.GetMappedAS(addrman.GetAsmap()); @@ -2836,7 +2836,7 @@ void CConnman::GetNodeStats(std::vector& vstats) const bool CConnman::DisconnectNode(const std::string& strNode) { - LOCK(cs_vNodes); + LOCK(m_nodes_mutex); if (CNode* pnode = FindNode(strNode)) { LogPrint(BCLog::NET, "disconnect by address%s matched peer=%d; disconnecting\n", (fLogIPs ? strprintf("=%s", strNode) : ""), pnode->GetId()); pnode->fDisconnect = true; @@ -2848,8 +2848,8 @@ bool CConnman::DisconnectNode(const std::string& strNode) bool CConnman::DisconnectNode(const CSubNet& subnet) { bool disconnected = false; - LOCK(cs_vNodes); - for (CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for (CNode* pnode : m_nodes) { if (subnet.Match(pnode->addr)) { LogPrint(BCLog::NET, "disconnect by subnet%s matched peer=%d; disconnecting\n", (fLogIPs ? strprintf("=%s", subnet.ToString()) : ""), pnode->GetId()); pnode->fDisconnect = true; @@ -2866,8 +2866,8 @@ bool CConnman::DisconnectNode(const CNetAddr& addr) bool CConnman::DisconnectNode(NodeId id) { - LOCK(cs_vNodes); - for(CNode* pnode : vNodes) { + LOCK(m_nodes_mutex); + for(CNode* pnode : m_nodes) { if (id == pnode->GetId()) { LogPrint(BCLog::NET, "disconnect by id peer=%d; disconnecting\n", pnode->GetId()); pnode->fDisconnect = true; @@ -3057,8 +3057,8 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg) bool CConnman::ForNode(NodeId id, std::function func) { CNode* found = nullptr; - LOCK(cs_vNodes); - for (auto&& pnode : vNodes) { + LOCK(m_nodes_mutex); + for (auto&& pnode : m_nodes) { if(pnode->GetId() == id) { found = pnode; break; diff --git a/src/net.h b/src/net.h index 7092a6524a..a10b055a97 100644 --- a/src/net.h +++ b/src/net.h @@ -791,8 +791,8 @@ public: } vWhitelistedRange = connOptions.vWhitelistedRange; { - LOCK(cs_vAddedNodes); - vAddedNodes = connOptions.m_added_nodes; + LOCK(m_added_nodes_mutex); + m_added_nodes = connOptions.m_added_nodes; } m_onion_binds = connOptions.onion_binds; } @@ -823,8 +823,8 @@ public: using NodeFn = std::function; void ForEachNode(const NodeFn& func) { - LOCK(cs_vNodes); - for (auto&& node : vNodes) { + LOCK(m_nodes_mutex); + for (auto&& node : m_nodes) { if (NodeFullyConnected(node)) func(node); } @@ -832,8 +832,8 @@ public: void ForEachNode(const NodeFn& func) const { - LOCK(cs_vNodes); - for (auto&& node : vNodes) { + LOCK(m_nodes_mutex); + for (auto&& node : m_nodes) { if (NodeFullyConnected(node)) func(node); } @@ -968,7 +968,7 @@ private: /** * Create a `CNode` object from a socket that has just been accepted and add the node to - * the `vNodes` member. + * the `m_nodes` member. * @param[in] hSocket Connected socket to communicate with the peer. * @param[in] permissionFlags The peer's permissions. * @param[in] addr_bind The address and port at our side of the connection. @@ -1099,11 +1099,11 @@ private: AddrMan& addrman; std::deque m_addr_fetches GUARDED_BY(m_addr_fetches_mutex); RecursiveMutex m_addr_fetches_mutex; - std::vector vAddedNodes GUARDED_BY(cs_vAddedNodes); - mutable RecursiveMutex cs_vAddedNodes; - std::vector vNodes GUARDED_BY(cs_vNodes); - std::list vNodesDisconnected; - mutable RecursiveMutex cs_vNodes; + std::vector m_added_nodes GUARDED_BY(m_added_nodes_mutex); + mutable RecursiveMutex m_added_nodes_mutex; + std::vector m_nodes GUARDED_BY(m_nodes_mutex); + std::list m_nodes_disconnected; + mutable RecursiveMutex m_nodes_mutex; std::atomic nLastNodeId{0}; unsigned int nPrevNodeCount{0}; @@ -1225,7 +1225,7 @@ private: std::vector m_onion_binds; /** - * RAII helper to atomically create a copy of `vNodes` and add a reference + * RAII helper to atomically create a copy of `m_nodes` and add a reference * to each of the nodes. The nodes are released when this object is destroyed. */ class NodesSnapshot @@ -1234,8 +1234,8 @@ private: explicit NodesSnapshot(const CConnman& connman, bool shuffle) { { - LOCK(connman.cs_vNodes); - m_nodes_copy = connman.vNodes; + LOCK(connman.m_nodes_mutex); + m_nodes_copy = connman.m_nodes; for (auto& node : m_nodes_copy) { node->AddRef(); } diff --git a/src/test/util/net.h b/src/test/util/net.h index d89fc34b75..2de6e712a2 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -25,16 +25,16 @@ struct ConnmanTestMsg : public CConnman { void AddTestNode(CNode& node) { - LOCK(cs_vNodes); - vNodes.push_back(&node); + LOCK(m_nodes_mutex); + m_nodes.push_back(&node); } void ClearTestNodes() { - LOCK(cs_vNodes); - for (CNode* node : vNodes) { + LOCK(m_nodes_mutex); + for (CNode* node : m_nodes) { delete node; } - vNodes.clear(); + m_nodes.clear(); } void ProcessMessagesOnce(CNode& node) { m_msgproc->ProcessMessages(&node, flagInterruptMsgProc); } From 7d52ff5c389643cde613d86fee33d7087f47b8b4 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 28 Aug 2021 19:59:41 +0200 Subject: [PATCH 3/4] refactor: replace RecursiveMutex m_addr_fetches_mutex with Mutex The RecursiveMutex m_addr_fetches_mutex is used at three places: - CConnman::AddAddrFetch() - CConnman::ProcessAddrFetch() - CConnman::ThreadOpenConnections() In each of the critical sections, only the the m_addr_fetches is accessed (and in the last case, also vAddedNodes), without any chance that within one section another one is called. Hence, we can use an ordinary Mutex instead of RecursiveMutex. --- src/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net.h b/src/net.h index a10b055a97..93d36a6e3d 100644 --- a/src/net.h +++ b/src/net.h @@ -1098,7 +1098,7 @@ private: bool fAddressesInitialized{false}; AddrMan& addrman; std::deque m_addr_fetches GUARDED_BY(m_addr_fetches_mutex); - RecursiveMutex m_addr_fetches_mutex; + Mutex m_addr_fetches_mutex; std::vector m_added_nodes GUARDED_BY(m_added_nodes_mutex); mutable RecursiveMutex m_added_nodes_mutex; std::vector m_nodes GUARDED_BY(m_nodes_mutex); From 3726a4595837b66d37f151faf1cec2796d6b74d7 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 28 Aug 2021 21:02:28 +0200 Subject: [PATCH 4/4] refactor: replace RecursiveMutex m_added_nodes_mutex with Mutex The RecursiveMutex m_added_nodes_mutex is used at three places: - CConnman::GetAddedNodeInfo() - CConnman::AddNode() - CConnman::ThreadOpenConnections() In each of the critical sections, only the the m_added_nodes member is accessed (and in the last case, also m_addr_fetches), without any chance that within one section another one is called. Hence, we can use an ordinary Mutex instead of RecursiveMutex. --- src/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net.h b/src/net.h index 93d36a6e3d..dd5cc66a04 100644 --- a/src/net.h +++ b/src/net.h @@ -1100,7 +1100,7 @@ private: std::deque m_addr_fetches GUARDED_BY(m_addr_fetches_mutex); Mutex m_addr_fetches_mutex; std::vector m_added_nodes GUARDED_BY(m_added_nodes_mutex); - mutable RecursiveMutex m_added_nodes_mutex; + 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;