Enable Clang Thread Safety Analysis.

https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
MeteorAtRains · Oct 4, 2018 · 4cc25e6 · 4cc25e6
1 parent e1860f2
commit 4cc25e6
Show file tree

Hide file tree

Showing 23 changed files with 126 additions and 50 deletions.
diff --git a/clang.diff b/clang.diff
@@ -1,7 +1,7 @@
 diff --git a/CMakeLists.txt b/CMakeLists.txt
 --- a/CMakeLists.txt
 +++ b/CMakeLists.txt
-@@ -18,7 +18,11 @@ set(CXX_FLAGS
+@@ -18,25 +18,29 @@ set(CXX_FLAGS
   -Wextra
   -Werror
   -Wconversion
@@ -13,7 +13,9 @@ diff --git a/CMakeLists.txt b/CMakeLists.txt
   -Wold-style-cast
   -Woverloaded-virtual
   -Wpointer-arith
-@@ -27,16 +31,15 @@ set(CXX_FLAGS
+  -Wshadow
++ -Wthread-safety
+  -Wwrite-strings
   -march=native
   # -MMD
   -std=c++0x

diff --git a/examples/ace/logging/client.cc b/examples/ace/logging/client.cc
@@ -109,8 +109,8 @@ class LogClient : noncopyable
   TcpClient client_;
   Codec codec_;
   MutexLock mutex_;
-  LogRecord logRecord_;
-  TcpConnectionPtr connection_;
+  LogRecord logRecord_ GUARDED_BY(mutex_);
+  TcpConnectionPtr connection_ GUARDED_BY(mutex_);
 };
 
 }

diff --git a/examples/asio/chat/client.cc b/examples/asio/chat/client.cc
@@ -73,7 +73,7 @@ class ChatClient : noncopyable
   TcpClient client_;
   LengthHeaderCodec codec_;
   MutexLock mutex_;
-  TcpConnectionPtr connection_;
+  TcpConnectionPtr connection_ GUARDED_BY(mutex_);
 };
 
 int main(int argc, char* argv[])

diff --git a/examples/asio/chat/server_threaded.cc b/examples/asio/chat/server_threaded.cc
@@ -71,7 +71,7 @@ class ChatServer : noncopyable
   TcpServer server_;
   LengthHeaderCodec codec_;
   MutexLock mutex_;
-  ConnectionList connections_;
+  ConnectionList connections_ GUARDED_BY(mutex_);
 };
 
 int main(int argc, char* argv[])

diff --git a/examples/asio/chat/server_threaded_efficient.cc b/examples/asio/chat/server_threaded_efficient.cc
@@ -86,7 +86,7 @@ class ChatServer : noncopyable
   TcpServer server_;
   LengthHeaderCodec codec_;
   MutexLock mutex_;
-  ConnectionListPtr connections_;
+  ConnectionListPtr connections_ GUARDED_BY(mutex_);
 };
 
 int main(int argc, char* argv[])

diff --git a/examples/asio/chat/server_threaded_highperformance.cc b/examples/asio/chat/server_threaded_highperformance.cc
@@ -100,7 +100,7 @@ class ChatServer : noncopyable
   typedef ThreadLocalSingleton<ConnectionList> LocalConnections;
 
   MutexLock mutex_;
-  std::set<EventLoop*> loops_;
+  std::set<EventLoop*> loops_ GUARDED_BY(mutex_);
 };
 
 int main(int argc, char* argv[])

diff --git a/examples/asio/tutorial/timer5/timer.cc b/examples/asio/tutorial/timer5/timer.cc
@@ -56,9 +56,9 @@ class Printer : muduo::noncopyable
 private:
 
   muduo::MutexLock mutex_;
-  muduo::net::EventLoop* loop1_;
-  muduo::net::EventLoop* loop2_;
-  int count_;
+  muduo::net::EventLoop* loop1_ PT_GUARDED_BY(mutex_);
+  muduo::net::EventLoop* loop2_ PT_GUARDED_BY(mutex_);
+  int count_ GUARDED_BY(mutex_);
 };
 
 int main()

