From 45e2e085612463dd9cca9f1b221733afa6d52991 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Mon, 6 Feb 2017 13:47:24 -0500 Subject: [PATCH 1/2] net: rearrange so that socket accesses can be grouped together --- src/net.cpp | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 4d0d781d6..b47514fd9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1152,9 +1152,6 @@ void CConnman::ThreadSocketHandler() { if (pnode->hSocket == INVALID_SOCKET) continue; - FD_SET(pnode->hSocket, &fdsetError); - hSocketMax = std::max(hSocketMax, pnode->hSocket); - have_fds = true; // Implement the following logic: // * If there is data to send, select() for sending data. As this only @@ -1166,16 +1163,24 @@ void CConnman::ThreadSocketHandler() // receiving data. // * Hand off all complete messages to the processor, to be handled without // blocking here. + + bool select_recv = !pnode->fPauseRecv; + bool select_send; { LOCK(pnode->cs_vSend); - if (!pnode->vSendMsg.empty()) { - FD_SET(pnode->hSocket, &fdsetSend); - continue; - } + select_send = !pnode->vSendMsg.empty(); } - { - if (!pnode->fPauseRecv) - FD_SET(pnode->hSocket, &fdsetRecv); + + FD_SET(pnode->hSocket, &fdsetError); + hSocketMax = std::max(hSocketMax, pnode->hSocket); + have_fds = true; + + if (select_send) { + FD_SET(pnode->hSocket, &fdsetSend); + continue; + } + if (select_recv) { + FD_SET(pnode->hSocket, &fdsetRecv); } } } @@ -1229,9 +1234,15 @@ void CConnman::ThreadSocketHandler() // // Receive // + bool recvSet = false; + bool sendSet = false; + bool errorSet = false; if (pnode->hSocket == INVALID_SOCKET) continue; - if (FD_ISSET(pnode->hSocket, &fdsetRecv) || FD_ISSET(pnode->hSocket, &fdsetError)) + recvSet = FD_ISSET(pnode->hSocket, &fdsetRecv); + sendSet = FD_ISSET(pnode->hSocket, &fdsetSend); + errorSet = FD_ISSET(pnode->hSocket, &fdsetError); + if (recvSet || errorSet) { { { @@ -1286,9 +1297,7 @@ void CConnman::ThreadSocketHandler() // // Send // - if (pnode->hSocket == INVALID_SOCKET) - continue; - if (FD_ISSET(pnode->hSocket, &fdsetSend)) + if (sendSet) { LOCK(pnode->cs_vSend); size_t nBytes = SocketSendData(pnode); From 9a0b784deaab6b9fffcab227d928987b981d0572 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Mon, 6 Feb 2017 14:05:45 -0500 Subject: [PATCH 2/2] net: add a lock around hSocket --- src/net.cpp | 43 ++++++++++++++++++++++++++++--------------- src/net.h | 1 + 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index b47514fd9..2625cccaa 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -425,6 +425,7 @@ void CConnman::DumpBanlist() void CNode::CloseSocketDisconnect() { fDisconnect = true; + LOCK(cs_hSocket); if (hSocket != INVALID_SOCKET) { LogPrint("net", "disconnecting peer=%d\n", id); @@ -789,7 +790,13 @@ size_t CConnman::SocketSendData(CNode *pnode) const while (it != pnode->vSendMsg.end()) { const auto &data = *it; assert(data.size() > pnode->nSendOffset); - int nBytes = send(pnode->hSocket, reinterpret_cast(data.data()) + pnode->nSendOffset, data.size() - pnode->nSendOffset, MSG_NOSIGNAL | MSG_DONTWAIT); + int nBytes = 0; + { + LOCK(pnode->cs_hSocket); + if (pnode->hSocket == INVALID_SOCKET) + break; + nBytes = send(pnode->hSocket, reinterpret_cast(data.data()) + pnode->nSendOffset, data.size() - pnode->nSendOffset, MSG_NOSIGNAL | MSG_DONTWAIT); + } if (nBytes > 0) { pnode->nLastSend = GetSystemTimeInSeconds(); pnode->nSendBytes += nBytes; @@ -1150,9 +1157,6 @@ void CConnman::ThreadSocketHandler() LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) { - if (pnode->hSocket == INVALID_SOCKET) - continue; - // 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 @@ -1171,6 +1175,10 @@ void CConnman::ThreadSocketHandler() select_send = !pnode->vSendMsg.empty(); } + LOCK(pnode->cs_hSocket); + if (pnode->hSocket == INVALID_SOCKET) + continue; + FD_SET(pnode->hSocket, &fdsetError); hSocketMax = std::max(hSocketMax, pnode->hSocket); have_fds = true; @@ -1237,18 +1245,27 @@ void CConnman::ThreadSocketHandler() bool recvSet = false; bool sendSet = false; bool errorSet = false; - if (pnode->hSocket == INVALID_SOCKET) - continue; - recvSet = FD_ISSET(pnode->hSocket, &fdsetRecv); - sendSet = FD_ISSET(pnode->hSocket, &fdsetSend); - errorSet = FD_ISSET(pnode->hSocket, &fdsetError); + { + LOCK(pnode->cs_hSocket); + if (pnode->hSocket == INVALID_SOCKET) + continue; + recvSet = FD_ISSET(pnode->hSocket, &fdsetRecv); + sendSet = FD_ISSET(pnode->hSocket, &fdsetSend); + errorSet = FD_ISSET(pnode->hSocket, &fdsetError); + } if (recvSet || errorSet) { { { // typical socket buffer is 8K-64K char pchBuf[0x10000]; - int nBytes = recv(pnode->hSocket, pchBuf, sizeof(pchBuf), MSG_DONTWAIT); + int nBytes = 0; + { + LOCK(pnode->cs_hSocket); + if (pnode->hSocket == INVALID_SOCKET) + continue; + nBytes = recv(pnode->hSocket, pchBuf, sizeof(pchBuf), MSG_DONTWAIT); + } if (nBytes > 0) { bool notify = false; @@ -2286,8 +2303,7 @@ void CConnman::Stop() // Close sockets BOOST_FOREACH(CNode* pnode, vNodes) - if (pnode->hSocket != INVALID_SOCKET) - CloseSocket(pnode->hSocket); + pnode->CloseSocketDisconnect(); BOOST_FOREACH(ListenSocket& hListenSocket, vhListenSocket) if (hListenSocket.socket != INVALID_SOCKET) if (!CloseSocket(hListenSocket.socket)) @@ -2688,9 +2704,6 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg) size_t nBytesSent = 0; { LOCK(pnode->cs_vSend); - if(pnode->hSocket == INVALID_SOCKET) { - return; - } bool optimisticSend(pnode->vSendMsg.empty()); //log total amount of bytes per command diff --git a/src/net.h b/src/net.h index 38f8d82ce..e5a19e0f4 100644 --- a/src/net.h +++ b/src/net.h @@ -572,6 +572,7 @@ public: uint64_t nSendBytes; std::deque> vSendMsg; CCriticalSection cs_vSend; + CCriticalSection cs_hSocket; CCriticalSection cs_vProcessMsg; std::list vProcessMsg;