logging: thread safety annotations

ajtowns · ajtowns · commit 5478d6c099e7 · 2020-05-27T01:31:51.000+10:00
Adds LockGuard helper in threadsafety.h to replace lock_guard&lt;mutex&gt;
when LOCK(Mutex) isn't available for use.
diff --git a/src/logging.cpp b/src/logging.cpp
@@ -41,7 +41,7 @@ static int FileWriteStr(const std::string &str, FILE *fp)
 
 bool BCLog::Logger::StartLogging()
 {
-    std::lock_guard<std::mutex> scoped_lock(m_cs);
+    LockGuard scoped_lock(m_cs);
 
     assert(m_buffering);
     assert(m_fileout == nullptr);
@@ -80,7 +80,7 @@ bool BCLog::Logger::StartLogging()
 
 void BCLog::Logger::DisconnectTestLogger()
 {
-    std::lock_guard<std::mutex> scoped_lock(m_cs);
+    LockGuard scoped_lock(m_cs);
     m_buffering = true;
     if (m_fileout != nullptr) fclose(m_fileout);
     m_fileout = nullptr;
@@ -246,7 +246,7 @@ namespace BCLog {
 
 void BCLog::Logger::LogPrintStr(const std::string& str)
 {
-    std::lock_guard<std::mutex> scoped_lock(m_cs);
+    LockGuard scoped_lock(m_cs);
     std::string str_prefixed = LogEscapeMessage(str);
 
     if (m_log_threadnames && m_started_new_line) {
diff --git a/src/logging.h b/src/logging.h
@@ -8,6 +8,7 @@
 
 #include <fs.h>
 #include <tinyformat.h>
+#include <threadsafety.h>
 #include <util/string.h>
 
 #include <atomic>
@@ -62,9 +63,10 @@ namespace BCLog {
     {
     private:
         mutable std::mutex m_cs;                   // Can not use Mutex from sync.h because in debug mode it would cause a deadlock when a potential deadlock was detected
-        FILE* m_fileout = nullptr;                 // GUARDED_BY(m_cs)
-        std::list<std::string> m_msgs_before_open; // GUARDED_BY(m_cs)
-        bool m_buffering{true};                    //!< Buffer messages before logging can be started. GUARDED_BY(m_cs)
+
+        FILE* m_fileout GUARDED_BY(m_cs) = nullptr;
+        std::list<std::string> m_msgs_before_open GUARDED_BY(m_cs);
+        bool m_buffering GUARDED_BY(m_cs) = true; //!< Buffer messages before logging can be started.
 
         /**
          * m_started_new_line is a state variable that will suppress printing of
@@ -79,7 +81,7 @@ namespace BCLog {
         std::string LogTimestampStr(const std::string& str);
 
         /** Slots that connect to the print signal */
-        std::list<std::function<void(const std::string&)>> m_print_callbacks /* GUARDED_BY(m_cs) */ {};
+        std::list<std::function<void(const std::string&)>> m_print_callbacks GUARDED_BY(m_cs) {};
 
     public:
         bool m_print_to_console = false;
@@ -98,22 +100,22 @@ namespace BCLog {
         /** Returns whether logs will be written to any output */
         bool Enabled() const
         {
-            std::lock_guard<std::mutex> scoped_lock(m_cs);
+            LockGuard scoped_lock(m_cs);
             return m_buffering || m_print_to_console || m_print_to_file || !m_print_callbacks.empty();
         }
 
         /** Connect a slot to the print signal and return the connection */
         std::list<std::function<void(const std::string&)>>::iterator PushBackCallback(std::function<void(const std::string&)> fun)
         {
-            std::lock_guard<std::mutex> scoped_lock(m_cs);
+            LockGuard scoped_lock(m_cs);
             m_print_callbacks.push_back(std::move(fun));
             return --m_print_callbacks.end();
         }
 
         /** Delete a connection */
         void DeleteCallback(std::list<std::function<void(const std::string&)>>::iterator it)
         {
-            std::lock_guard<std::mutex> scoped_lock(m_cs);
+            LockGuard scoped_lock(m_cs);
             m_print_callbacks.erase(it);
         }
 
diff --git a/src/threadsafety.h b/src/threadsafety.h
@@ -6,6 +6,8 @@
 #ifndef BITCOIN_THREADSAFETY_H
 #define BITCOIN_THREADSAFETY_H
 
+#include <mutex>
+
 #ifdef __clang__
 // TL;DR Add GUARDED_BY(mutex) to member variables. The others are
 // rarely necessary. Ex: int nFoo GUARDED_BY(cs_foo);
@@ -54,4 +56,13 @@
 #define ASSERT_EXCLUSIVE_LOCK(...)
 #endif // __GNUC__
 
+// LockGuard provides an annotated version of lock_guard for us
+// should only be used when sync.h Mutex/LOCK/etc aren't usable
+class SCOPED_LOCKABLE LockGuard : public std::lock_guard<std::mutex>
+{
+public:
+    explicit LockGuard(std::mutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard<std::mutex>(cs) { }
+    ~LockGuard() UNLOCK_FUNCTION() {};
+};
+
 #endif // BITCOIN_THREADSAFETY_H
