net: RecordBytesSent under cs_vSend lock

MarcoFalke · MarcoFalke · commit faae6ca9e8a3 · 2020-05-13T12:10:43.000-04:00
The CNode member nSendBytes is incremented under the node's lock
cs_vSend. However, RecordBytesSent is not. An RPC thread that acquires
the cs_vSend lock puts the message handler thread on hold. While the
thread is on hold, getnettotals returns "stale" values or values that
don't add up.

This can be fixed by making cs_vSend a "write lock" for the total bytes
sent in connman.

After this commit, both calls to RecordBytesSent are done under the
LOCK(pnode-&gt;cs_vSend);
diff --git a/src/net.cpp b/src/net.cpp
@@ -1424,8 +1424,7 @@ void CConnman::SocketHandler()
         //
         // Send
         //
-        if (sendSet)
-        {
+        if (sendSet) {
             LOCK(pnode->cs_vSend);
             size_t nBytes = SocketSendData(pnode);
             if (nBytes) {
@@ -2744,7 +2743,6 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg)
     pnode->m_serializer->prepareForTransport(msg, serializedHeader);
     size_t nTotalSize = nMessageSize + serializedHeader.size();
 
-    size_t nBytesSent = 0;
     {
         LOCK(pnode->cs_vSend);
         bool optimisticSend(pnode->vSendMsg.empty());
@@ -2760,11 +2758,11 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg)
             pnode->vSendMsg.push_back(std::move(msg.data));
 
         // If write queue empty, attempt "optimistic write"
-        if (optimisticSend == true)
-            nBytesSent = SocketSendData(pnode);
+        if (optimisticSend == true) {
+            const size_t bytes_sent{SocketSendData(pnode)};
+            if (bytes_sent) RecordBytesSent(bytes_sent);
+        }
     }
-    if (nBytesSent)
-        RecordBytesSent(nBytesSent);
 }
 
 bool CConnman::ForNode(NodeId id, std::function<bool(CNode* pnode)> func)
