Skip to content

Commit 5478d6c

Browse files
committed
logging: thread safety annotations
Adds LockGuard helper in threadsafety.h to replace lock_guard<mutex> when LOCK(Mutex) isn't available for use.
1 parent e685ca1 commit 5478d6c

File tree

3 files changed

+23
-10
lines changed

3 files changed

+23
-10
lines changed

src/logging.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ static int FileWriteStr(const std::string &str, FILE *fp)
4141

4242
bool BCLog::Logger::StartLogging()
4343
{
44-
std::lock_guard<std::mutex> scoped_lock(m_cs);
44+
LockGuard scoped_lock(m_cs);
4545

4646
assert(m_buffering);
4747
assert(m_fileout == nullptr);
@@ -80,7 +80,7 @@ bool BCLog::Logger::StartLogging()
8080

8181
void BCLog::Logger::DisconnectTestLogger()
8282
{
83-
std::lock_guard<std::mutex> scoped_lock(m_cs);
83+
LockGuard scoped_lock(m_cs);
8484
m_buffering = true;
8585
if (m_fileout != nullptr) fclose(m_fileout);
8686
m_fileout = nullptr;
@@ -246,7 +246,7 @@ namespace BCLog {
246246

247247
void BCLog::Logger::LogPrintStr(const std::string& str)
248248
{
249-
std::lock_guard<std::mutex> scoped_lock(m_cs);
249+
LockGuard scoped_lock(m_cs);
250250
std::string str_prefixed = LogEscapeMessage(str);
251251

252252
if (m_log_threadnames && m_started_new_line) {

src/logging.h

+9-7
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include <fs.h>
1010
#include <tinyformat.h>
11+
#include <threadsafety.h>
1112
#include <util/string.h>
1213

1314
#include <atomic>
@@ -62,9 +63,10 @@ namespace BCLog {
6263
{
6364
private:
6465
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
65-
FILE* m_fileout = nullptr; // GUARDED_BY(m_cs)
66-
std::list<std::string> m_msgs_before_open; // GUARDED_BY(m_cs)
67-
bool m_buffering{true}; //!< Buffer messages before logging can be started. GUARDED_BY(m_cs)
66+
67+
FILE* m_fileout GUARDED_BY(m_cs) = nullptr;
68+
std::list<std::string> m_msgs_before_open GUARDED_BY(m_cs);
69+
bool m_buffering GUARDED_BY(m_cs) = true; //!< Buffer messages before logging can be started.
6870

6971
/**
7072
* m_started_new_line is a state variable that will suppress printing of
@@ -79,7 +81,7 @@ namespace BCLog {
7981
std::string LogTimestampStr(const std::string& str);
8082

8183
/** Slots that connect to the print signal */
82-
std::list<std::function<void(const std::string&)>> m_print_callbacks /* GUARDED_BY(m_cs) */ {};
84+
std::list<std::function<void(const std::string&)>> m_print_callbacks GUARDED_BY(m_cs) {};
8385

8486
public:
8587
bool m_print_to_console = false;
@@ -98,22 +100,22 @@ namespace BCLog {
98100
/** Returns whether logs will be written to any output */
99101
bool Enabled() const
100102
{
101-
std::lock_guard<std::mutex> scoped_lock(m_cs);
103+
LockGuard scoped_lock(m_cs);
102104
return m_buffering || m_print_to_console || m_print_to_file || !m_print_callbacks.empty();
103105
}
104106

105107
/** Connect a slot to the print signal and return the connection */
106108
std::list<std::function<void(const std::string&)>>::iterator PushBackCallback(std::function<void(const std::string&)> fun)
107109
{
108-
std::lock_guard<std::mutex> scoped_lock(m_cs);
110+
LockGuard scoped_lock(m_cs);
109111
m_print_callbacks.push_back(std::move(fun));
110112
return --m_print_callbacks.end();
111113
}
112114

113115
/** Delete a connection */
114116
void DeleteCallback(std::list<std::function<void(const std::string&)>>::iterator it)
115117
{
116-
std::lock_guard<std::mutex> scoped_lock(m_cs);
118+
LockGuard scoped_lock(m_cs);
117119
m_print_callbacks.erase(it);
118120
}
119121

src/threadsafety.h

+11
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#ifndef BITCOIN_THREADSAFETY_H
77
#define BITCOIN_THREADSAFETY_H
88

9+
#include <mutex>
10+
911
#ifdef __clang__
1012
// TL;DR Add GUARDED_BY(mutex) to member variables. The others are
1113
// rarely necessary. Ex: int nFoo GUARDED_BY(cs_foo);
@@ -54,4 +56,13 @@
5456
#define ASSERT_EXCLUSIVE_LOCK(...)
5557
#endif // __GNUC__
5658

59+
// LockGuard provides an annotated version of lock_guard for us
60+
// should only be used when sync.h Mutex/LOCK/etc aren't usable
61+
class SCOPED_LOCKABLE LockGuard : public std::lock_guard<std::mutex>
62+
{
63+
public:
64+
explicit LockGuard(std::mutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard<std::mutex>(cs) { }
65+
~LockGuard() UNLOCK_FUNCTION() {};
66+
};
67+
5768
#endif // BITCOIN_THREADSAFETY_H

0 commit comments

Comments
 (0)