Annotate Peer class

Author: marta-lokhova

src/overlay/Hmac.h

@@ -5,16 +5,16 @@
 // of this distribution or at http://www.apache.org/licenses/LICENSE-2.0
 
 #include "Tracy.hpp"
+#include "util/ThreadAnnotations.h"
 #include "xdr/Stellar-overlay.h"
 #include "xdr/Stellar-types.h"
-#include <mutex>
 
 using namespace stellar;
 
 class Hmac
 {
 #ifndef USE_TRACY
-    std::mutex mMutex;
+    Mutex mMutex;
 #else
     TracyLockable(std::mutex, mMutex);
 #endif

src/overlay/Peer.h

@@ -13,6 +13,7 @@
 #include "overlay/PeerBareAddress.h"
 #include "transactions/TransactionFrameBase.h"
 #include "util/NonCopyable.h"
+#include "util/ThreadAnnotations.h"
 #include "util/Timer.h"
 #include "xdrpp/message.h"
 #include <medida/counter.h>
@@ -187,13 +188,16 @@ class Peer : public std::enable_shared_from_this<Peer>,
 
     PeerRole const mRole;
     OverlayMetrics& mOverlayMetrics;
+    // No need for GUARDED_BY, PeerMettrics is thread-safe
     PeerMetrics mPeerMetrics;
-    std::string mDropReason;
+#ifdef BUILD_TESTS
+    std::string mDropReason GUARDED_BY(mStateMutex);
+#endif
 
     // Mutex to protect PeerState, which can be accessed and modified from
     // multiple threads
 #ifndef USE_TRACY
-    std::recursive_mutex mutable mStateMutex;
+    RecursiveMutex mutable mStateMutex;
 #else
     mutable TracyLockable(std::recursive_mutex, mStateMutex);
 #endif
@@ -217,10 +221,12 @@ class Peer : public std::enable_shared_from_this<Peer>,
     // they all take a LockGuard that should be holding mStateMutex,
     // but do not lock that mutex themselves (to allow atomic
     // read-modify-write cycles or similar patterns in callers).
-    bool shouldAbort(RecursiveLockGuard const& stateGuard) const;
-    void setState(RecursiveLockGuard const& stateGuard, PeerState newState);
+    bool shouldAbort(RecursiveLockGuard const& stateGuard) const
+        REQUIRES(mStateMutex);
+    void setState(RecursiveLockGuard const& stateGuard, PeerState newState)
+        REQUIRES(mStateMutex);
     PeerState
-    getState(RecursiveLockGuard const& stateGuard) const
+    getState(RecursiveLockGuard const& stateGuard) const REQUIRES(mStateMutex)
     {
         return mState;
     }
@@ -248,7 +254,7 @@ class Peer : public std::enable_shared_from_this<Peer>,
     // IOW, all methods using these private variables and functions below must
     // synchronize access manually
   private:
-    PeerState mState;
+    PeerState mState GUARDED_BY(mStateMutex);
     NodeID mPeerID;
     uint256 mSendNonce;
     uint256 mRecvNonce;
@@ -424,8 +430,10 @@ class Peer : public std::enable_shared_from_this<Peer>,
 
     /* The following functions can be called from background thread, so they
      * must be thread-safe */
-    bool isConnected(RecursiveLockGuard const& stateGuard) const;
-    bool isAuthenticated(RecursiveLockGuard const& stateGuard) const;
+    bool isConnected(RecursiveLockGuard const& stateGuard) const
+        REQUIRES(mStateMutex);
+    bool isAuthenticated(RecursiveLockGuard const& stateGuard) const
+        REQUIRES(mStateMutex);
 
     PeerMetrics&
     getPeerMetrics()

src/overlay/test/LoopbackPeer.h

@@ -22,6 +22,8 @@ namespace stellar
 // pair of them wrapped in a LoopbackPeerConnection that explicitly manages the
 // lifecycle of the connection.
 
+// This class is not thread-safe and is not meant to utilize multi-threading. It
+// is only safe to call its methods from the main thread.
 class LoopbackPeer : public Peer
 {
   private:
@@ -57,7 +59,7 @@ class LoopbackPeer : public Peer
                      std::shared_ptr<StellarMessage const> msg) override;
     AuthCert getAuthCert() override;
 
-    void processInQueue();
+    void processInQueue() NO_THREAD_SAFETY_ANALYSIS;
     void recvMessage(xdr::msg_ptr const& xdrBytes);
 
   public:
@@ -70,11 +72,12 @@ class LoopbackPeer : public Peer
 
     static std::pair<std::shared_ptr<LoopbackPeer>,
                      std::shared_ptr<LoopbackPeer>>
-    initiate(Application& app, Application& otherApp);
+    initiate(Application& app, Application& otherApp) NO_THREAD_SAFETY_ANALYSIS;
 
-    void drop(std::string const& reason, DropDirection dropDirection) override;
+    void drop(std::string const& reason,
+              DropDirection dropDirection) NO_THREAD_SAFETY_ANALYSIS override;
 
-    void deliverOne();
+    void deliverOne() NO_THREAD_SAFETY_ANALYSIS;
     void deliverAll();
     void dropAll();
     size_t getBytesQueued() const;

src/util/GlobalChecks.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include "util/ThreadAnnotations.h"
 #include <Tracy.hpp>
 #include <mutex>
 
@@ -46,8 +47,8 @@ void dbgAbort();
 #endif
 
 #ifndef USE_TRACY
-using RecursiveLockGuard = std::lock_guard<std::recursive_mutex>;
-using LockGuard = std::lock_guard<std::mutex>;
+using RecursiveLockGuard = RecursiveMutexLocker;
+using LockGuard = MutexLocker;
 #else
 using RecursiveLockGuard = std::lock_guard<LockableBase(std::recursive_mutex)>;
 using LockGuard = std::lock_guard<LockableBase(std::mutex)>;

src/util/ThreadAnnotations.h

@@ -157,17 +157,17 @@ class LOCKABLE Mutex
 
 // MutexLocker is an RAII class that acquires a mutex in its constructor, and
 // releases it in its destructor.
-class SCOPED_LOCKABLE MutexLocker
+template <typename MutexType> class SCOPED_LOCKABLE MutexLockerT
 {
   private:
-    Mutex& mut;
+    MutexType& mut;
 
   public:
-    MutexLocker(Mutex& mu) EXCLUSIVE_LOCK_FUNCTION(mu) : mut(mu)
+    MutexLockerT(MutexType& mu) EXCLUSIVE_LOCK_FUNCTION(mu) : mut(mu)
     {
         mu.Lock();
     }
-    ~MutexLocker() UNLOCK_FUNCTION()
+    ~MutexLockerT() UNLOCK_FUNCTION()
     {
         mut.Unlock();
     }
@@ -213,40 +213,53 @@ class LOCKABLE SharedMutex
     }
 };
 
