Split CNode::cs_vSend: message processing and message sending

cs_vSend is used for two purposes - to lock the datastructures used
to queue messages to place on the wire and to only call
SendMessages once at a time per-node. I believe SendMessages used
to access some of the vSendMsg stuff, but it doesn't anymore, so
these locks do not need to be on the same mutex, and also make
deadlocking much more likely.

2 files changed, 11 insertions, 13 deletions

diff --git a/src/net.cpp b/src/net.cpp
index b275bdd80..e7b4562ea 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1147,12 +1147,10 @@ void CConnman::ThreadSocketHandler()
                 // * Hand off all complete messages to the processor, to be handled without
                 //   blocking here.
                 {
-                    TRY_LOCK(pnode->cs_vSend, lockSend);
-                    if (lockSend) {
-                        if (!pnode->vSendMsg.empty()) {
-                            FD_SET(pnode->hSocket, &fdsetSend);
-                            continue;
-                        }
+                    LOCK(pnode->cs_vSend);
+                    if (!pnode->vSendMsg.empty()) {
+                        FD_SET(pnode->hSocket, &fdsetSend);
+                        continue;
                     }
                 }
                 {
@@ -1272,12 +1270,10 @@ void CConnman::ThreadSocketHandler()
                 continue;
             if (FD_ISSET(pnode->hSocket, &fdsetSend))
             {
-                TRY_LOCK(pnode->cs_vSend, lockSend);
-                if (lockSend) {
-                    size_t nBytes = SocketSendData(pnode);
-                    if (nBytes) {
-                        RecordBytesSent(nBytes);
-                    }
+                LOCK(pnode->cs_vSend);
+                size_t nBytes = SocketSendData(pnode);
+                if (nBytes) {
+                    RecordBytesSent(nBytes);
                 }
             }
 
@@ -1875,7 +1871,7 @@ void CConnman::ThreadMessageHandler()
 
             // Send messages
             {
-                TRY_LOCK(pnode->cs_vSend, lockSend);
+                TRY_LOCK(pnode->cs_sendProcessing, lockSend);
                 if (lockSend)
                     GetNodeSignals().SendMessages(pnode, *this, flagInterruptMsgProc);
             }
diff --git a/src/net.h b/src/net.h
index 2baf82702..aef3ef9a7 100644
--- a/src/net.h
+++ b/src/net.h
@@ -618,6 +618,8 @@ public:
     std::list<CNetMessage> vProcessMsg;
     size_t nProcessQueueSize;
 
+    CCriticalSection cs_sendProcessing;
+
     std::deque<CInv> vRecvGetData;
     uint64_t nRecvBytes;
     std::atomic<int> nRecvVersion;