From d7c58ad514ee00db00589216166808258bc16b60 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Sat, 24 Dec 2016 14:34:20 -0500
Subject: 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.
---
 src/net.cpp | 22 +++++++++-------------
 src/net.h   |  2 ++
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index b275bdd809..e7b4562eab 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 2baf82702c..aef3ef9a7d 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;
-- 
cgit v1.2.3


From 376b3c2c6e329357e4793c1d1b90d1dc0f30fed0 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Thu, 12 Jan 2017 20:08:52 -0800
Subject: Make the cs_sendProcessing a LOCK instead of a TRY_LOCK

Technically cs_sendProcessing is entirely useless now because it
is only ever taken on the one MessageHandler thread, but because
there may be multiple of those in the future, it is left in place
---
 src/net.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index e7b4562eab..1019d59544 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1871,9 +1871,8 @@ void CConnman::ThreadMessageHandler()
 
             // Send messages
             {
-                TRY_LOCK(pnode->cs_sendProcessing, lockSend);
-                if (lockSend)
-                    GetNodeSignals().SendMessages(pnode, *this, flagInterruptMsgProc);
+                LOCK(pnode->cs_sendProcessing);
+                GetNodeSignals().SendMessages(pnode, *this, flagInterruptMsgProc);
             }
             if (flagInterruptMsgProc)
                 return;
-- 
cgit v1.2.3