net: change FindNode() to not return a node and rename it

All callers of `CConnman::FindNode()` use its return value `CNode*` only
as a boolean null/notnull. So change that method to return `bool`.

This removes the dangerous pattern of handling a `CNode` object (the
return value of `FindNode()`) without holding `CConnman::m_nodes_mutex`
and without having that object's reference count incremented for the
duration of the usage.

Also rename the method to better describe what it does.
This commit is contained in:
Vasil Dimov committed 2025-10-01 16:39:56 +02:00
1 parent 4268abae1a
commit 2a4450ccbb
2 files changed
+26 -24

No files matched your search

+9 -22
View File
@@ -331,26 +331,16 @@ bool IsLocal(const CService& addr)
return mapLocalHost.count(addr) > 0; return mapLocalHost.count(addr) > 0;
} }
CNode* CConnman::FindNode(const std::string& addrName) bool CConnman::AlreadyConnectedToHost(const std::string& host) const
{ {
LOCK(m_nodes_mutex); LOCK(m_nodes_mutex);
for (CNode* pnode : m_nodes) { return std::ranges::any_of(m_nodes, [&host](CNode* node) { return node->m_addr_name == host; });
if (pnode->m_addr_name == addrName) {
return pnode;
}
}
return nullptr;
} }
CNode* CConnman::FindNode(const CService& addr) bool CConnman::AlreadyConnectedToAddressPort(const CService& addr_port) const
{ {
LOCK(m_nodes_mutex); LOCK(m_nodes_mutex);
for (CNode* pnode : m_nodes) { return std::ranges::any_of(m_nodes, [&addr_port](CNode* node) { return node->addr == addr_port; });
if (static_cast<CService>(pnode->addr) == addr) {
return pnode;
}
}
return nullptr;
} }
bool CConnman::AlreadyConnectedToAddress(const CNetAddr& addr) const bool CConnman::AlreadyConnectedToAddress(const CNetAddr& addr) const
@@ -393,10 +383,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
return nullptr; return nullptr;
// Look for an existing connection // Look for an existing connection
CNode* pnode = FindNode(static_cast<CService>(addrConnect)); if (AlreadyConnectedToAddressPort(addrConnect)) {
if (pnode) LogInfo("Failed to open new connection to %s, already connected", addrConnect.ToStringAddrPort());
{
LogPrintf("Failed to open new connection, already connected\n");
return nullptr; return nullptr;
} }
} }
@@ -426,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. // 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. // In that case, drop the connection that was just created.
LOCK(m_nodes_mutex); if (AlreadyConnectedToAddressPort(addrConnect)) {
CNode* pnode = FindNode(static_cast<CService>(addrConnect));
if (pnode) {
LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort()); LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort());
return nullptr; return nullptr;
} }
@@ -2996,8 +2982,9 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
if (IsLocal(addrConnect) || banned_or_discouraged || AlreadyConnectedToAddress(addrConnect)) { if (IsLocal(addrConnect) || banned_or_discouraged || AlreadyConnectedToAddress(addrConnect)) {
return; return;
} }
} else if (FindNode(std::string(pszDest))) } else if (AlreadyConnectedToHost(pszDest)) {
return; return;
}
CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, conn_type, use_v2transport); CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, conn_type, use_v2transport);
+17 -2
View File
@@ -1365,8 +1365,23 @@ private:
uint64_t CalculateKeyedNetGroup(const CNetAddr& ad) const; uint64_t CalculateKeyedNetGroup(const CNetAddr& ad) const;
CNode* FindNode(const std::string& addrName); /**
CNode* FindNode(const CService& addr); * Determine whether we're already connected to a given "host:port".
* Note that for inbound connections, the peer is likely using a random outbound
* port on their side, so this will likely not match any inbound connections.
* @param[in] host String of the form "host[:port]", e.g. "localhost" or "localhost:8333" or "1.2.3.4:8333".
* @return true if connected to `host`.
*/
bool AlreadyConnectedToHost(const std::string& host) const;
/**
* Determine whether we're already connected to a given address:port.
* Note that for inbound connections, the peer is likely using a random outbound
* port on their side, so this will likely not match any inbound connections.
* @param[in] addr_port Address and port to check.
* @return true if connected to addr_port.
*/
bool AlreadyConnectedToAddressPort(const CService& addr_port) const;
/** /**
* Determine whether we're already connected to a given address. * Determine whether we're already connected to a given address.