From ddfdf32b448bb47a89f0d70ed5c98652ef5b50b4 Mon Sep 17 00:00:00 2001 From: Brenton Bostick Date: Fri, 14 Apr 2023 13:42:25 -0400 Subject: [PATCH] fix thread-safety issues by adding appropriate locks --- node/Bond.cpp | 25 ++++++++++++++----------- node/Bond.hpp | 1 + node/Peer.cpp | 4 ++++ node/Peer.hpp | 1 + 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/node/Bond.cpp b/node/Bond.cpp index 21a8a5638..77a23fc6b 100644 --- a/node/Bond.cpp +++ b/node/Bond.cpp @@ -1946,17 +1946,20 @@ void Bond::dumpInfo(int64_t now, bool force) _lastSummaryDump = now; float overhead = (_overheadBytes / (timeSinceLastDump / 1000.0f) / 1000.0f); _overheadBytes = 0; - log("bond: bp=%d, fi=%" PRIu64 ", mi=%d, ud=%d, dd=%d, flows=%zu, leaf=%d, overhead=%f KB/s, links=(%d/%d)", - _policy, - _failoverInterval, - _monitorInterval, - _upDelay, - _downDelay, - _flows.size(), - _isLeaf, - overhead, - _numAliveLinks, - _numTotalLinks); + { + Mutex::Lock l(_flows_m); + log("bond: bp=%d, fi=%" PRIu64 ", mi=%d, ud=%d, dd=%d, flows=%zu, leaf=%d, overhead=%f KB/s, links=(%d/%d)", + _policy, + _failoverInterval, + _monitorInterval, + _upDelay, + _downDelay, + _flows.size(), + _isLeaf, + overhead, + _numAliveLinks, + _numTotalLinks); + } for (int i = 0; i < ZT_MAX_PEER_NETWORK_PATHS; ++i) { if (_paths[i].p) { dumpPathStatus(now, i); diff --git a/node/Bond.hpp b/node/Bond.hpp index 81b4691b5..e425a3f39 100644 --- a/node/Bond.hpp +++ b/node/Bond.hpp @@ -344,6 +344,7 @@ class Bond { */ static bool inUse() { + Mutex::Lock l(_bonds_m); return ! _bondPolicyTemplates.empty() || _defaultPolicy; } diff --git a/node/Peer.cpp b/node/Peer.cpp index 6fcf193d9..01e0691b1 100644 --- a/node/Peer.cpp +++ b/node/Peer.cpp @@ -123,6 +123,7 @@ void Peer::received( } // If same address on same interface then don't learn unless existing path isn't alive (prevents learning loop) if (_paths[i].p->address().ipsEqual(path->address()) && _paths[i].p->localSocket() == path->localSocket()) { + Mutex::Lock _l2(_bond_m); if (_paths[i].p->alive(now) && !_bond) { havePath = true; break; @@ -669,6 +670,7 @@ void Peer::recordOutgoingPacket(const SharedPtr &path, const uint64_t pack #ifndef ZT_NO_PEER_METRICS _outgoing_packet++; #endif + Mutex::Lock l(_bond_m); if (_localMultipathSupported && _bond) { _bond->recordOutgoingPacket(path, packetId, payloadLength, verb, flowId, now); } @@ -679,6 +681,7 @@ void Peer::recordIncomingInvalidPacket(const SharedPtr& path) #ifndef ZT_NO_PEER_METRICS _packet_errors++; #endif + Mutex::Lock l(_bond_m); if (_localMultipathSupported && _bond) { _bond->recordIncomingInvalidPacket(path); } @@ -687,6 +690,7 @@ void Peer::recordIncomingInvalidPacket(const SharedPtr& path) void Peer::recordIncomingPacket(const SharedPtr &path, const uint64_t packetId, uint16_t payloadLength, const Packet::Verb verb, const int32_t flowId, int64_t now) { + Mutex::Lock l(_bond_m); if (_localMultipathSupported && _bond) { _bond->recordIncomingPacket(path, packetId, payloadLength, verb, flowId, now); } diff --git a/node/Peer.hpp b/node/Peer.hpp index 551783f22..b2dc52433 100644 --- a/node/Peer.hpp +++ b/node/Peer.hpp @@ -521,6 +521,7 @@ public: * @return The bonding policy used to reach this peer */ SharedPtr bond() { + Mutex::Lock l(_bond_m); return _bond; }