Skip to content

Commit 2a4450c

Browse files
committed
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.
1 parent 4268aba commit 2a4450c

File tree

2 files changed

+26
-24
lines changed

2 files changed

+26
-24
lines changed

src/net.cpp

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -331,26 +331,16 @@ bool IsLocal(const CService& addr)
331331
return mapLocalHost.count(addr) > 0;
332332
}
333333

334-
CNode* CConnman::FindNode(const std::string& addrName)
334+
bool CConnman::AlreadyConnectedToHost(const std::string& host) const
335335
{
336336
LOCK(m_nodes_mutex);
337-
for (CNode* pnode : m_nodes) {
338-
if (pnode->m_addr_name == addrName) {
339-
return pnode;
340-
}
341-
}
342-
return nullptr;
337+
return std::ranges::any_of(m_nodes, [&host](CNode* node) { return node->m_addr_name == host; });
343338
}
344339

345-
CNode* CConnman::FindNode(const CService& addr)
340+
bool CConnman::AlreadyConnectedToAddressPort(const CService& addr_port) const
346341
{
347342
LOCK(m_nodes_mutex);
348-
for (CNode* pnode : m_nodes) {
349-
if (static_cast<CService>(pnode->addr) == addr) {
350-
return pnode;
351-
}
352-
}
353-
return nullptr;
343+
return std::ranges::any_of(m_nodes, [&addr_port](CNode* node) { return node->addr == addr_port; });
354344
}
355345

356346
bool CConnman::AlreadyConnectedToAddress(const CNetAddr& addr) const
@@ -393,10 +383,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
393383
return nullptr;
394384

395385
// Look for an existing connection
396-
CNode* pnode = FindNode(static_cast<CService>(addrConnect));
397-
if (pnode)
398-
{
399-
LogPrintf("Failed to open new connection, already connected\n");
386+
if (AlreadyConnectedToAddressPort(addrConnect)) {
387+
LogInfo("Failed to open new connection to %s, already connected", addrConnect.ToStringAddrPort());
400388
return nullptr;
401389
}
402390
}
@@ -426,9 +414,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
426414
}
427415
// It is possible that we already have a connection to the IP/port pszDest resolved to.
428416
// In that case, drop the connection that was just created.
429-
LOCK(m_nodes_mutex);
430-
CNode* pnode = FindNode(static_cast<CService>(addrConnect));
431-
if (pnode) {
417+
if (AlreadyConnectedToAddressPort(addrConnect)) {
432418
LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort());
433419
return nullptr;
434420
}
@@ -2996,8 +2982,9 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
29962982
if (IsLocal(addrConnect) || banned_or_discouraged || AlreadyConnectedToAddress(addrConnect)) {
29972983
return;
29982984
}
2999-
} else if (FindNode(std::string(pszDest)))
2985+
} else if (AlreadyConnectedToHost(pszDest)) {
30002986
return;
2987+
}
30012988

30022989
CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, conn_type, use_v2transport);
30032990

src/net.h

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1365,8 +1365,23 @@ class CConnman
13651365

13661366
uint64_t CalculateKeyedNetGroup(const CNetAddr& ad) const;
13671367

1368-
CNode* FindNode(const std::string& addrName);
1369-
CNode* FindNode(const CService& addr);
1368+
/**
1369+
* Determine whether we're already connected to a given "host:port".
1370+
* Note that for inbound connections, the peer is likely using a random outbound
1371+
* port on their side, so this will likely not match any inbound connections.
1372+
* @param[in] host String of the form "host[:port]", e.g. "localhost" or "localhost:8333" or "1.2.3.4:8333".
1373+
* @return true if connected to `host`.
1374+
*/
1375+
bool AlreadyConnectedToHost(const std::string& host) const;
1376+
1377+
/**
1378+
* Determine whether we're already connected to a given address:port.
1379+
* Note that for inbound connections, the peer is likely using a random outbound
1380+
* port on their side, so this will likely not match any inbound connections.
1381+
* @param[in] addr_port Address and port to check.
1382+
* @return true if connected to addr_port.
1383+
*/
1384+
bool AlreadyConnectedToAddressPort(const CService& addr_port) const;
13701385

13711386
/**
13721387
* Determine whether we're already connected to a given address.

0 commit comments

Comments
 (0)