net: keep reference to each node during socket wait

Create the snapshot of `CConnman::vNodes` to operate on earlier in
`CConnman::SocketHandler()`, before calling `CConnman::SocketEvents()`
and pass the `vNodes` copy from the snapshot to `SocketEvents()`.

This will keep the refcount of each node incremented during
`SocketEvents()` so that the `CNode` object is not destroyed before
`SocketEvents()` has finished.

Currently in `SocketEvents()` we only remember file descriptor numbers
(when not holding `CConnman::cs_vNodes`) which is safe, but we will
change this to remember pointers to `CNode::m_sock`.
This commit is contained in:
Vasil Dimov 2021-04-28 18:29:32 +02:00
parent 75e8bf55f5
commit 664ac22c53
No known key found for this signature in database
GPG Key ID: 54DF06F64B55CBBF
2 changed files with 45 additions and 13 deletions

View File

@ -1331,16 +1331,17 @@ bool CConnman::InactivityCheck(const CNode& node) const
return false;
}
bool CConnman::GenerateSelectSet(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set)
bool CConnman::GenerateSelectSet(const std::vector<CNode*>& nodes,
std::set<SOCKET>& recv_set,
std::set<SOCKET>& send_set,
std::set<SOCKET>& error_set)
{
for (const ListenSocket& hListenSocket : vhListenSocket) {
recv_set.insert(hListenSocket.socket);
}
{
LOCK(cs_vNodes);
for (CNode* pnode : vNodes)
{
for (CNode* pnode : nodes) {
// Implement the following logic:
// * If there is data to send, select() for sending data. As this only
// happens when optimistic write failed, we choose to first drain the
@ -1378,10 +1379,13 @@ bool CConnman::GenerateSelectSet(std::set<SOCKET> &recv_set, std::set<SOCKET> &s
}
#ifdef USE_POLL
void CConnman::SocketEvents(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set)
void CConnman::SocketEvents(const std::vector<CNode*>& nodes,
std::set<SOCKET>& recv_set,
std::set<SOCKET>& send_set,
std::set<SOCKET>& error_set)
{
std::set<SOCKET> recv_select_set, send_select_set, error_select_set;
if (!GenerateSelectSet(recv_select_set, send_select_set, error_select_set)) {
if (!GenerateSelectSet(nodes, recv_select_set, send_select_set, error_select_set)) {
interruptNet.sleep_for(std::chrono::milliseconds(SELECT_TIMEOUT_MILLISECONDS));
return;
}
@ -1420,10 +1424,13 @@ void CConnman::SocketEvents(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_s
}
}
#else
void CConnman::SocketEvents(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set)
void CConnman::SocketEvents(const std::vector<CNode*>& nodes,
std::set<SOCKET>& recv_set,
std::set<SOCKET>& send_set,
std::set<SOCKET>& error_set)
{
std::set<SOCKET> recv_select_set, send_select_set, error_select_set;
if (!GenerateSelectSet(recv_select_set, send_select_set, error_select_set)) {
if (!GenerateSelectSet(nodes, recv_select_set, send_select_set, error_select_set)) {
interruptNet.sleep_for(std::chrono::milliseconds(SELECT_TIMEOUT_MILLISECONDS));
return;
}
@ -1497,8 +1504,10 @@ void CConnman::SocketEvents(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_s
void CConnman::SocketHandler()
{
const NodesSnapshot snap{*this, /*shuffle=*/false};
std::set<SOCKET> recv_set, send_set, error_set;
SocketEvents(recv_set, send_set, error_set);
SocketEvents(snap.Nodes(), recv_set, send_set, error_set);
if (interruptNet) return;
@ -1513,8 +1522,6 @@ void CConnman::SocketHandler()
}
}
const NodesSnapshot snap{*this, /*shuffle=*/false};
//
// Service each socket
//

View File

@ -983,8 +983,33 @@ private:
void NotifyNumConnectionsChanged();
/** Return true if the peer is inactive and should be disconnected. */
bool InactivityCheck(const CNode& node) const;
bool GenerateSelectSet(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set);
void SocketEvents(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set);
/**
* Generate a collection of sockets to check for IO readiness.
* @param[in] nodes Select from these nodes' sockets.
* @param[out] recv_set Sockets to check for read readiness.
* @param[out] send_set Sockets to check for write readiness.
* @param[out] error_set Sockets to check for errors.
* @return true if at least one socket is to be checked (the returned set is not empty)
*/
bool GenerateSelectSet(const std::vector<CNode*>& nodes,
std::set<SOCKET>& recv_set,
std::set<SOCKET>& send_set,
std::set<SOCKET>& error_set);
/**
* Check which sockets are ready for IO.
* @param[in] nodes Select from these nodes' sockets.
* @param[out] recv_set Sockets which are ready for read.
* @param[out] send_set Sockets which are ready for write.
* @param[out] error_set Sockets which have errors.
* This calls `GenerateSelectSet()` to gather a list of sockets to check.
*/
void SocketEvents(const std::vector<CNode*>& nodes,
std::set<SOCKET>& recv_set,
std::set<SOCKET>& send_set,
std::set<SOCKET>& error_set);
void SocketHandler();
void ThreadSocketHandler();
void ThreadDNSAddressSeed();