diff --git a/examples/asio/tutorial/timer6/timer.cc b/examples/asio/tutorial/timer6/timer.cc
@@ -98,7 +98,7 @@ class Printer : muduo::noncopyable
   muduo::MutexLock mutex_;
   muduo::net::EventLoop* loop1_;
   muduo::net::EventLoop* loop2_;
-  int count_;
+  int count_ GUARDED_BY(mutex_);
 };
 
 int main()

diff --git a/examples/memcached/server/MemcacheServer.h b/examples/memcached/server/MemcacheServer.h
@@ -47,7 +47,7 @@ class MemcacheServer : muduo::noncopyable
   const time_t startTime_;
 
   mutable muduo::MutexLock mutex_;
-  std::unordered_map<string, SessionPtr> sessions_;
+  std::unordered_map<string, SessionPtr> sessions_ GUARDED_BY(mutex_);
 
   // a complicated solution to save memory
   struct Hash
@@ -81,7 +81,7 @@ class MemcacheServer : muduo::noncopyable
   // NOT guarded by mutex_, but here because server_ has to destructs before
   // sessions_
   muduo::net::TcpServer server_;
-  std::unique_ptr<Stats> stats_;
+  std::unique_ptr<Stats> stats_ PT_GUARDED_BY(mutex_);
 };
 
 #endif  // MUDUO_EXAMPLES_MEMCACHED_SERVER_MEMCACHESERVER_H
diff --git a/examples/multiplexer/multiplexer.cc b/examples/multiplexer/multiplexer.cc
@@ -286,9 +286,9 @@ class MultiplexServer
   int64_t oldCounter_;
   Timestamp startTime_;
   MutexLock mutex_;
-  TcpConnectionPtr backendConn_;
-  std::map<int, TcpConnectionPtr> clientConns_;
-  std::queue<int> availIds_;
+  TcpConnectionPtr backendConn_ GUARDED_BY(mutex_);
+  std::map<int, TcpConnectionPtr> clientConns_ GUARDED_BY(mutex_);
+  std::queue<int> availIds_ GUARDED_BY(mutex_);
 };
 
 int main(int argc, char* argv[])

diff --git a/muduo/base/AsyncLogging.h b/muduo/base/AsyncLogging.h
@@ -39,7 +39,7 @@ class AsyncLogging : noncopyable
     latch_.wait();
   }
 
-  void stop()
+  void stop() NO_THREAD_SAFETY_ANALYSIS
   {
     running_ = false;
     cond_.notify();
@@ -56,15 +56,15 @@ class AsyncLogging : noncopyable
 
   const int flushInterval_;
   std::atomic<bool> running_;
-  string basename_;
-  off_t rollSize_;
+  const string basename_;
+  const off_t rollSize_;
   muduo::Thread thread_;
   muduo::CountDownLatch latch_;
   muduo::MutexLock mutex_;
-  muduo::Condition cond_;
-  BufferPtr currentBuffer_;
-  BufferPtr nextBuffer_;
-  BufferVector buffers_;
+  muduo::Condition cond_ GUARDED_BY(mutex_);
+  BufferPtr currentBuffer_ GUARDED_BY(mutex_);
+  BufferPtr nextBuffer_ GUARDED_BY(mutex_);
+  BufferVector buffers_ GUARDED_BY(mutex_);
 };
 
 }

diff --git a/muduo/base/BlockingQueue.h b/muduo/base/BlockingQueue.h
@@ -63,8 +63,8 @@ class BlockingQueue : noncopyable
 
  private:
   mutable MutexLock mutex_;
-  Condition         notEmpty_;
-  std::deque<T>     queue_;
+  Condition         notEmpty_ GUARDED_BY(mutex_);
+  std::deque<T>     queue_ GUARDED_BY(mutex_);
 };
 
 }

