Merge bitcoin/bitcoin#24157: p2p: Replace RecursiveMutex `cs_totalBytesSent` with Mutex and rename it

709af67add93f6674fb80e3ae8e3f175653a62f0 p2p: replace RecursiveMutex `m_total_bytes_sent_mutex` with Mutex (w0xlt)
8be75fd0f0039eeea5f9af7c1eb17c584ed9f507 p2p: add assertions and negative TS annotations for `m_total_bytes_sent_mutex` (w0xlt)
a237a065cc2c6337e3797cc30a0f84c56c6d2f3b scripted-diff: rename cs_totalBytesSent -> m_total_bytes_sent_mutex (w0xlt)

Pull request description:

  Related to #19303, this PR gets rid of the RecursiveMutex `cs_totalBytesSent` and also adds `AssertLockNotHeld` macros combined with `LOCKS_EXCLUDED` thread safety annotations to avoid recursive locking.

ACKs for top commit:
  jonatack:
    ACK 709af67add93f6674fb80e3ae8e3f175653a62f0 per `git range-diff 7a4ac71 eff7918 709af67`, rebase to master, clang 15 debug build, and build with -Wthread-safety-negative
  vasild:
    ACK 709af67add93f6674fb80e3ae8e3f175653a62f0
  hebasto:
    ACK 709af67add93f6674fb80e3ae8e3f175653a62f0, tested on Ubuntu 22.04.

Tree-SHA512: 560b4e6c92b1511911d69185207df6ee809db09b96d97f96430d8d2595dc05c98cc691aaec8a58ef87cf2ab0a98675c210b8ce0be3dedb81e31114bbbfdfd8be

1 files changed, 29 insertions, 7 deletions

diff --git a/src/net.cpp b/src/net.cpp
index 77fa06ce26..aa70bed226 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1567,6 +1567,8 @@ void CConnman::SocketEvents(const std::vector<CNode*>& nodes,
 
 void CConnman::SocketHandler()
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+
     std::set<SOCKET> recv_set;
     std::set<SOCKET> send_set;
     std::set<SOCKET> error_set;
@@ -1593,6 +1595,8 @@ void CConnman::SocketHandlerConnected(const std::vector<CNode*>& nodes,
                                       const std::set<SOCKET>& send_set,
                                       const std::set<SOCKET>& error_set)
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+
     for (CNode* pnode : nodes) {
         if (interruptNet)
             return;
@@ -1694,6 +1698,8 @@ void CConnman::SocketHandlerListening(const std::set<SOCKET>& recv_set)
 
 void CConnman::ThreadSocketHandler()
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+
     SetSyscallSandboxPolicy(SyscallSandboxPolicy::NET);
     while (!interruptNet)
     {
@@ -2578,6 +2584,7 @@ bool CConnman::InitBinds(const Options& options)
 
 bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
     Init(connOptions);
 
     if (fListen && !InitBinds(connOptions)) {
@@ -2930,7 +2937,9 @@ void CConnman::RecordBytesRecv(uint64_t bytes)
 
 void CConnman::RecordBytesSent(uint64_t bytes)
 {
-    LOCK(cs_totalBytesSent);
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+    LOCK(m_total_bytes_sent_mutex);
+
     nTotalBytesSent += bytes;
 
     const auto now = GetTime<std::chrono::seconds>();
@@ -2946,7 +2955,8 @@ void CConnman::RecordBytesSent(uint64_t bytes)
 
 uint64_t CConnman::GetMaxOutboundTarget() const
 {
-    LOCK(cs_totalBytesSent);
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+    LOCK(m_total_bytes_sent_mutex);
     return nMaxOutboundLimit;
 }
 
@@ -2957,7 +2967,15 @@ std::chrono::seconds CConnman::GetMaxOutboundTimeframe() const
 
 std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle() const
 {
-    LOCK(cs_totalBytesSent);
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+    LOCK(m_total_bytes_sent_mutex);
+    return GetMaxOutboundTimeLeftInCycle_();
+}
+
+std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle_() const
+{
+    AssertLockHeld(m_total_bytes_sent_mutex);
+
     if (nMaxOutboundLimit == 0)
         return 0s;
 
@@ -2971,14 +2989,15 @@ std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle() const
 
 bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const
 {
-    LOCK(cs_totalBytesSent);
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+    LOCK(m_total_bytes_sent_mutex);
     if (nMaxOutboundLimit == 0)
         return false;
 
     if (historicalBlockServingLimit)
     {
         // keep a large enough buffer to at least relay each block once
-        const std::chrono::seconds timeLeftInCycle = GetMaxOutboundTimeLeftInCycle();
+        const std::chrono::seconds timeLeftInCycle = GetMaxOutboundTimeLeftInCycle_();
         const uint64_t buffer = timeLeftInCycle / std::chrono::minutes{10} * MAX_BLOCK_SERIALIZED_SIZE;
         if (buffer >= nMaxOutboundLimit || nMaxOutboundTotalBytesSentInCycle >= nMaxOutboundLimit - buffer)
             return true;
@@ -2991,7 +3010,8 @@ bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const
 
 uint64_t CConnman::GetOutboundTargetBytesLeft() const
 {
-    LOCK(cs_totalBytesSent);
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+    LOCK(m_total_bytes_sent_mutex);
     if (nMaxOutboundLimit == 0)
         return 0;
 
@@ -3005,7 +3025,8 @@ uint64_t CConnman::GetTotalBytesRecv() const
 
 uint64_t CConnman::GetTotalBytesSent() const
 {
-    LOCK(cs_totalBytesSent);
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+    LOCK(m_total_bytes_sent_mutex);
     return nTotalBytesSent;
 }
 
@@ -3052,6 +3073,7 @@ bool CConnman::NodeFullyConnected(const CNode* pnode)
 
 void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg)
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
     size_t nMessageSize = msg.data.size();
     LogPrint(BCLog::NET, "sending %s (%d bytes) peer=%d\n", msg.m_type, nMessageSize, pnode->GetId());
     if (gArgs.GetBoolArg("-capturemessages", false)) {