-// SharedLockExclusive is an RAII class that acquires a shared mutex in
-// exclusive mode in its constructor, and releases it in its destructor.
-class SCOPED_LOCKABLE SharedLockExclusive
+// SharedLockShared is an RAII class that acquires a shared mutex in shared
+// mode in its constructor, and releases it in its destructor.
+class SCOPED_LOCKABLE SharedLockShared
 {
   private:
     SharedMutex& mut;
 
   public:
-    SharedLockExclusive(SharedMutex& mu) EXCLUSIVE_LOCK_FUNCTION(mu) : mut(mu)
+    SharedLockShared(SharedMutex& mu) SHARED_LOCK_FUNCTION(mu) : mut(mu)
     {
-        mu.Lock();
+        mu.LockShared();
     }
-    ~SharedLockExclusive() UNLOCK_FUNCTION()
+    ~SharedLockShared() UNLOCK_FUNCTION()
     {
-        mut.Unlock();
+        mut.UnlockShared();
     }
 };
 
-// SharedLockShared is an RAII class that acquires a shared mutex in shared
-// mode in its constructor, and releases it in its destructor.
-class SCOPED_LOCKABLE SharedLockShared
+// Defines an annotated interface for recursive mutexes.
+// These methods can be implemented to use any internal recursive_mutex
+// implementation.
+class LOCKABLE RecursiveMutex
 {
   private:
-    SharedMutex& mut;
+    std::recursive_mutex mRecursiveMutex;
 
   public:
-    SharedLockShared(SharedMutex& mu) SHARED_LOCK_FUNCTION(mu) : mut(mu)
+    // Acquire/lock this mutex exclusively. The same thread may acquire the
+    // mutex multiple times without blocking. The owning thread must release the
+    // mutex the same number of times it was acquired.
+    void
+    Lock() EXCLUSIVE_LOCK_FUNCTION()
     {
-        mu.LockShared();
+        mRecursiveMutex.lock();
     }
-    ~SharedLockShared() UNLOCK_FUNCTION()
+
+    // Release/unlock the mutex. Only the owning thread can release the mutex,
+    // and the mutex must be released as many times as it was acquired.
+    void
+    Unlock() UNLOCK_FUNCTION()
     {
-        mut.UnlockShared();
+        mRecursiveMutex.unlock();
     }
 };
 
+using MutexLocker = MutexLockerT<Mutex>;
+using RecursiveMutexLocker = MutexLockerT<RecursiveMutex>;
+using SharedLockExclusive = MutexLockerT<SharedMutex>;
+
 #endif // THREAD_ANNOTATIONS_H_