Merge pull request #2003 from alexanderkjeldaas/documented-locking-part-2

Documented locking part 1+2

src/net.h

@@ -305,7 +305,8 @@ public:
 
 
 
-    void BeginMessage(const char* pszCommand)
+    // TODO: Document the postcondition of this function.  Is cs_vSend locked?
+    void BeginMessage(const char* pszCommand) EXCLUSIVE_LOCK_FUNCTION(cs_vSend)
     {
         ENTER_CRITICAL_SECTION(cs_vSend);
         if (nHeaderStart != -1)
@@ -317,7 +318,8 @@ public:
             printf("sending: %s ", pszCommand);
     }
 
-    void AbortMessage()
+    // TODO: Document the precondition of this function.  Is cs_vSend locked?
+    void AbortMessage() UNLOCK_FUNCTION(cs_vSend)
     {
         if (nHeaderStart < 0)
             return;
@@ -330,7 +332,8 @@ public:
             printf("(aborted)\n");
     }
 
-    void EndMessage()
+    // TODO: Document the precondition of this function.  Is cs_vSend locked?
+    void EndMessage() UNLOCK_FUNCTION(cs_vSend)
     {
         if (mapArgs.count("-dropmessagestest") && GetRand(atoi(mapArgs["-dropmessagestest"])) == 0)
         {
@@ -362,19 +365,6 @@ public:
         LEAVE_CRITICAL_SECTION(cs_vSend);
     }
 
-    void EndMessageAbortIfEmpty()
-    {
-        if (nHeaderStart < 0)
-            return;
-        int nSize = vSend.size() - nMessageStart;
-        if (nSize > 0)
-            EndMessage();
-        else
-            AbortMessage();
-    }
-
-
-
     void PushVersion();
 
 

src/sync.h

@@ -9,15 +9,36 @@
 #include <boost/thread/recursive_mutex.hpp>
 #include <boost/thread/locks.hpp>
 #include <boost/thread/condition_variable.hpp>
+#include "threadsafety.h"
 
+// Template mixin that adds -Wthread-safety locking annotations to a
+// subset of the mutex API.
+template <typename PARENT>
+class LOCKABLE AnnotatedMixin : public PARENT
+{
+public:
+    void lock() EXCLUSIVE_LOCK_FUNCTION()
+    {
+      PARENT::lock();
+    }
 
+    void unlock() UNLOCK_FUNCTION()
+    {
+      PARENT::unlock();
+    }
 
+    bool try_lock() EXCLUSIVE_TRYLOCK_FUNCTION(true)
+    {
+      return PARENT::try_lock();
+    }
+};
 
 /** Wrapped boost mutex: supports recursive locking, but no waiting  */
-typedef boost::recursive_mutex CCriticalSection;
+// TODO: We should move away from using the recursive lock by default.
+typedef AnnotatedMixin<boost::recursive_mutex> CCriticalSection;
 
 /** Wrapped boost mutex: supports waiting but not recursive locking */
-typedef boost::mutex CWaitableCriticalSection;
+typedef AnnotatedMixin<boost::mutex> CWaitableCriticalSection;
 
 #ifdef DEBUG_LOCKORDER
 void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false);

src/threadsafety.h

@@ -0,0 +1,53 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2012 The Bitcoin developers
+// Distributed under the MIT/X11 software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+#ifndef BITCOIN_THREADSAFETY_H
+#define BITCOIN_THREADSAFETY_H
+
+#ifdef __clang__
+// TL;DR Add GUARDED_BY(mutex) to member variables. The others are
+// rarely necessary. Ex: int nFoo GUARDED_BY(cs_foo);
+//
+// See http://clang.llvm.org/docs/LanguageExtensions.html#threadsafety
+// for documentation.  The clang compiler can do advanced static analysis
+// of locking when given the -Wthread-safety option.
+#define LOCKABLE                        __attribute__ ((lockable))
+#define SCOPED_LOCKABLE                 __attribute__ ((scoped_lockable))
+#define GUARDED_BY(x)                   __attribute__ ((guarded_by(x)))
+#define GUARDED_VAR                     __attribute__ ((guarded_var))
+#define PT_GUARDED_BY(x)                __attribute__ ((pt_guarded_by(x)))
+#define PT_GUARDED_VAR                  __attribute__ ((pt_guarded_var))
+#define ACQUIRED_AFTER(...)             __attribute__ ((acquired_after(__VA_ARGS__)))
+#define ACQUIRED_BEFORE(...)            __attribute__ ((acquired_before(__VA_ARGS__)))
+#define EXCLUSIVE_LOCK_FUNCTION(...)    __attribute__ ((exclusive_lock_function(__VA_ARGS__)))
+#define SHARED_LOCK_FUNCTION(...)       __attribute__ ((shared_lock_function(__VA_ARGS__)))
+#define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__ ((exclusive_trylock_function(__VA_ARGS__)))
+#define SHARED_TRYLOCK_FUNCTION(...)    __attribute__ ((shared_trylock_function(__VA_ARGS__)))
+#define UNLOCK_FUNCTION(...)            __attribute__ ((unlock_function(__VA_ARGS__)))
+#define LOCK_RETURNED(x)                __attribute__ ((lock_returned(x)))
+#define LOCKS_EXCLUDED(...)             __attribute__ ((locks_excluded(__VA_ARGS__)))
+#define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__ ((exclusive_locks_required(__VA_ARGS__)))
+#define SHARED_LOCKS_REQUIRED(...)      __attribute__ ((shared_locks_required(__VA_ARGS__)))
+#define NO_THREAD_SAFETY_ANALYSIS       __attribute__ ((no_thread_safety_analysis))
+#else
+#define LOCKABLE
+#define SCOPED_LOCKABLE
+#define GUARDED_BY(x)
+#define GUARDED_VAR
+#define PT_GUARDED_BY(x)
+#define PT_GUARDED_VAR
+#define ACQUIRED_AFTER(...)
+#define ACQUIRED_BEFORE(...)
+#define EXCLUSIVE_LOCK_FUNCTION(...)
+#define SHARED_LOCK_FUNCTION(...)
+#define EXCLUSIVE_TRYLOCK_FUNCTION(...)
+#define SHARED_TRYLOCK_FUNCTION(...)
+#define UNLOCK_FUNCTION(...)
+#define LOCK_RETURNED(x)
+#define LOCKS_EXCLUDED(...)
+#define EXCLUSIVE_LOCKS_REQUIRED(...)
+#define SHARED_LOCKS_REQUIRED(...)
+#define NO_THREAD_SAFETY_ANALYSIS
+#endif  // __GNUC__
+#endif  // BITCOIN_THREADSAFETY_H