logging: thread safety annotations

Adds LockGuard helper in threadsafety.h to replace lock_guard<mutex>
when LOCK(Mutex) isn't available for use.
This commit is contained in:
Anthony Towns
2020-02-11 16:50:22 +10:00
parent e685ca1992
commit 5478d6c099
3 changed files with 23 additions and 10 deletions

View File

@@ -41,7 +41,7 @@ static int FileWriteStr(const std::string &str, FILE *fp)
bool BCLog::Logger::StartLogging() bool BCLog::Logger::StartLogging()
{ {
std::lock_guard<std::mutex> scoped_lock(m_cs); LockGuard scoped_lock(m_cs);
assert(m_buffering); assert(m_buffering);
assert(m_fileout == nullptr); assert(m_fileout == nullptr);
@@ -80,7 +80,7 @@ bool BCLog::Logger::StartLogging()
void BCLog::Logger::DisconnectTestLogger() void BCLog::Logger::DisconnectTestLogger()
{ {
std::lock_guard<std::mutex> scoped_lock(m_cs); LockGuard scoped_lock(m_cs);
m_buffering = true; m_buffering = true;
if (m_fileout != nullptr) fclose(m_fileout); if (m_fileout != nullptr) fclose(m_fileout);
m_fileout = nullptr; m_fileout = nullptr;
@@ -246,7 +246,7 @@ namespace BCLog {
void BCLog::Logger::LogPrintStr(const std::string& str) 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); std::string str_prefixed = LogEscapeMessage(str);
if (m_log_threadnames && m_started_new_line) { if (m_log_threadnames && m_started_new_line) {

View File

@@ -8,6 +8,7 @@
#include <fs.h> #include <fs.h>
#include <tinyformat.h> #include <tinyformat.h>
#include <threadsafety.h>
#include <util/string.h> #include <util/string.h>
#include <atomic> #include <atomic>
@@ -62,9 +63,10 @@ namespace BCLog {
{ {
private: 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 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) FILE* m_fileout GUARDED_BY(m_cs) = nullptr;
bool m_buffering{true}; //!< Buffer messages before logging can be started. GUARDED_BY(m_cs) 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 * 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); std::string LogTimestampStr(const std::string& str);
/** Slots that connect to the print signal */ /** 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: public:
bool m_print_to_console = false; bool m_print_to_console = false;
@@ -98,14 +100,14 @@ namespace BCLog {
/** Returns whether logs will be written to any output */ /** Returns whether logs will be written to any output */
bool Enabled() const 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(); 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 */ /** 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::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)); m_print_callbacks.push_back(std::move(fun));
return --m_print_callbacks.end(); return --m_print_callbacks.end();
} }
@@ -113,7 +115,7 @@ namespace BCLog {
/** Delete a connection */ /** Delete a connection */
void DeleteCallback(std::list<std::function<void(const std::string&)>>::iterator it) 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); m_print_callbacks.erase(it);
} }

View File

@@ -6,6 +6,8 @@
#ifndef BITCOIN_THREADSAFETY_H #ifndef BITCOIN_THREADSAFETY_H
#define BITCOIN_THREADSAFETY_H #define BITCOIN_THREADSAFETY_H
#include <mutex>
#ifdef __clang__ #ifdef __clang__
// TL;DR Add GUARDED_BY(mutex) to member variables. The others are // TL;DR Add GUARDED_BY(mutex) to member variables. The others are
// rarely necessary. Ex: int nFoo GUARDED_BY(cs_foo); // rarely necessary. Ex: int nFoo GUARDED_BY(cs_foo);
@@ -54,4 +56,13 @@
#define ASSERT_EXCLUSIVE_LOCK(...) #define ASSERT_EXCLUSIVE_LOCK(...)
#endif // __GNUC__ #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 #endif // BITCOIN_THREADSAFETY_H