mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-04-29 06:49:38 -04:00
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:
parent
c24e6d025e
commit
cc8f239663
2 changed files with 26 additions and 24 deletions
31
src/net.cpp
31
src/net.cpp
|
@ -332,26 +332,16 @@ bool IsLocal(const CService& addr)
|
|||
return mapLocalHost.count(addr) > 0;
|
||||
}
|
||||
|
||||
CNode* CConnman::FindNode(const std::string& addrName)
|
||||
bool CConnman::OutboundConnectedToStr(const std::string& str_addr)
|
||||
{
|
||||
LOCK(m_nodes_mutex);
|
||||
for (CNode* pnode : m_nodes) {
|
||||
if (pnode->m_addr_name == addrName) {
|
||||
return pnode;
|
||||
}
|
||||
}
|
||||
return nullptr;
|
||||
return std::ranges::any_of(m_nodes, [&str_addr](CNode* node) { return node->m_addr_name == str_addr; });
|
||||
}
|
||||
|
||||
CNode* CConnman::FindNode(const CService& addr)
|
||||
bool CConnman::OutboundConnectedToService(const CService& addr)
|
||||
{
|
||||
LOCK(m_nodes_mutex);
|
||||
for (CNode* pnode : m_nodes) {
|
||||
if (static_cast<CService>(pnode->addr) == addr) {
|
||||
return pnode;
|
||||
}
|
||||
}
|
||||
return nullptr;
|
||||
return std::ranges::any_of(m_nodes, [&addr](CNode* node) { return node->addr == addr; });
|
||||
}
|
||||
|
||||
bool CConnman::AlreadyConnectedToAddress(const CAddress& addr)
|
||||
|
@ -399,10 +389,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
|
|||
return nullptr;
|
||||
|
||||
// Look for an existing connection
|
||||
CNode* pnode = FindNode(static_cast<CService>(addrConnect));
|
||||
if (pnode)
|
||||
{
|
||||
LogPrintf("Failed to open new connection, already connected\n");
|
||||
if (OutboundConnectedToService(addrConnect)) {
|
||||
LogPrintf("Failed to open new connection to %s, already connected", addrConnect.ToStringAddrPort());
|
||||
return nullptr;
|
||||
}
|
||||
}
|
||||
|
@ -432,9 +420,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<CService>(addrConnect));
|
||||
if (pnode) {
|
||||
if (OutboundConnectedToService(addrConnect)) {
|
||||
LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort());
|
||||
return nullptr;
|
||||
}
|
||||
|
@ -2999,8 +2985,9 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
|
|||
if (IsLocal(addrConnect) || banned_or_discouraged || AlreadyConnectedToAddress(addrConnect)) {
|
||||
return;
|
||||
}
|
||||
} else if (FindNode(std::string(pszDest)))
|
||||
} else if (OutboundConnectedToStr(pszDest)) {
|
||||
return;
|
||||
}
|
||||
|
||||
CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, conn_type, use_v2transport);
|
||||
|
||||
|
|
19
src/net.h
19
src/net.h
|
@ -1352,8 +1352,23 @@ private:
|
|||
|
||||
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 "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] str_addr String of the form "address:port", e.g. "1.2.3.4:8333".
|
||||
* @return true if connected to addrName.
|
||||
*/
|
||||
bool OutboundConnectedToStr(const std::string& str_addr);
|
||||
|
||||
/**
|
||||
* 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 Address and port to check.
|
||||
* @return true if connected to addr.
|
||||
*/
|
||||
bool OutboundConnectedToService(const CService& addr);
|
||||
|
||||
/**
|
||||
* Determine whether we're already connected to a given address, in order to
|
||||
|
|
Loading…
Add table
Reference in a new issue