Made use of clang's Thread Safety Analysis

CMakeLists.txt

@@ -7,6 +7,9 @@ option(BUILD_TESTING "set ON to build library tests" OFF)
 
 if(NOT MSVC)
     add_compile_options(-std=c++11 -Wall -Wextra -Wsign-conversion)
+    if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+        add_compile_options(-Wthread-safety)
+    endif()
 else()
     add_compile_options(/W1)
 endif()

client_http.hpp

@@ -2,9 +2,9 @@
 #define CLIENT_HTTP_HPP
 
 #include "asio_compatibility.hpp"
+#include "mutex.hpp"
 #include "utility.hpp"
 #include <limits>
-#include <mutex>
 #include <random>
 #include <unordered_set>
 #include <vector>
@@ -208,12 +208,12 @@ namespace SimpleWeb {
       });
 
       {
-        std::lock_guard<std::mutex> lock(concurrent_synchronous_requests_mutex);
+        LockGuard lock(concurrent_synchronous_requests_mutex);
         ++concurrent_synchronous_requests;
       }
       io_service->run();
       {
-        std::lock_guard<std::mutex> lock(concurrent_synchronous_requests_mutex);
+        LockGuard lock(concurrent_synchronous_requests_mutex);
         --concurrent_synchronous_requests;
         if(!concurrent_synchronous_requests)
           restart(*io_service);
@@ -238,12 +238,12 @@ namespace SimpleWeb {
       });
 
       {
-        std::lock_guard<std::mutex> lock(concurrent_synchronous_requests_mutex);
+        LockGuard lock(concurrent_synchronous_requests_mutex);
         ++concurrent_synchronous_requests;
       }
       io_service->run();
       {
-        std::lock_guard<std::mutex> lock(concurrent_synchronous_requests_mutex);
+        LockGuard lock(concurrent_synchronous_requests_mutex);
         --concurrent_synchronous_requests;
         if(!concurrent_synchronous_requests)
           restart(*io_service);
@@ -266,7 +266,7 @@ namespace SimpleWeb {
       session->callback = [this, session_weak, request_callback](const error_code &ec) {
         if(auto session = session_weak.lock()) {
           {
-            std::lock_guard<std::mutex> lock(this->connections_mutex);
+            LockGuard lock(this->connections_mutex);
             if(!session->connection->event_stream)
               session->connection->in_use = false;
 
@@ -341,7 +341,7 @@ namespace SimpleWeb {
       session->callback = [this, session_weak, request_callback](const error_code &ec) {
         if(auto session = session_weak.lock()) {
           {
-            std::lock_guard<std::mutex> lock(this->connections_mutex);
+            LockGuard lock(this->connections_mutex);
             if(!session->connection->event_stream)
               session->connection->in_use = false;
 
@@ -396,7 +396,7 @@ namespace SimpleWeb {
 
     /// Close connections
     void stop() noexcept {
-      std::lock_guard<std::mutex> lock(connections_mutex);
+      LockGuard lock(connections_mutex);
       for(auto it = connections.begin(); it != connections.end();) {
         (*it)->close();
         it = connections.erase(it);
@@ -417,13 +417,13 @@ namespace SimpleWeb {
 
     std::unique_ptr<std::pair<std::string, std::string>> host_port;
 
-    std::unordered_set<std::shared_ptr<Connection>> connections;
+    Mutex connections_mutex;
-    std::mutex connections_mutex;
+    std::unordered_set<std::shared_ptr<Connection>> connections GUARDED_BY(connections_mutex);
 
     std::shared_ptr<ScopeRunner> handler_runner;
 
-    std::size_t concurrent_synchronous_requests = 0;
+    Mutex concurrent_synchronous_requests_mutex;
-    std::mutex concurrent_synchronous_requests_mutex;
+    std::size_t concurrent_synchronous_requests GUARDED_BY(concurrent_synchronous_requests_mutex) = 0;
 
     ClientBase(const std::string &host_port, unsigned short default_port) noexcept : default_port(default_port), handler_runner(new ScopeRunner()) {
       auto parsed_host_port = parse_host_port(host_port, default_port);
@@ -433,7 +433,7 @@ namespace SimpleWeb {
 
     std::shared_ptr<Connection> get_connection() noexcept {
       std::shared_ptr<Connection> connection;
-      std::lock_guard<std::mutex> lock(connections_mutex);
+      LockGuard lock(connections_mutex);
 
       if(!io_service) {
         io_service = std::make_shared<io_context>();
@@ -586,7 +586,7 @@ namespace SimpleWeb {
 
               if(!ec) {
                 {
-                  std::lock_guard<std::mutex> lock(this->connections_mutex);
+                  LockGuard lock(this->connections_mutex);
                   this->connections.erase(session->connection);
                 }
                 session->callback(ec);
@@ -616,7 +616,7 @@ namespace SimpleWeb {
         }
         else {
           if(session->connection->attempt_reconnect && ec != error::operation_aborted) {
-            std::unique_lock<std::mutex> lock(connections_mutex);
+            LockGuard lock(connections_mutex);
             auto it = connections.find(session->connection);
             if(it != connections.end()) {
               connections.erase(it);

mutex.hpp

@@ -0,0 +1,106 @@
+// Based on https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
+#ifndef SIMPLE_WEB_MUTEX_HPP
+#define SIMPLE_WEB_MUTEX_HPP
+
+#include <mutex>
+
+// Enable thread safety attributes only with clang.
+#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)
+
+namespace SimpleWeb {
+  /// Defines an annotated interface for mutexes.
+  class CAPABILITY("mutex") Mutex {
+    std::mutex mutex;
+
+  public:
+    void lock() ACQUIRE() {
+      mutex.lock();
+    }
+
+    void unlock() RELEASE() {
+      mutex.unlock();
+    }
+  };
+
+  class SCOPED_CAPABILITY LockGuard {
+    Mutex &mutex;
+    bool locked = true;
+
+  public:
+    LockGuard(Mutex &mutex_) ACQUIRE(mutex_) : mutex(mutex_) {
+      mutex.lock();
+    }
+    void unlock() RELEASE() {
+      mutex.unlock();
+      locked = false;
+    }
+    ~LockGuard() RELEASE() {
+      if(locked)
+        mutex.unlock();
+    }
+  };
+
+} // namespace SimpleWeb
+
+#endif // SIMPLE_WEB_MUTEX_HPP

server_http.hpp

@@ -2,13 +2,13 @@
 #define SERVER_HTTP_HPP
 
 #include "asio_compatibility.hpp"
+#include "mutex.hpp"
 #include "utility.hpp"
 #include <functional>
 #include <iostream>
 #include <limits>
 #include <list>
 #include <map>
-#include <mutex>
 #include <sstream>
 #include <thread>
 #include <unordered_set>
@@ -45,8 +45,8 @@ namespace SimpleWeb {
       std::shared_ptr<Session> session;
       long timeout_content;
 
-      std::mutex send_queue_mutex;
+      Mutex send_queue_mutex;
-      std::list<std::pair<std::shared_ptr<asio::streambuf>, std::function<void(const error_code &)>>> send_queue;
+      std::list<std::pair<std::shared_ptr<asio::streambuf>, std::function<void(const error_code &)>>> send_queue GUARDED_BY(send_queue_mutex);
 
       Response(std::shared_ptr<Session> session_, long timeout_content) noexcept : std::ostream(nullptr), session(std::move(session_)), timeout_content(timeout_content) {
         rdbuf(streambuf.get());
@@ -70,15 +70,14 @@ namespace SimpleWeb {
           *this << "\r\n";
       }
 
-      /// send_queue_mutex must be locked here
+      void send_from_queue() REQUIRES(send_queue_mutex) {
-      void send_from_queue() {
         auto self = this->shared_from_this();
         asio::async_write(*self->session->connection->socket, *send_queue.begin()->first, [self](const error_code &ec, std::size_t /*bytes_transferred*/) {
           auto lock = self->session->connection->handler_runner->continue_lock();
           if(!lock)
             return;
           {
-            std::unique_lock<std::mutex> lock(self->send_queue_mutex);
+            LockGuard lock(self->send_queue_mutex);
             if(!ec) {
               auto it = self->send_queue.begin();
               auto callback = std::move(it->second);
@@ -133,7 +132,7 @@ namespace SimpleWeb {
         this->streambuf = std::unique_ptr<asio::streambuf>(new asio::streambuf());
         rdbuf(this->streambuf.get());
 
-        std::lock_guard<std::mutex> lock(send_queue_mutex);
+        LockGuard lock(send_queue_mutex);
         send_queue.emplace_back(streambuf, callback);
         if(send_queue.size() == 1)
           send_from_queue();
@@ -451,10 +450,10 @@ namespace SimpleWeb {
         acceptor->close(ec);
 
         {
-          std::lock_guard<std::mutex> lock(*connections_mutex);
+          LockGuard lock(connections->mutex);
-          for(auto &connection : *connections)
+          for(auto &connection : connections->set)
             connection->close();
-          connections->clear();
+          connections->set.clear();
         }
 
         if(internal_io_service)
@@ -473,12 +472,15 @@ namespace SimpleWeb {
     std::unique_ptr<asio::ip::tcp::acceptor> acceptor;
     std::vector<std::thread> threads;
 
-    std::shared_ptr<std::unordered_set<Connection *>> connections;
+    struct Connections {
-    std::shared_ptr<std::mutex> connections_mutex;
+      Mutex mutex;
+      std::unordered_set<Connection *> set GUARDED_BY(mutex);
+    };
+    std::shared_ptr<Connections> connections;
 
     std::shared_ptr<ScopeRunner> handler_runner;
 
-    ServerBase(unsigned short port) noexcept : config(port), connections(new std::unordered_set<Connection *>()), connections_mutex(new std::mutex()), handler_runner(new ScopeRunner()) {}
+    ServerBase(unsigned short port) noexcept : config(port), connections(new Connections()), handler_runner(new ScopeRunner()) {}
 
     virtual void after_bind() {}
     virtual void accept() = 0;
@@ -486,19 +488,18 @@ namespace SimpleWeb {
     template <typename... Args>
     std::shared_ptr<Connection> create_connection(Args &&... args) noexcept {
       auto connections = this->connections;
-      auto connections_mutex = this->connections_mutex;
+      auto connection = std::shared_ptr<Connection>(new Connection(handler_runner, std::forward<Args>(args)...), [connections](Connection *connection) {
-      auto connection = std::shared_ptr<Connection>(new Connection(handler_runner, std::forward<Args>(args)...), [connections, connections_mutex](Connection *connection) {
         {
-          std::lock_guard<std::mutex> lock(*connections_mutex);
+          LockGuard lock(connections->mutex);
-          auto it = connections->find(connection);
+          auto it = connections->set.find(connection);
-          if(it != connections->end())
+          if(it != connections->set.end())
-            connections->erase(it);
+            connections->set.erase(it);
         }
         delete connection;
       });
       {
-        std::lock_guard<std::mutex> lock(*connections_mutex);
+        LockGuard lock(connections->mutex);
-        connections->emplace(connection.get());
+        connections->set.emplace(connection.get());
       }
       return connection;
     }
@@ -684,10 +685,10 @@ namespace SimpleWeb {
         if(it != session->request->header.end()) {
           // remove connection from connections
           {
-            std::lock_guard<std::mutex> lock(*connections_mutex);
+            LockGuard lock(connections->mutex);
-            auto it = connections->find(session->connection.get());
+            auto it = connections->set.find(session->connection.get());
-            if(it != connections->end())
+            if(it != connections->set.end())
-              connections->erase(it);
+              connections->set.erase(it);
           }
 
           on_upgrade(session->connection->socket, session->request);

tests/CMakeLists.txt

@@ -1,5 +1,8 @@
 if(NOT MSVC)
     add_compile_options(-fno-access-control)
+    if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+        add_compile_options(-Wno-thread-safety)
+    endif()
     
     add_executable(io_test io_test.cpp)
     target_link_libraries(io_test simple-web-server)