From a237a065cc2c6337e3797cc30a0f84c56c6d2f3b Mon Sep 17 00:00:00 2001 From: w0xlt <94266259+w0xlt@users.noreply.github.com> Date: Tue, 25 Jan 2022 18:05:04 -0300 Subject: [PATCH 1/3] scripted-diff: rename cs_totalBytesSent -> m_total_bytes_sent_mutex -BEGIN VERIFY SCRIPT- sed -i 's/cs_totalBytesSent/m_total_bytes_sent_mutex/g' -- $(git grep --files-with-matches 'cs_totalBytesSent') -END VERIFY SCRIPT- --- src/net.cpp | 12 ++++++------ src/net.h | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 602d56ab984..77a1d1339bb 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2913,7 +2913,7 @@ void CConnman::RecordBytesRecv(uint64_t bytes) void CConnman::RecordBytesSent(uint64_t bytes) { - LOCK(cs_totalBytesSent); + LOCK(m_total_bytes_sent_mutex); nTotalBytesSent += bytes; const auto now = GetTime(); @@ -2929,7 +2929,7 @@ void CConnman::RecordBytesSent(uint64_t bytes) uint64_t CConnman::GetMaxOutboundTarget() const { - LOCK(cs_totalBytesSent); + LOCK(m_total_bytes_sent_mutex); return nMaxOutboundLimit; } @@ -2940,7 +2940,7 @@ std::chrono::seconds CConnman::GetMaxOutboundTimeframe() const std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle() const { - LOCK(cs_totalBytesSent); + LOCK(m_total_bytes_sent_mutex); if (nMaxOutboundLimit == 0) return 0s; @@ -2954,7 +2954,7 @@ std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle() const bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const { - LOCK(cs_totalBytesSent); + LOCK(m_total_bytes_sent_mutex); if (nMaxOutboundLimit == 0) return false; @@ -2974,7 +2974,7 @@ bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const uint64_t CConnman::GetOutboundTargetBytesLeft() const { - LOCK(cs_totalBytesSent); + LOCK(m_total_bytes_sent_mutex); if (nMaxOutboundLimit == 0) return 0; @@ -2988,7 +2988,7 @@ uint64_t CConnman::GetTotalBytesRecv() const uint64_t CConnman::GetTotalBytesSent() const { - LOCK(cs_totalBytesSent); + LOCK(m_total_bytes_sent_mutex); return nTotalBytesSent; } diff --git a/src/net.h b/src/net.h index 1bc011c7489..e9737278d9f 100644 --- a/src/net.h +++ b/src/net.h @@ -776,7 +776,7 @@ public: nReceiveFloodSize = connOptions.nReceiveFloodSize; m_peer_connect_timeout = std::chrono::seconds{connOptions.m_peer_connect_timeout}; { - LOCK(cs_totalBytesSent); + LOCK(m_total_bytes_sent_mutex); nMaxOutboundLimit = connOptions.nMaxOutboundLimit; } vWhitelistedRange = connOptions.vWhitelistedRange; @@ -1062,14 +1062,14 @@ private: static bool NodeFullyConnected(const CNode* pnode); // Network usage totals - mutable RecursiveMutex cs_totalBytesSent; + mutable RecursiveMutex m_total_bytes_sent_mutex; std::atomic nTotalBytesRecv{0}; - uint64_t nTotalBytesSent GUARDED_BY(cs_totalBytesSent) {0}; + uint64_t nTotalBytesSent GUARDED_BY(m_total_bytes_sent_mutex) {0}; // outbound limit & stats - uint64_t nMaxOutboundTotalBytesSentInCycle GUARDED_BY(cs_totalBytesSent) {0}; - std::chrono::seconds nMaxOutboundCycleStartTime GUARDED_BY(cs_totalBytesSent) {0}; - uint64_t nMaxOutboundLimit GUARDED_BY(cs_totalBytesSent); + uint64_t nMaxOutboundTotalBytesSentInCycle GUARDED_BY(m_total_bytes_sent_mutex) {0}; + std::chrono::seconds nMaxOutboundCycleStartTime GUARDED_BY(m_total_bytes_sent_mutex) {0}; + uint64_t nMaxOutboundLimit GUARDED_BY(m_total_bytes_sent_mutex); // P2P timeout in seconds std::chrono::seconds m_peer_connect_timeout; From 8be75fd0f0039eeea5f9af7c1eb17c584ed9f507 Mon Sep 17 00:00:00 2001 From: w0xlt <94266259+w0xlt@users.noreply.github.com> Date: Tue, 25 Jan 2022 18:18:52 -0300 Subject: [PATCH 2/3] p2p: add assertions and negative TS annotations for `m_total_bytes_sent_mutex` Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> --- src/net.cpp | 24 +++++++++++++++++++++++- src/net.h | 34 ++++++++++++++++++++-------------- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 77a1d1339bb..a2ba625cdbc 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1563,6 +1563,8 @@ void CConnman::SocketEvents(const std::vector& nodes, void CConnman::SocketHandler() { + AssertLockNotHeld(m_total_bytes_sent_mutex); + std::set recv_set; std::set send_set; std::set error_set; @@ -1589,6 +1591,8 @@ void CConnman::SocketHandlerConnected(const std::vector& nodes, const std::set& send_set, const std::set& error_set) { + AssertLockNotHeld(m_total_bytes_sent_mutex); + for (CNode* pnode : nodes) { if (interruptNet) return; @@ -1690,6 +1694,8 @@ void CConnman::SocketHandlerListening(const std::set& recv_set) void CConnman::ThreadSocketHandler() { + AssertLockNotHeld(m_total_bytes_sent_mutex); + SetSyscallSandboxPolicy(SyscallSandboxPolicy::NET); while (!interruptNet) { @@ -2561,6 +2567,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)) { @@ -2913,7 +2920,9 @@ void CConnman::RecordBytesRecv(uint64_t bytes) void CConnman::RecordBytesSent(uint64_t bytes) { + AssertLockNotHeld(m_total_bytes_sent_mutex); LOCK(m_total_bytes_sent_mutex); + nTotalBytesSent += bytes; const auto now = GetTime(); @@ -2929,6 +2938,7 @@ void CConnman::RecordBytesSent(uint64_t bytes) uint64_t CConnman::GetMaxOutboundTarget() const { + AssertLockNotHeld(m_total_bytes_sent_mutex); LOCK(m_total_bytes_sent_mutex); return nMaxOutboundLimit; } @@ -2940,7 +2950,15 @@ std::chrono::seconds CConnman::GetMaxOutboundTimeframe() const std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle() const { + 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; @@ -2954,6 +2972,7 @@ std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle() const bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const { + AssertLockNotHeld(m_total_bytes_sent_mutex); LOCK(m_total_bytes_sent_mutex); if (nMaxOutboundLimit == 0) return false; @@ -2961,7 +2980,7 @@ bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const 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; @@ -2974,6 +2993,7 @@ bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const uint64_t CConnman::GetOutboundTargetBytesLeft() const { + AssertLockNotHeld(m_total_bytes_sent_mutex); LOCK(m_total_bytes_sent_mutex); if (nMaxOutboundLimit == 0) return 0; @@ -2988,6 +3008,7 @@ uint64_t CConnman::GetTotalBytesRecv() const uint64_t CConnman::GetTotalBytesSent() const { + AssertLockNotHeld(m_total_bytes_sent_mutex); LOCK(m_total_bytes_sent_mutex); return nTotalBytesSent; } @@ -3035,6 +3056,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)) { diff --git a/src/net.h b/src/net.h index e9737278d9f..8cd83e00b50 100644 --- a/src/net.h +++ b/src/net.h @@ -760,7 +760,10 @@ public: bool m_i2p_accept_incoming; }; - void Init(const Options& connOptions) { + void Init(const Options& connOptions) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex) + { + AssertLockNotHeld(m_total_bytes_sent_mutex); + nLocalServices = connOptions.nLocalServices; nMaxConnections = connOptions.nMaxConnections; m_max_outbound_full_relay = std::min(connOptions.m_max_outbound_full_relay, connOptions.nMaxConnections); @@ -789,7 +792,7 @@ public: CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, bool network_active = true); ~CConnman(); - bool Start(CScheduler& scheduler, const Options& options); + bool Start(CScheduler& scheduler, const Options& options) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); void StopThreads(); void StopNodes(); @@ -808,7 +811,7 @@ public: bool ForNode(NodeId id, std::function func); - void PushMessage(CNode* pnode, CSerializedNetMsg&& msg); + void PushMessage(CNode* pnode, CSerializedNetMsg&& msg) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); using NodeFn = std::function; void ForEachNode(const NodeFn& func) @@ -899,24 +902,22 @@ public: //! that peer during `net_processing.cpp:PushNodeVersion()`. ServiceFlags GetLocalServices() const; - uint64_t GetMaxOutboundTarget() const; + uint64_t GetMaxOutboundTarget() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); std::chrono::seconds GetMaxOutboundTimeframe() const; //! check if the outbound target is reached //! if param historicalBlockServingLimit is set true, the function will //! response true if the limit for serving historical blocks has been reached - bool OutboundTargetReached(bool historicalBlockServingLimit) const; + bool OutboundTargetReached(bool historicalBlockServingLimit) const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); //! response the bytes left in the current max outbound cycle //! in case of no limit, it will always response 0 - uint64_t GetOutboundTargetBytesLeft() const; + uint64_t GetOutboundTargetBytesLeft() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); - //! returns the time left in the current max outbound cycle - //! in case of no limit, it will always return 0 - std::chrono::seconds GetMaxOutboundTimeLeftInCycle() const; + std::chrono::seconds GetMaxOutboundTimeLeftInCycle() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); uint64_t GetTotalBytesRecv() const; - uint64_t GetTotalBytesSent() const; + uint64_t GetTotalBytesSent() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); /** Get a unique deterministic randomizer. */ CSipHasher GetDeterministicRandomizer(uint64_t id) const; @@ -942,6 +943,10 @@ private: NetPermissionFlags m_permissions; }; + //! returns the time left in the current max outbound cycle + //! in case of no limit, it will always return 0 + std::chrono::seconds GetMaxOutboundTimeLeftInCycle_() const EXCLUSIVE_LOCKS_REQUIRED(m_total_bytes_sent_mutex); + bool BindListenPort(const CService& bindAddr, bilingual_str& strError, NetPermissionFlags permissions); bool Bind(const CService& addr, unsigned int flags, NetPermissionFlags permissions); bool InitBinds(const Options& options); @@ -1001,7 +1006,7 @@ private: /** * Check connected and listening sockets for IO readiness and process them accordingly. */ - void SocketHandler(); + void SocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); /** * Do the read/write for connected sockets that are ready for IO. @@ -1014,7 +1019,8 @@ private: void SocketHandlerConnected(const std::vector& nodes, const std::set& recv_set, const std::set& send_set, - const std::set& error_set); + const std::set& error_set) + EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); /** * Accept incoming connections, one from each read-ready listening socket. @@ -1022,7 +1028,7 @@ private: */ void SocketHandlerListening(const std::set& recv_set); - void ThreadSocketHandler(); + void ThreadSocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); void ThreadDNSAddressSeed(); uint64_t CalculateKeyedNetGroup(const CAddress& ad) const; @@ -1051,7 +1057,7 @@ private: // Network stats void RecordBytesRecv(uint64_t bytes); - void RecordBytesSent(uint64_t bytes); + void RecordBytesSent(uint64_t bytes) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); /** * Return vector of current BLOCK_RELAY peers. From 709af67add93f6674fb80e3ae8e3f175653a62f0 Mon Sep 17 00:00:00 2001 From: w0xlt <94266259+w0xlt@users.noreply.github.com> Date: Tue, 25 Jan 2022 18:20:13 -0300 Subject: [PATCH 3/3] p2p: replace RecursiveMutex `m_total_bytes_sent_mutex` with Mutex --- src/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net.h b/src/net.h index 8cd83e00b50..f77edb253b6 100644 --- a/src/net.h +++ b/src/net.h @@ -1068,7 +1068,7 @@ private: static bool NodeFullyConnected(const CNode* pnode); // Network usage totals - mutable RecursiveMutex m_total_bytes_sent_mutex; + mutable Mutex m_total_bytes_sent_mutex; std::atomic nTotalBytesRecv{0}; uint64_t nTotalBytesSent GUARDED_BY(m_total_bytes_sent_mutex) {0};