diff --git a/src/net.cpp b/src/net.cpp index 7035045eb5483..ea4f08246e13a 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -338,56 +338,15 @@ bool IsLocal(const CService& addr) return mapLocalHost.count(addr) > 0; } -CNode* CConnman::FindNode(const CNetAddr& ip, bool fExcludeDisconnecting) +bool CConnman::AlreadyConnectedToAddress(const CAddress& addr) const { - LOCK(m_nodes_mutex); - for (CNode* pnode : m_nodes) { - if (fExcludeDisconnecting && pnode->fDisconnect) { - continue; - } - if (static_cast(pnode->addr) == ip) { - return pnode; - } - } - return nullptr; + READ_LOCK(m_nodes_mutex); + return FindNode(static_cast(addr)) != nullptr; } -CNode* CConnman::FindNode(const std::string& addrName, bool fExcludeDisconnecting) +bool CConnman::CheckIncomingNonce(uint64_t nonce) const { - LOCK(m_nodes_mutex); - for (CNode* pnode : m_nodes) { - if (fExcludeDisconnecting && pnode->fDisconnect) { - continue; - } - if (pnode->m_addr_name == addrName) { - return pnode; - } - } - return nullptr; -} - -CNode* CConnman::FindNode(const CService& addr, bool fExcludeDisconnecting) -{ - LOCK(m_nodes_mutex); - for (CNode* pnode : m_nodes) { - if (fExcludeDisconnecting && pnode->fDisconnect) { - continue; - } - if (static_cast(pnode->addr) == addr) { - return pnode; - } - } - return nullptr; -} - -bool CConnman::AlreadyConnectedToAddress(const CAddress& addr) -{ - return FindNode(addr.ToStringAddrPort()); -} - -bool CConnman::CheckIncomingNonce(uint64_t nonce) -{ - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); for (const CNode* pnode : m_nodes) { if (!pnode->fSuccessfullyConnected && !pnode->IsInboundConn() && pnode->GetLocalNonce() == nonce) return false; @@ -420,9 +379,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo } // Look for an existing connection - CNode* pnode = FindNode(static_cast(addrConnect)); - if (pnode) - { + if (ExistsNode(static_cast(addrConnect))) { LogPrintf("Failed to open new connection, already connected\n"); return nullptr; } @@ -457,9 +414,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(m_nodes_mutex); - CNode* pnode = FindNode(static_cast(addrConnect)); - if (pnode) { + if (ExistsNode(static_cast(addrConnect))) { LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort()); return nullptr; } @@ -637,7 +592,7 @@ bool CNode::IsConnectedThroughPrivacyNet() const #undef X #define X(name) stats.name = name -void CNode::CopyStats(CNodeStats& stats) +void CNode::CopyStats(CNodeStats& stats) const { stats.nodeid = this->GetId(); X(addr); @@ -1783,6 +1738,56 @@ std::pair CConnman::SocketSendData(CNode& node) const return {nSentSize, data_left}; } +std::vector CConnman::GetEvictionCandidates() const +{ + std::vector vEvictionCandidates; + READ_LOCK(m_nodes_mutex); + + for (const CNode* node : m_nodes) { + if (node->fDisconnect) + continue; + + if (m_active_masternode) { + // This handles eviction protected nodes. Nodes are always protected for a short time after the connection + // was accepted. This short time is meant for the VERSION/VERACK exchange and the possible MNAUTH that might + // follow when the incoming connection is from another masternode. When a message other than MNAUTH + // is received after VERSION/VERACK, the protection is lifted immediately. + bool isProtected = GetTime() - node->m_connected < INBOUND_EVICTION_PROTECTION_TIME; + if (node->nTimeFirstMessageReceived.load() != 0s && !node->fFirstMessageIsMNAUTH) { + isProtected = false; + } + // if MNAUTH was valid, the node is always protected (and at the same time not accounted when + // checking incoming connection limits) + if (!node->GetVerifiedProRegTxHash().IsNull()) { + isProtected = true; + } + if (isProtected) { + continue; + } + } + + NodeEvictionCandidate candidate{ + .id = node->GetId(), + .m_connected = node->m_connected, + .m_min_ping_time = node->m_min_ping_time, + .m_last_block_time = node->m_last_block_time, + .m_last_tx_time = node->m_last_tx_time, + .fRelevantServices = node->m_has_all_wanted_services, + .m_relay_txs = node->m_relays_txs.load(), + .fBloomFilter = node->m_bloom_filter_loaded.load(), + .nKeyedNetGroup = node->nKeyedNetGroup, + .prefer_evict = node->m_prefer_evict, + .m_is_local = node->addr.IsLocal(), + .m_network = node->ConnectedThroughNetwork(), + .m_noban = node->HasPermission(NetPermissionFlags::NoBan), + .m_conn_type = node->m_conn_type, + }; + vEvictionCandidates.push_back(candidate); + } + + return vEvictionCandidates; +} + /** Try to find a connection to evict when the node is full. * Extreme care must be taken to avoid opening the node to attacker * triggered network partitioning. @@ -1793,65 +1798,16 @@ std::pair CConnman::SocketSendData(CNode& node) const */ bool CConnman::AttemptToEvictConnection() { - std::vector vEvictionCandidates; - { - LOCK(m_nodes_mutex); - - for (const CNode* node : m_nodes) { - if (node->fDisconnect) - continue; - - if (m_active_masternode) { - // This handles eviction protected nodes. Nodes are always protected for a short time after the connection - // was accepted. This short time is meant for the VERSION/VERACK exchange and the possible MNAUTH that might - // follow when the incoming connection is from another masternode. When a message other than MNAUTH - // is received after VERSION/VERACK, the protection is lifted immediately. - bool isProtected = GetTime() - node->m_connected < INBOUND_EVICTION_PROTECTION_TIME; - if (node->nTimeFirstMessageReceived.load() != 0s && !node->fFirstMessageIsMNAUTH) { - isProtected = false; - } - // if MNAUTH was valid, the node is always protected (and at the same time not accounted when - // checking incoming connection limits) - if (!node->GetVerifiedProRegTxHash().IsNull()) { - isProtected = true; - } - if (isProtected) { - continue; - } - } - - NodeEvictionCandidate candidate{ - .id = node->GetId(), - .m_connected = node->m_connected, - .m_min_ping_time = node->m_min_ping_time, - .m_last_block_time = node->m_last_block_time, - .m_last_tx_time = node->m_last_tx_time, - .fRelevantServices = node->m_has_all_wanted_services, - .m_relay_txs = node->m_relays_txs.load(), - .fBloomFilter = node->m_bloom_filter_loaded.load(), - .nKeyedNetGroup = node->nKeyedNetGroup, - .prefer_evict = node->m_prefer_evict, - .m_is_local = node->addr.IsLocal(), - .m_network = node->ConnectedThroughNetwork(), - .m_noban = node->HasPermission(NetPermissionFlags::NoBan), - .m_conn_type = node->m_conn_type, - }; - vEvictionCandidates.push_back(candidate); - } - } + std::vector vEvictionCandidates = GetEvictionCandidates(); const std::optional node_id_to_evict = SelectNodeToEvict(std::move(vEvictionCandidates)); if (!node_id_to_evict) { return false; } - LOCK(m_nodes_mutex); - for (CNode* pnode : m_nodes) { - if (pnode->GetId() == *node_id_to_evict) { - LogPrint(BCLog::NET_NETCONN, "selected %s connection for eviction peer=%d; disconnecting\n", pnode->ConnectionTypeAsString(), pnode->GetId()); - pnode->fDisconnect = true; - return true; - } - } - return false; + return WithNodeMutable(*node_id_to_evict, [](CNode* pnode){ + LogPrint(BCLog::NET_NETCONN, "selected %s connection for eviction peer=%d; disconnecting\n", pnode->ConnectionTypeAsString(), pnode->GetId()); + pnode->fDisconnect = true; + return true; + }).value_or(false); } void CConnman::AcceptConnection(const ListenSocket& hListenSocket, CMasternodeSync& mn_sync) { @@ -1901,8 +1857,9 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, NetPermissions::AddFlag(permission_flags, NetPermissionFlags::NoBan); } + { - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); for (const CNode* pnode : m_nodes) { if (pnode->IsInboundConn()) { nInbound++; @@ -1911,7 +1868,6 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, } } } - } std::string strDropped; @@ -2060,7 +2016,7 @@ 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(m_nodes_mutex, + int existing_connections = WITH_READ_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 @@ -2201,11 +2157,7 @@ void CConnman::DisconnectNodes() void CConnman::NotifyNumConnectionsChanged(CMasternodeSync& mn_sync) { - size_t nodes_size; - { - LOCK(m_nodes_mutex); - nodes_size = m_nodes.size(); - } + size_t nodes_size = WITH_READ_LOCK(m_nodes_mutex, return m_nodes.size();); // If we had zero connections before and new connections now or if we just dropped // to zero connections reset the sync process if its outdated. @@ -2373,7 +2325,8 @@ void CConnman::SocketHandler(CMasternodeSync& mn_sync) bool only_poll = [this]() { // Check if we have work to do and thus should avoid waiting for events - LOCK2(m_nodes_mutex, cs_sendable_receivable_nodes); + READ_LOCK(m_nodes_mutex); // We acquire this to avoid the pointers stored in mapSendableNodes and mapReceivableNodes being invalidated by ThreadSocketHandler + LOCK(cs_sendable_receivable_nodes); if (!mapReceivableNodes.empty()) { return true; } @@ -2584,7 +2537,7 @@ size_t CConnman::SocketRecvData(CNode *pnode) { bool notify = false; if (!pnode->ReceiveMsgBytes(Span(pchBuf, nBytes), notify)) { - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); // is this here for lock ordering? pnode->CloseSocketDisconnect(this); } RecordBytesRecv(nBytes); @@ -2599,7 +2552,7 @@ size_t CConnman::SocketRecvData(CNode *pnode) if (!pnode->fDisconnect) { LogPrint(BCLog::NET, "socket closed for peer=%d\n", pnode->GetId()); } - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); // is this here for lock ordering? pnode->fOtherSideDisconnected = true; // avoid lingering pnode->CloseSocketDisconnect(this); } @@ -2612,7 +2565,7 @@ size_t CConnman::SocketRecvData(CNode *pnode) if (!pnode->fDisconnect){ LogPrint(BCLog::NET, "socket recv error for peer=%d: %s\n", pnode->GetId(), NetworkErrorString(nErr)); } - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); // is this here for lock ordering? pnode->fOtherSideDisconnected = true; // avoid lingering pnode->CloseSocketDisconnect(this); } @@ -2703,7 +2656,7 @@ void CConnman::ThreadDNSAddressSeed() int nRelevant = 0; { - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); for (const CNode* pnode : m_nodes) { if (pnode->fSuccessfullyConnected && !pnode->IsFullOutboundConn() && !pnode->m_masternode_probe_connection) ++nRelevant; } @@ -2828,7 +2781,7 @@ int CConnman::GetExtraFullOutboundCount() const { int full_outbound_peers = 0; { - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); for (const CNode* pnode : m_nodes) { // don't count outbound masternodes if (pnode->m_masternode_connection) { @@ -2846,7 +2799,7 @@ int CConnman::GetExtraBlockRelayCount() const { int block_relay_peers = 0; { - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); for (const CNode* pnode : m_nodes) { if (pnode->fSuccessfullyConnected && !pnode->fDisconnect && pnode->IsBlockOnlyConn()) { ++block_relay_peers; @@ -2871,7 +2824,7 @@ std::unordered_set CConnman::GetReachableEmptyNetworks() const bool CConnman::MultipleManualOrFullOutboundConns(Network net) const { - AssertLockHeld(m_nodes_mutex); + AssertSharedLockHeld(m_nodes_mutex); return m_network_conn_counts[net] > 1; } @@ -2880,7 +2833,7 @@ bool CConnman::MaybePickPreferredNetwork(std::optional& network) std::array nets{NET_IPV4, NET_IPV6, NET_ONION, NET_I2P, NET_CJDNS}; Shuffle(nets.begin(), nets.end(), FastRandomContext()); - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); for (const auto net : nets) { if (g_reachable_nets.Contains(net) && m_network_conn_counts[net] == 0 && addrman.Size(net) != 0) { network = net; @@ -3003,7 +2956,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe std::set> outbound_ipv46_peer_netgroups; if (!Params().AllowMultipleAddressesFromGroup()) { - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); for (const CNode* pnode : m_nodes) { if (pnode->IsFullOutboundConn() && !pnode->m_masternode_connection) nOutboundFullRelay++; if (pnode->IsBlockOnlyConn()) nOutboundBlockRelay++; @@ -3042,8 +2995,8 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe std::set setConnectedMasternodes; { - LOCK(m_nodes_mutex); - for (CNode* pnode : m_nodes) { + READ_LOCK(m_nodes_mutex); + for (const CNode* pnode : m_nodes) { auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash(); if (!verifiedProRegTxHash.IsNull()) { setConnectedMasternodes.emplace(verifiedProRegTxHash); @@ -3272,7 +3225,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe std::vector CConnman::GetCurrentBlockRelayOnlyConns() const { std::vector ret; - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); for (const CNode* pnode : m_nodes) { if (pnode->IsBlockRelayOnly()) { ret.push_back(pnode->addr); @@ -3298,7 +3251,7 @@ std::vector CConnman::GetAddedNodeInfo(bool include_connected) co std::map mapConnected; std::map> mapConnectedByName; { - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); for (const CNode* pnode : m_nodes) { if (pnode->addr.IsValid()) { mapConnected[pnode->addr] = pnode->IsInboundConn(); @@ -3415,40 +3368,47 @@ void CConnman::ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, MasternodeProbeConn isProbe = MasternodeProbeConn::IsNotConnection; - const auto getPendingQuorumNodes = [&]() EXCLUSIVE_LOCKS_REQUIRED(cs_vPendingMasternodes) { + const auto getPendingQuorumNodes = [&]() SHARED_LOCKS_REQUIRED(m_nodes_mutex) EXCLUSIVE_LOCKS_REQUIRED(cs_vPendingMasternodes) { + AssertSharedLockHeld(m_nodes_mutex); AssertLockHeld(cs_vPendingMasternodes); std::vector ret; for (const auto& group : masternodeQuorumNodes) { for (const auto& proRegTxHash : group.second) { + if (connectedProRegTxHashes.count(proRegTxHash)) { + continue; + } auto dmn = mnList.GetMN(proRegTxHash); if (!dmn) { continue; } const auto addr2 = dmn->pdmnState->netInfo->GetPrimary(); - if (connectedNodes.count(addr2) && !connectedProRegTxHashes.count(proRegTxHash)) { + CNode* pnode = FindNodeMutable(addr2); + if (pnode && pnode->m_masternode_connection) { + // node is masternode, skip it + continue; + } + if (connectedNodes.count(addr2)) { // we probably connected to it before it became a masternode // or maybe we are still waiting for mnauth - (void)ForNode(addr2, [&](CNode* pnode) { - if (pnode->nTimeFirstMessageReceived.load() != 0s && GetTime() - pnode->nTimeFirstMessageReceived.load() > 5s) { - // clearly not expecting mnauth to take that long even if it wasn't the first message - // we received (as it should normally), disconnect - LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- dropping non-mnauth connection to %s, service=%s\n", _func_, proRegTxHash.ToString(), addr2.ToStringAddrPort()); - pnode->fDisconnect = true; - return true; - } - return false; - }); + bool slow_handshake = pnode && pnode->nTimeFirstMessageReceived.load() != 0s && + GetTime() - pnode->nTimeFirstMessageReceived.load() > 5s; + if (slow_handshake) { + // clearly not expecting mnauth to take that long even if it wasn't the first message + // we received (as it should normally), disconnect + LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- dropping non-mnauth connection to %s, service=%s\n", + _func_, proRegTxHash.ToString(), addr2.ToStringAddrPort()); + pnode->fDisconnect = true; + } // either way - it's not ready, skip it for now continue; } - if (!connectedNodes.count(addr2) && !IsMasternodeOrDisconnectRequested(addr2) && !connectedProRegTxHashes.count(proRegTxHash)) { - int64_t lastAttempt = mn_metaman.GetMetaInfo(dmn->proTxHash)->GetLastOutboundAttempt(); - // back off trying connecting to an address if we already tried recently - if (nANow - lastAttempt < chainParams.LLMQConnectionRetryTimeout()) { - continue; - } - ret.emplace_back(dmn); + // back off connecting to an address if we already tried recently + int64_t last_attempt = mn_metaman.GetMetaInfo(dmn->proTxHash)->GetLastOutboundAttempt(); + if (nANow - last_attempt < chainParams.LLMQConnectionRetryTimeout()) { + continue; } + // all checks passed + ret.emplace_back(dmn); } } return ret; @@ -3485,14 +3445,23 @@ 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); + READ_LOCK(m_nodes_mutex); + LOCK(cs_vPendingMasternodes); if (!vPendingMasternodes.empty()) { auto dmn = mnList.GetValidMN(vPendingMasternodes.front()); vPendingMasternodes.erase(vPendingMasternodes.begin()); - if (dmn && !connectedNodes.count(dmn->pdmnState->netInfo->GetPrimary()) && !IsMasternodeOrDisconnectRequested(dmn->pdmnState->netInfo->GetPrimary())) { - LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- opening pending masternode connection to %s, service=%s\n", _func_, dmn->proTxHash.ToString(), dmn->pdmnState->netInfo->GetPrimary().ToStringAddrPort()); - return dmn; + // Check if we should connect to this masternode + // We already hold m_nodes_mutex here, so check m_masternode_connection directly + if (dmn && !connectedNodes.count(dmn->pdmnState->netInfo->GetPrimary())) { + if (const CNode* pnode = FindNode(dmn->pdmnState->netInfo->GetPrimary())) { + if (!pnode->m_masternode_connection) { + LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- opening pending masternode connection to %s, service=%s\n", + _func_, dmn->proTxHash.ToString(), + dmn->pdmnState->netInfo->GetPrimary().ToStringAddrPort()); + return dmn; + } + } } } @@ -3528,7 +3497,7 @@ void CConnman::ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, OpenMasternodeConnection(CAddress(connectToDmn->pdmnState->netInfo->GetPrimary(), NODE_NETWORK), /*use_v2transport=*/GetLocalServices() & NODE_P2P_V2, isProbe); // should be in the list now if connection was opened - bool connected = ForNode(connectToDmn->pdmnState->netInfo->GetPrimary(), CConnman::AllNodes, [&](CNode* pnode) { + bool connected = ForNode(connectToDmn->pdmnState->netInfo->GetPrimary(), CConnman::AllNodes, [&](const CNode* pnode) { if (pnode->fDisconnect) { return false; } @@ -3578,8 +3547,9 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai if (addrConnect.GetPort() == GetListenPort() && IsLocal(addrConnect)) { return; } - } else if (FindNode(std::string(pszDest))) + } else if (ExistsNode(std::string(pszDest))) { return; + } LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- connecting to %s\n", __func__, getIpStr()); CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, conn_type, use_v2transport); @@ -4350,7 +4320,8 @@ Uint256HashSet CConnman::GetMasternodeQuorums(Consensus::LLMQType llmqType) cons std::vector CConnman::GetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const { - LOCK2(m_nodes_mutex, cs_vPendingMasternodes); + READ_LOCK(m_nodes_mutex); + LOCK(cs_vPendingMasternodes); auto it = masternodeQuorumNodes.find(std::make_pair(llmqType, quorumHash)); if (it == masternodeQuorumNodes.end()) { return {}; @@ -4431,7 +4402,7 @@ void CConnman::AddPendingProbeConnections(const std::set &proTxHashes) size_t CConnman::GetNodeCount(ConnectionDirection flags) const { - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); int nNum = 0; for (const auto& pnode : m_nodes) { @@ -4470,9 +4441,9 @@ size_t CConnman::GetMaxOutboundOnionNodeCount() void CConnman::GetNodeStats(std::vector& vstats) const { vstats.clear(); - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); vstats.reserve(m_nodes.size()); - for (CNode* pnode : m_nodes) { + for (const CNode* pnode : m_nodes) { if (pnode->fDisconnect) { continue; } @@ -4484,19 +4455,17 @@ void CConnman::GetNodeStats(std::vector& vstats) const bool CConnman::DisconnectNode(const std::string& strNode) { - LOCK(m_nodes_mutex); - if (CNode* pnode = FindNode(strNode)) { + return WithNodeMutable(strNode, [&](CNode* pnode){ LogPrint(BCLog::NET_NETCONN, "disconnect by address%s matched peer=%d; disconnecting\n", (fLogIPs ? strprintf("=%s", strNode) : ""), pnode->GetId()); pnode->fDisconnect = true; return true; - } - return false; + }).value_or(false); } bool CConnman::DisconnectNode(const CSubNet& subnet) { bool disconnected = false; - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); for (CNode* pnode : m_nodes) { if (subnet.Match(pnode->addr)) { LogPrint(BCLog::NET_NETCONN, "disconnect by subnet%s matched peer=%d; disconnecting\n", (fLogIPs ? strprintf("=%s", subnet.ToString()) : ""), pnode->GetId()); @@ -4514,15 +4483,11 @@ bool CConnman::DisconnectNode(const CNetAddr& addr) bool CConnman::DisconnectNode(NodeId id) { - LOCK(m_nodes_mutex); - for(CNode* pnode : m_nodes) { - if (id == pnode->GetId()) { - LogPrint(BCLog::NET_NETCONN, "disconnect by id peer=%d; disconnecting\n", pnode->GetId()); - pnode->fDisconnect = true; - return true; - } - } - return false; + return WithNodeMutable(id, [&](CNode* pnode){ + LogPrint(BCLog::NET_NETCONN, "disconnect by id peer=%d; disconnecting\n", pnode->GetId()); + pnode->fDisconnect = true; + return true; + }).value_or(false); } void CConnman::RecordBytesRecv(uint64_t bytes) @@ -4772,32 +4737,35 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg) bool CConnman::ForNode(const CService& addr, std::function cond, std::function func) { - CNode* found = nullptr; - LOCK(m_nodes_mutex); - for (auto&& pnode : m_nodes) { - if((CService)pnode->addr == addr) { - found = pnode; - break; - } - } + READ_LOCK(m_nodes_mutex); + CNode* found = FindNodeMutable(addr, false); + return found != nullptr && cond(found) && func(found); +} + +bool CConnman::ForNode(const CService& addr, std::function cond, std::function func) const +{ + READ_LOCK(m_nodes_mutex); + const CNode* found = FindNode(addr, false); return found != nullptr && cond(found) && func(found); } bool CConnman::ForNode(NodeId id, std::function cond, std::function func) { - CNode* found = nullptr; - LOCK(m_nodes_mutex); - for (auto&& pnode : m_nodes) { - if(pnode->GetId() == id) { - found = pnode; - break; - } - } + READ_LOCK(m_nodes_mutex); + CNode* found = FindNodeMutable(id, false); return found != nullptr && cond(found) && func(found); } -bool CConnman::IsMasternodeOrDisconnectRequested(const CService& addr) { - return ForNode(addr, AllNodes, [](CNode* pnode){ +bool CConnman::ForNode(NodeId id, std::function cond, std::function func) const +{ + READ_LOCK(m_nodes_mutex); + const CNode* found = FindNode(id, false); + return found != nullptr && cond(found) && func(found); +} + +bool CConnman::IsMasternodeOrDisconnectRequested(const CService& addr) const +{ + return ForNode(addr, AllNodes, [](const CNode* pnode){ return pnode->m_masternode_connection || pnode->fDisconnect; }); } @@ -4806,7 +4774,7 @@ CConnman::NodesSnapshot::NodesSnapshot(const CConnman& connman, std::function #include #include +#include #include #include @@ -61,6 +62,7 @@ class CMasternodeSync; class CNode; class CScheduler; struct bilingual_str; +struct NodeEvictionCandidate; /** Default for -whitelistrelay. */ static const bool DEFAULT_WHITELISTRELAY = true; @@ -730,9 +732,9 @@ class CNode /** Messages still to be fed to m_transport->SetMessageToSend. */ std::deque vSendMsg GUARDED_BY(cs_vSend); std::atomic nSendMsgSize{0}; - Mutex cs_vSend; + mutable Mutex cs_vSend; Mutex m_sock_mutex; - Mutex cs_vRecv; + mutable Mutex cs_vRecv; uint64_t nRecvBytes GUARDED_BY(cs_vRecv){0}; @@ -755,7 +757,7 @@ class CNode const bool m_inbound_onion; std::atomic nNumWarningsSkipped{0}; std::atomic nVersion{0}; - Mutex m_subver_mutex; + mutable Mutex m_subver_mutex; /** * cleanSubVer is a sanitized string of the user agent byte array we read * from the wire. This cleaned string can safely be logged or displayed. @@ -1027,7 +1029,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, !cs_mnauth); + void CopyStats(CNodeStats& stats) const 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); } @@ -1244,8 +1246,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() EXCLUSIVE_LOCKS_REQUIRED(!cs_mapSocketToNode, !cs_sendable_receivable_nodes); - void Stop() EXCLUSIVE_LOCKS_REQUIRED(!cs_mapSocketToNode, !cs_sendable_receivable_nodes) + void StopNodes() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !cs_mapSocketToNode, !cs_sendable_receivable_nodes); + void Stop() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !cs_mapSocketToNode, !cs_sendable_receivable_nodes) { StopThreads(); StopNodes(); @@ -1274,13 +1276,13 @@ 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, !cs_mapSocketToNode); + EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !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, !cs_mapSocketToNode); - bool CheckIncomingNonce(uint64_t nonce); + EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !m_unused_i2p_sessions_mutex, !mutexMsgProc, !cs_mapSocketToNode); + bool CheckIncomingNonce(uint64_t nonce) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); // alias for thread safety annotations only, not defined - RecursiveMutex& GetNodesMutex() const LOCK_RETURNED(m_nodes_mutex); + SharedMutex& GetNodesMutex() const LOCK_RETURNED(m_nodes_mutex); struct CFullyConnectedOnly { bool operator() (const CNode* pnode) const { @@ -1296,39 +1298,41 @@ friend class CNode; 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(NodeId id, std::function cond, std::function func) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + bool ForNode(const CService& addr, std::function cond, std::function func) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + bool ForNode(const CService& addr, std::function cond, std::function func) const 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 ForNode(addr, cond, [](const CNode* pnode){ return true; }); } - bool IsMasternodeOrDisconnectRequested(const CService& addr); + bool IsMasternodeOrDisconnectRequested(const CService& addr) const 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); + READ_LOCK(m_nodes_mutex); for (auto&& node : m_nodes) if (cond(node)) if(!func(node)) @@ -1337,15 +1341,15 @@ friend class CNode; }; template - bool ForEachNodeContinueIf(Callable&& func) + bool ForEachNodeContinueIf(Callable&& func) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { return ForEachNodeContinueIf(FullyConnectedOnly, func); } 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); + READ_LOCK(m_nodes_mutex); for (const auto& node : m_nodes) if (cond(node)) if(!func(node)) @@ -1354,62 +1358,41 @@ friend class CNode; }; template - bool ForEachNodeContinueIf(Callable&& func) const + bool ForEachNodeContinueIf(Callable&& func) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { return ForEachNodeContinueIf(FullyConnectedOnly, func); } - template - void ForEachNode(const Condition& cond, Callable&& func) - { - LOCK(m_nodes_mutex); - for (auto&& node : m_nodes) { - if (cond(node)) - func(node); - } - }; - - 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); + READ_LOCK(m_nodes_mutex); for (auto&& node : m_nodes) { if (cond(node)) func(node); } }; - 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) - { - LOCK(m_nodes_mutex); - for (auto&& node : m_nodes) { - if (cond(node)) - pre(node); - } - post(); - }; - 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); + READ_LOCK(m_nodes_mutex); for (auto&& node : m_nodes) { if (cond(node)) pre(node); @@ -1418,7 +1401,7 @@ friend class CNode; }; template - void ForEachNodeThen(Callable&& pre, CallableAfter&& post) const + void ForEachNodeThen(Callable&& pre, CallableAfter&& post) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) { ForEachNodeThen(FullyConnectedOnly, pre, post); } @@ -1454,14 +1437,15 @@ friend class CNode; // 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. @@ -1477,29 +1461,29 @@ 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, !cs_mapSocketToNode); + EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !m_unused_i2p_sessions_mutex, !mutexMsgProc, !cs_mapSocketToNode); bool AddPendingMasternode(const uint256& proTxHash); void SetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash, const Uint256HashSet& proTxHashes); - void SetMasternodeQuorumRelayMembers(Consensus::LLMQType llmqType, const uint256& quorumHash, const Uint256HashSet& proTxHashes); + void SetMasternodeQuorumRelayMembers(Consensus::LLMQType llmqType, const uint256& quorumHash, const Uint256HashSet& proTxHashes) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); bool HasMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const; Uint256HashSet GetMasternodeQuorums(Consensus::LLMQType llmqType) const; // also returns QWATCH nodes - std::vector GetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const; + std::vector 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); std::map getNetLocalAddresses() const; 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. @@ -1534,7 +1518,7 @@ friend class CNode; /** Return true if we should disconnect the peer for failing an inactivity check. */ bool ShouldRunInactivityChecks(const CNode& node, std::chrono::seconds now) const; - bool MultipleManualOrFullOutboundConns(Network net) const EXCLUSIVE_LOCKS_REQUIRED(m_nodes_mutex); + bool MultipleManualOrFullOutboundConns(Network net) const SHARED_LOCKS_REQUIRED(m_nodes_mutex); /** * RAII helper to atomically create a copy of `m_nodes` and add a reference @@ -1544,7 +1528,8 @@ friend class CNode; { 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 @@ -1579,16 +1564,19 @@ 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, !cs_mapSocketToNode); + EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_nodes_mutex, !m_reconnections_mutex, + !m_unused_i2p_sessions_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, !cs_mapSocketToNode); + EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_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, !cs_mapSocketToNode); - void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); - void ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !cs_mapSocketToNode); + EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !m_reconnections_mutex, + !m_unused_i2p_sessions_mutex, !mutexMsgProc, !cs_mapSocketToNode); + void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !mutexMsgProc); + void ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !mutexMsgProc, !cs_mapSocketToNode); void AcceptConnection(const ListenSocket& hListenSocket, CMasternodeSync& mn_sync) - EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !cs_mapSocketToNode); + EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !mutexMsgProc, !cs_mapSocketToNode); /** * Create a `CNode` object from a socket that has just been accepted and add the node to @@ -1602,11 +1590,12 @@ friend class CNode; NetPermissionFlags permission_flags, const CAddress& addr_bind, const CAddress& addr, - CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !cs_mapSocketToNode); + CMasternodeSync& mn_sync) + EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !mutexMsgProc, !cs_mapSocketToNode); 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; @@ -1620,21 +1609,21 @@ 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, !cs_mapSocketToNode, !cs_sendable_receivable_nodes); + void SocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !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, !cs_sendable_receivable_nodes, !cs_mapSocketToNode, !cs_sendable_receivable_nodes); + EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !m_total_bytes_sent_mutex, !mutexMsgProc, !cs_sendable_receivable_nodes, !cs_mapSocketToNode); /** * 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, !cs_mapSocketToNode); + EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !mutexMsgProc, !cs_mapSocketToNode); void ThreadSocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex, !m_reconnections_mutex, !cs_mapSocketToNode, !cs_sendable_receivable_nodes); @@ -1645,18 +1634,73 @@ friend class CNode; uint64_t CalculateKeyedNetGroup(const CAddress& ad) const; - CNode* FindNode(const CNetAddr& ip, bool fExcludeDisconnecting = true); - CNode* FindNode(const std::string& addrName, bool fExcludeDisconnecting = true); - CNode* FindNode(const CService& addr, bool fExcludeDisconnecting = true); + // Type-agnostic node matching helpers + static inline bool NodeMatches(const CNode* p, const CService& addr) + { + return static_cast(p->addr) == addr; + } + static inline bool NodeMatches(const CNode* p, const CNetAddr& ip) + { + return static_cast(p->addr) == ip; + } + static inline bool NodeMatches(const CNode* p, const std::string& addrName) + { + return p->m_addr_name == addrName; + } + static inline bool NodeMatches(const CNode* p, const NodeId id) + { + return p->GetId() == id; + } + + template + const CNode* FindNode(const Key& key, bool fExcludeDisconnecting = true) const SHARED_LOCKS_REQUIRED(m_nodes_mutex) + { + AssertSharedLockHeld(m_nodes_mutex); + for (const CNode* pnode : m_nodes) { + if (fExcludeDisconnecting && pnode->fDisconnect) continue; + if (NodeMatches(pnode, key)) return pnode; + } + return nullptr; + } + + template + CNode* FindNodeMutable(const Key& key, bool fExcludeDisconnecting = true) SHARED_LOCKS_REQUIRED(m_nodes_mutex) + { + AssertSharedLockHeld(m_nodes_mutex); + for (CNode* pnode : m_nodes) { + if (fExcludeDisconnecting && pnode->fDisconnect) continue; + if (NodeMatches(pnode, key)) return pnode; + } + return nullptr; + } + + // Callback helpers with explicit lock semantics (templated on key type) + // Lambda-based shared accessor returning optional result (nullopt = not found) + template + std::optional> WithNodeMutable(const Key& key, Callable&& fn) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) + { + READ_LOCK(m_nodes_mutex); + if (CNode* p = FindNodeMutable(key)) return std::optional>{fn(p)}; + return std::nullopt; + } + + // Fast existence check under shared lock + template + bool ExistsNode(const Key& key) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex) + { + READ_LOCK(m_nodes_mutex); + return FindNode(key) != nullptr; + } /** * 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) const 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); + std::vector GetEvictionCandidates() const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_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_nodes_mutex, !m_unused_i2p_sessions_mutex); void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const; void DeleteNode(CNode* pnode); @@ -1666,7 +1710,7 @@ friend class CNode; /** (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(!m_nodes_mutex, !mutexMsgProc); void DumpAddresses(); @@ -1683,7 +1727,7 @@ friend class CNode; /** * 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 @@ -1695,7 +1739,7 @@ friend class CNode; * * @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); @@ -1735,12 +1779,12 @@ friend class CNode; 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 SharedMutex m_nodes_mutex; std::atomic nLastNodeId{0}; unsigned int nPrevNodeCount{0}; // Stores number of full-tx connections (outbound and manual) per network - std::array m_network_conn_counts GUARDED_BY(m_nodes_mutex) = {}; + std::array m_network_conn_counts GUARDED_BY(m_nodes_mutex) = {}; // TODO consider moving this to seperate mutex std::vector vPendingMasternodes; mutable RecursiveMutex cs_vPendingMasternodes; @@ -1934,7 +1978,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, !cs_mapSocketToNode); + void PerformReconnections() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !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 63d5aac9657b0..ea3800bdaab1a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1299,23 +1299,23 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) } m_connman.ForNode(nodeid, [this](CNode* pfrom) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); - if (lNodesAnnouncingHeaderAndIDs.size() >= 3) { - // As per BIP152, we only get 3 of our peers to announce - // blocks using compact encodings. - m_connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [this](CNode* pnodeStop){ - m_connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); - // save BIP152 bandwidth state: we select peer to be low-bandwidth - pnodeStop->m_bip152_highbandwidth_to = false; - return true; - }); - lNodesAnnouncingHeaderAndIDs.pop_front(); - } m_connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/true, /*version=*/CMPCTBLOCKS_VERSION)); // save BIP152 bandwidth state: we select peer to be high-bandwidth pfrom->m_bip152_highbandwidth_to = true; lNodesAnnouncingHeaderAndIDs.push_back(pfrom->GetId()); return true; }); + if (lNodesAnnouncingHeaderAndIDs.size() > 3) { + // As per BIP152, we only get 3 of our peers to announce + // blocks using compact encodings. + m_connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [this](CNode* pnodeStop){ + m_connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); + // save BIP152 bandwidth state: we select peer to be low-bandwidth + pnodeStop->m_bip152_highbandwidth_to = false; + return true; + }); + lNodesAnnouncingHeaderAndIDs.pop_front(); + } } bool PeerManagerImpl::TipMayBeStale() diff --git a/src/sync.cpp b/src/sync.cpp index d1c06376dc0c1..1747b69f6fee1 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -287,6 +287,15 @@ template void AssertLockHeldInternal(const char*, const char*, int, Mutex*); template void AssertLockHeldInternal(const char*, const char*, int, RecursiveMutex*); template void AssertLockHeldInternal(const char*, const char*, int, SharedMutex*); +template +void AssertSharedLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) +{ + if (LockHeld(cs)) return; + tfm::format(std::cerr, "Assertion failed: shared lock %s not held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld()); + abort(); +} +template void AssertSharedLockHeldInternal(const char*, const char*, int, SharedMutex*); + template void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) { diff --git a/src/sync.h b/src/sync.h index 8ca945cca76f4..3a4220c40c356 100644 --- a/src/sync.h +++ b/src/sync.h @@ -61,6 +61,8 @@ void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, c template void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs); template +void AssertSharedLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) SHARED_LOCKS_REQUIRED(cs); +template void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) LOCKS_EXCLUDED(cs); void DeleteLock(void* cs); bool LockStackEmpty(); @@ -79,6 +81,8 @@ inline void CheckLastCritical(void* cs, std::string& lockname, const char* guard template inline void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs) {} template +inline void AssertSharedLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) SHARED_LOCKS_REQUIRED(cs) {}; +template void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) LOCKS_EXCLUDED(cs) {} inline void DeleteLock(void* cs) {} inline bool LockStackEmpty() { return true; } @@ -160,6 +164,7 @@ class GlobalMutex : public Mutex { }; using SharedMutex = SharedAnnotatedMixin; #define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) +#define AssertSharedLockHeld(cs) AssertSharedLockHeldInternal(#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); } diff --git a/src/test/util/net.h b/src/test/util/net.h index ab1d958546c89..a52a3ec753d10 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -41,7 +41,7 @@ struct ConnmanTestMsg : public CConnman { std::vector TestNodes() { - LOCK(m_nodes_mutex); + READ_LOCK(m_nodes_mutex); return m_nodes; }