diff --git a/muduo/base/BoundedBlockingQueue.h b/muduo/base/BoundedBlockingQueue.h
@@ -79,9 +79,9 @@ class BoundedBlockingQueue : noncopyable
 
  private:
   mutable MutexLock          mutex_;
-  Condition                  notEmpty_;
-  Condition                  notFull_;
-  boost::circular_buffer<T>  queue_;
+  Condition                  notEmpty_ GUARDED_BY(mutex_);
+  Condition                  notFull_ GUARDED_BY(mutex_);
+  boost::circular_buffer<T>  queue_ GUARDED_BY(mutex_);
 };
 
 }

diff --git a/muduo/base/CountDownLatch.h b/muduo/base/CountDownLatch.h
@@ -26,8 +26,8 @@ class CountDownLatch : noncopyable
 
  private:
   mutable MutexLock mutex_;
-  Condition condition_;
-  int count_;
+  Condition condition_ GUARDED_BY(mutex_);
+  int count_ GUARDED_BY(mutex_);
 };
 
 }

diff --git a/muduo/base/Mutex.h b/muduo/base/Mutex.h
@@ -11,6 +11,76 @@
 #include <assert.h>
 #include <pthread.h>
 
+// Thread safety annotations {
+// https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
+
+// Enable thread safety attributes only with clang.
+// The attributes can be safely erased when compiling with other compilers.
+#if defined(__clang__) && (!defined(SWIG))
+#define THREAD_ANNOTATION_ATTRIBUTE__(x)   __attribute__((x))
+#else
+#define THREAD_ANNOTATION_ATTRIBUTE__(x)   // no-op
+#endif
+
+#define CAPABILITY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(capability(x))
+
+#define SCOPED_CAPABILITY \
+  THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable)
+
+#define GUARDED_BY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x))
+
+#define PT_GUARDED_BY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(pt_guarded_by(x))
+
+#define ACQUIRED_BEFORE(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(acquired_before(__VA_ARGS__))
+
+#define ACQUIRED_AFTER(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(acquired_after(__VA_ARGS__))
+
+#define REQUIRES(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(requires_capability(__VA_ARGS__))
+
+#define REQUIRES_SHARED(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(requires_shared_capability(__VA_ARGS__))
+
+#define ACQUIRE(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(acquire_capability(__VA_ARGS__))
+
+#define ACQUIRE_SHARED(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(acquire_shared_capability(__VA_ARGS__))
+
+#define RELEASE(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(release_capability(__VA_ARGS__))
+
+#define RELEASE_SHARED(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(release_shared_capability(__VA_ARGS__))
+
+#define TRY_ACQUIRE(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_capability(__VA_ARGS__))
+
+#define TRY_ACQUIRE_SHARED(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_shared_capability(__VA_ARGS__))
+
+#define EXCLUDES(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(locks_excluded(__VA_ARGS__))
+
+#define ASSERT_CAPABILITY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(assert_capability(x))
+
+#define ASSERT_SHARED_CAPABILITY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(assert_shared_capability(x))
+
+#define RETURN_CAPABILITY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(lock_returned(x))
+
+#define NO_THREAD_SAFETY_ANALYSIS \
+  THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis)
+
+// End of thread safety annotations }
+
 #ifdef CHECK_PTHREAD_RETURN_VALUE
 
 #ifdef NDEBUG
@@ -46,9 +116,9 @@ namespace muduo
 //
 //  private:
 //   mutable MutexLock mutex_;
-//   std::vector<int> data_; // GUARDED BY mutex_
+//   std::vector<int> data_ GUARDED_BY(mutex_);
 // };
-class MutexLock : noncopyable
+class CAPABILITY("mutex") MutexLock : noncopyable
 {
  public:
   MutexLock()
@@ -69,20 +139,20 @@ class MutexLock : noncopyable
     return holder_ == CurrentThread::tid();
   }
 
