Merge #16127: More thread safety annotation coverage

5478d6c099e76fe070703cc5383cba7b91468b0f logging: thread safety annotations (Anthony Towns)
e685ca19928eec4e687c66f5edfcfff085a42c27 util/system.cpp: add thread safety annotations for dir_locks (Anthony Towns)
a7887899480db72328784009181d93904e6d479d test/checkqueue_tests: thread safety annotations (Anthony Towns)
479c5846f7477625ec275fbb8a076c7ef157172b rpc/blockchain.cpp: thread safety annotations for latestblock (Anthony Towns)
8b5af3d4c1270267ad85e78f661bf8fab06f3aad net: fMsgProcWake use LOCK instead of lock_guard (Anthony Towns)
de7c5f41aba860751ef7824245e6d9d5088a1200 wallet/wallet.h: Remove mutexScanning which was only protecting a single atomic bool (Anthony Towns)
c3cf2f55013c4ea1c1ef4a878fc7ff8e92f2c42d rpc/blockchain.cpp: Remove g_utxosetscan mutex that is only protecting a single atomic variable (Anthony Towns)

Pull request description:

  In a few cases we need to use `std::mutex` rather than the sync.h primitives. But `std::lock_guard<std::mutex>` doesn't include the clang thread safety annotations unless you also use clang's C library, which means you can't indicate when variables should be guarded by `std::mutex` mutexes.

  This adds an annotated version of `std::lock_guard<std::mutex>` to threadsafety.h to fix that, and modifies places where `std::mutex` is used to take advantage of the annotations.

  It's based on top of #16112, and turns the thread safety comments included there into annotations.

  It also changes the RAII classes in wallet/wallet.h and rpc/blockchain.cpp to just use the atomic<bool> flag for synchronisation rather than having a mutex that doesn't actually guard anything as well.

ACKs for top commit:
  MarcoFalke:
    ACK 5478d6c099e76fe070703cc5383cba7b91468b0f 🗾
  hebasto:
    re-ACK 5478d6c099e76fe070703cc5383cba7b91468b0f, only renamed s/`MutexGuard`/`LockGuard`/, and dropped the commit "test/util_threadnames_tests: add thread safety annotations" since the [previous](https://github.com/bitcoin/bitcoin/pull/16127#pullrequestreview-414184113) review.
  ryanofsky:
    Code review ACK 5478d6c099e76fe070703cc5383cba7b91468b0f. Thanks for taking suggestions! Only changes since last review are dropping thread rename test commit d53072ec730d8eec5a5b72f7e65a54b141e62b19 and renaming mutex guard to lock guard

Tree-SHA512: 7b00d31f6f2b5a222ec69431eb810a74abf0542db3a65d1bbad54e354c40df2857ec89c00b4a5e466c81ba223267ca95f3f98d5fbc1a1d052a2c3a7d2209790a

1 files changed, 9 insertions, 7 deletions

diff --git a/src/logging.h b/src/logging.h
index ab07010316..c55f581916 100644
--- 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,14 +100,14 @@ 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();
         }
@@ -113,7 +115,7 @@ namespace BCLog {
         /** 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);
         }