-  void assertLocked() const
+  void assertLocked() const ASSERT_CAPABILITY(this)
   {
     assert(isLockedByThisThread());
   }
 
   // internal usage
 
-  void lock()
+  void lock() ACQUIRE()
   {
     MCHECK(pthread_mutex_lock(&mutex_));
     assignHolder();
   }
 
-  void unlock()
+  void unlock() RELEASE()
   {
     unassignHolder();
     MCHECK(pthread_mutex_unlock(&mutex_));
@@ -134,16 +204,16 @@ class MutexLock : noncopyable
 //   MutexLockGuard lock(mutex_);
 //   return data_.size();
 // }
-class MutexLockGuard : noncopyable
+class SCOPED_CAPABILITY MutexLockGuard : noncopyable
 {
  public:
-  explicit MutexLockGuard(MutexLock& mutex)
+  explicit MutexLockGuard(MutexLock& mutex) ACQUIRE(mutex)
     : mutex_(mutex)
   {
     mutex_.lock();
   }
 
-  ~MutexLockGuard()
+  ~MutexLockGuard() RELEASE()
   {
     mutex_.unlock();
   }

diff --git a/muduo/base/ThreadPool.h b/muduo/base/ThreadPool.h
@@ -47,12 +47,12 @@ class ThreadPool : noncopyable
   Task take();
 
   mutable MutexLock mutex_;
-  Condition notEmpty_;
-  Condition notFull_;
+  Condition notEmpty_ GUARDED_BY(mutex_);
+  Condition notFull_ GUARDED_BY(mutex_);
   string name_;
   Task threadInitCallback_;
   std::vector<std::unique_ptr<muduo::Thread>> threads_;
-  std::deque<Task> queue_;
+  std::deque<Task> queue_ GUARDED_BY(mutex_);
   size_t maxQueueSize_;
   bool running_;
 };

diff --git a/muduo/net/EventLoop.h b/muduo/net/EventLoop.h
@@ -157,7 +157,7 @@ class EventLoop : noncopyable
   Channel* currentActiveChannel_;
 
   mutable MutexLock mutex_;
-  std::vector<Functor> pendingFunctors_; // @GuardedBy mutex_
+  std::vector<Functor> pendingFunctors_ GUARDED_BY(mutex_);
 };
 
 }

diff --git a/muduo/net/EventLoopThread.cc b/muduo/net/EventLoopThread.cc
@@ -41,15 +41,17 @@ EventLoop* EventLoopThread::startLoop()
   assert(!thread_.started());
   thread_.start();
 
+  EventLoop* loop = NULL;
   {
     MutexLockGuard lock(mutex_);
     while (loop_ == NULL)
     {
       cond_.wait();
     }
+    loop = loop_;
   }
 
-  return loop_;
+  return loop;
 }
 
 void EventLoopThread::threadFunc()
@@ -69,6 +71,7 @@ void EventLoopThread::threadFunc()
 
   loop.loop();
   //assert(exiting_);
+  MutexLockGuard lock(mutex_);
   loop_ = NULL;
 }
 
diff --git a/muduo/net/EventLoopThread.h b/muduo/net/EventLoopThread.h
@@ -35,11 +35,11 @@ class EventLoopThread : noncopyable
  private:
   void threadFunc();
 
-  EventLoop* loop_;
+  EventLoop* loop_ GUARDED_BY(mutex_);
   bool exiting_;
   Thread thread_;
   MutexLock mutex_;
-  Condition cond_;
+  Condition cond_ GUARDED_BY(mutex_);
   ThreadInitCallback callback_;
 };
 

diff --git a/muduo/net/TcpClient.h b/muduo/net/TcpClient.h
@@ -81,7 +81,7 @@ class TcpClient : noncopyable
   // always in loop thread
   int nextConnId_;
   mutable MutexLock mutex_;
-  TcpConnectionPtr connection_; // @GuardedBy mutex_
+  TcpConnectionPtr connection_ GUARDED_BY(mutex_);
 };
 
 }