mirror of
https://github.com/dashpay/dash.git
synced 2024-12-25 12:02:48 +01:00
merge #16127: more thread safety annotation coverage
This commit is contained in:
parent
8bc1cae8e2
commit
d559983eeb
@ -33,7 +33,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);
|
||||
@ -263,7 +263,7 @@ std::string BCLog::Logger::LogThreadNameStr(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 strThreadLogged = LogThreadNameStr(str);
|
||||
std::string strTimestamped = LogTimestampStr(strThreadLogged);
|
||||
|
||||
|
@ -8,6 +8,7 @@
|
||||
|
||||
#include <fs.h>
|
||||
#include <tinyformat.h>
|
||||
#include <threadsafety.h>
|
||||
|
||||
#include <atomic>
|
||||
#include <cstdint>
|
||||
@ -83,9 +84,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
|
||||
@ -117,7 +119,7 @@ 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;
|
||||
}
|
||||
|
||||
|
@ -2040,7 +2040,7 @@ void CConnman::ThreadSocketHandler()
|
||||
void CConnman::WakeMessageHandler()
|
||||
{
|
||||
{
|
||||
std::lock_guard<std::mutex> lock(mutexMsgProc);
|
||||
LOCK(mutexMsgProc);
|
||||
fMsgProcWake = true;
|
||||
}
|
||||
condMsgProc.notify_one();
|
||||
@ -2864,7 +2864,7 @@ void CConnman::ThreadMessageHandler()
|
||||
|
||||
WAIT_LOCK(mutexMsgProc, lock);
|
||||
if (!fMoreWork) {
|
||||
condMsgProc.wait_until(lock, std::chrono::steady_clock::now() + std::chrono::milliseconds(100), [this] { return fMsgProcWake; });
|
||||
condMsgProc.wait_until(lock, std::chrono::steady_clock::now() + std::chrono::milliseconds(100), [this]() EXCLUSIVE_LOCKS_REQUIRED(mutexMsgProc) { return fMsgProcWake; });
|
||||
}
|
||||
fMsgProcWake = false;
|
||||
}
|
||||
@ -3303,7 +3303,7 @@ void CExplicitNetCleanup::callCleanup()
|
||||
void CConnman::Interrupt()
|
||||
{
|
||||
{
|
||||
std::lock_guard<std::mutex> lock(mutexMsgProc);
|
||||
LOCK(mutexMsgProc);
|
||||
flagInterruptMsgProc = true;
|
||||
}
|
||||
condMsgProc.notify_all();
|
||||
|
@ -617,7 +617,7 @@ private:
|
||||
const uint64_t nSeed0, nSeed1;
|
||||
|
||||
/** flag for waking the message processor. */
|
||||
bool fMsgProcWake;
|
||||
bool fMsgProcWake GUARDED_BY(mutexMsgProc);
|
||||
|
||||
std::condition_variable condMsgProc;
|
||||
Mutex mutexMsgProc;
|
||||
|
@ -55,7 +55,7 @@ struct CUpdatedBlock
|
||||
|
||||
static Mutex cs_blockchange;
|
||||
static std::condition_variable cond_blockchange;
|
||||
static CUpdatedBlock latestblock;
|
||||
static CUpdatedBlock latestblock GUARDED_BY(cs_blockchange);
|
||||
|
||||
extern void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry);
|
||||
|
||||
@ -247,7 +247,7 @@ static UniValue getbestchainlock(const JSONRPCRequest& request)
|
||||
void RPCNotifyBlockChange(bool ibd, const CBlockIndex * pindex)
|
||||
{
|
||||
if(pindex) {
|
||||
std::lock_guard<std::mutex> lock(cs_blockchange);
|
||||
LOCK(cs_blockchange);
|
||||
latestblock.hash = pindex->GetBlockHash();
|
||||
latestblock.height = pindex->nHeight;
|
||||
}
|
||||
@ -281,9 +281,9 @@ static UniValue waitfornewblock(const JSONRPCRequest& request)
|
||||
WAIT_LOCK(cs_blockchange, lock);
|
||||
block = latestblock;
|
||||
if(timeout)
|
||||
cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&block]{return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); });
|
||||
cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&block]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); });
|
||||
else
|
||||
cond_blockchange.wait(lock, [&block]{return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); });
|
||||
cond_blockchange.wait(lock, [&block]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); });
|
||||
block = latestblock;
|
||||
}
|
||||
UniValue ret(UniValue::VOBJ);
|
||||
@ -322,9 +322,9 @@ static UniValue waitforblock(const JSONRPCRequest& request)
|
||||
{
|
||||
WAIT_LOCK(cs_blockchange, lock);
|
||||
if(timeout)
|
||||
cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&hash]{return latestblock.hash == hash || !IsRPCRunning();});
|
||||
cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&hash]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.hash == hash || !IsRPCRunning();});
|
||||
else
|
||||
cond_blockchange.wait(lock, [&hash]{return latestblock.hash == hash || !IsRPCRunning(); });
|
||||
cond_blockchange.wait(lock, [&hash]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.hash == hash || !IsRPCRunning(); });
|
||||
block = latestblock;
|
||||
}
|
||||
|
||||
@ -365,11 +365,12 @@ static UniValue waitforblockheight(const JSONRPCRequest& request)
|
||||
{
|
||||
WAIT_LOCK(cs_blockchange, lock);
|
||||
if(timeout)
|
||||
cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&height]{return latestblock.height >= height || !IsRPCRunning();});
|
||||
cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&height]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height >= height || !IsRPCRunning();});
|
||||
else
|
||||
cond_blockchange.wait(lock, [&height]{return latestblock.height >= height || !IsRPCRunning(); });
|
||||
cond_blockchange.wait(lock, [&height]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height >= height || !IsRPCRunning(); });
|
||||
block = latestblock;
|
||||
}
|
||||
// TODO: Backport g_utxosetscan and associated logic from #16127
|
||||
UniValue ret(UniValue::VOBJ);
|
||||
ret.pushKV("hash", block.hash.GetHex());
|
||||
ret.pushKV("height", block.height);
|
||||
|
@ -170,7 +170,7 @@ static __attribute__((noinline)) std::vector<uint64_t> GetStackFrames(size_t ski
|
||||
|
||||
// dbghelp is not thread safe
|
||||
static std::mutex m;
|
||||
std::lock_guard<std::mutex> l(m);
|
||||
LockGuard l(m);
|
||||
|
||||
HANDLE process = GetCurrentProcess();
|
||||
HANDLE thread = GetCurrentThread();
|
||||
@ -594,7 +594,7 @@ extern "C" void* __attribute__((noinline)) WRAPPED_NAME(__cxa_allocate_exception
|
||||
|
||||
void* p = __real___cxa_allocate_exception(thrown_size);
|
||||
|
||||
std::lock_guard<std::mutex> l(g_stacktraces_mutex);
|
||||
LockGuard l(g_stacktraces_mutex);
|
||||
g_stacktraces.emplace(p, st);
|
||||
return p;
|
||||
}
|
||||
@ -603,7 +603,7 @@ extern "C" void __attribute__((noinline)) WRAPPED_NAME(__cxa_free_exception)(voi
|
||||
{
|
||||
__real___cxa_free_exception(thrown_exception);
|
||||
|
||||
std::lock_guard<std::mutex> l(g_stacktraces_mutex);
|
||||
LockGuard l(g_stacktraces_mutex);
|
||||
g_stacktraces.erase(thrown_exception);
|
||||
}
|
||||
|
||||
@ -667,7 +667,7 @@ static std::shared_ptr<std::vector<uint64_t>> GetExceptionStacktrace(const std::
|
||||
#ifdef ENABLE_CRASH_HOOKS
|
||||
void* p = *(void**)&e;
|
||||
|
||||
std::lock_guard<std::mutex> l(g_stacktraces_mutex);
|
||||
LockGuard l(g_stacktraces_mutex);
|
||||
auto it = g_stacktraces.find(p);
|
||||
if (it == g_stacktraces.end()) {
|
||||
return nullptr;
|
||||
|
@ -2,6 +2,7 @@
|
||||
// Distributed under the MIT software license, see the accompanying
|
||||
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
|
||||
|
||||
#include <sync.h>
|
||||
#include <util.h>
|
||||
#include <utiltime.h>
|
||||
#include <validation.h>
|
||||
@ -59,14 +60,14 @@ struct FailingCheck {
|
||||
};
|
||||
|
||||
struct UniqueCheck {
|
||||
static std::mutex m;
|
||||
static std::unordered_multiset<size_t> results;
|
||||
static Mutex m;
|
||||
static std::unordered_multiset<size_t> results GUARDED_BY(m);
|
||||
size_t check_id;
|
||||
UniqueCheck(size_t check_id_in) : check_id(check_id_in){};
|
||||
UniqueCheck() : check_id(0){};
|
||||
bool operator()()
|
||||
{
|
||||
std::lock_guard<std::mutex> l(m);
|
||||
LOCK(m);
|
||||
results.insert(check_id);
|
||||
return true;
|
||||
}
|
||||
@ -129,7 +130,7 @@ struct FrozenCleanupCheck {
|
||||
std::mutex FrozenCleanupCheck::m{};
|
||||
std::atomic<uint64_t> FrozenCleanupCheck::nFrozen{0};
|
||||
std::condition_variable FrozenCleanupCheck::cv{};
|
||||
std::mutex UniqueCheck::m;
|
||||
Mutex UniqueCheck::m;
|
||||
std::unordered_multiset<size_t> UniqueCheck::results;
|
||||
std::atomic<size_t> FakeCheckCheckCompletion::n_calls{0};
|
||||
std::atomic<size_t> MemoryCheck::fake_allocated_memory{0};
|
||||
@ -293,11 +294,15 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck)
|
||||
control.Add(vChecks);
|
||||
}
|
||||
}
|
||||
{
|
||||
LOCK(UniqueCheck::m);
|
||||
bool r = true;
|
||||
BOOST_REQUIRE_EQUAL(UniqueCheck::results.size(), COUNT);
|
||||
for (size_t i = 0; i < COUNT; ++i)
|
||||
for (size_t i = 0; i < COUNT; ++i) {
|
||||
r = r && UniqueCheck::results.count(i) == 1;
|
||||
}
|
||||
BOOST_REQUIRE(r);
|
||||
}
|
||||
tg.interrupt_all();
|
||||
tg.join_all();
|
||||
}
|
||||
@ -442,4 +447,3 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks)
|
||||
}
|
||||
}
|
||||
BOOST_AUTO_TEST_SUITE_END()
|
||||
|
||||
|
@ -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
|
||||
|
11
src/util.cpp
11
src/util.cpp
@ -153,18 +153,19 @@ public:
|
||||
}
|
||||
instance_of_cinit;
|
||||
|
||||
/** Mutex to protect dir_locks. */
|
||||
static Mutex cs_dir_locks;
|
||||
|
||||
/** A map that contains all the currently held directory locks. After
|
||||
* successful locking, these will be held here until the global destructor
|
||||
* cleans them up and thus automatically unlocks them, or ReleaseDirectoryLocks
|
||||
* is called.
|
||||
*/
|
||||
static std::map<std::string, std::unique_ptr<fsbridge::FileLock>> dir_locks;
|
||||
/** Mutex to protect dir_locks. */
|
||||
static std::mutex cs_dir_locks;
|
||||
static std::map<std::string, std::unique_ptr<fsbridge::FileLock>> dir_locks GUARDED_BY(cs_dir_locks);
|
||||
|
||||
bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only)
|
||||
{
|
||||
std::lock_guard<std::mutex> ulock(cs_dir_locks);
|
||||
LOCK(cs_dir_locks);
|
||||
fs::path pathLockFile = directory / lockfile_name;
|
||||
|
||||
// If a lock for this directory already exists in the map, don't try to re-lock it
|
||||
@ -188,7 +189,7 @@ bool LockDirectory(const fs::path& directory, const std::string lockfile_name, b
|
||||
|
||||
void ReleaseDirectoryLocks()
|
||||
{
|
||||
std::lock_guard<std::mutex> ulock(cs_dir_locks);
|
||||
LOCK(cs_dir_locks);
|
||||
dir_locks.clear();
|
||||
}
|
||||
|
||||
|
@ -703,7 +703,6 @@ private:
|
||||
std::atomic<bool> fScanningWallet{false}; // controlled by WalletRescanReserver
|
||||
std::atomic<int64_t> m_scanning_start{0};
|
||||
std::atomic<double> m_scanning_progress{0};
|
||||
std::mutex mutexScanning;
|
||||
friend class WalletRescanReserver;
|
||||
|
||||
WalletBatch *encrypted_batch = nullptr;
|
||||
@ -1332,13 +1331,11 @@ public:
|
||||
bool reserve()
|
||||
{
|
||||
assert(!m_could_reserve);
|
||||
std::lock_guard<std::mutex> lock(m_wallet->mutexScanning);
|
||||
if (m_wallet->fScanningWallet) {
|
||||
if (m_wallet->fScanningWallet.exchange(true)) {
|
||||
return false;
|
||||
}
|
||||
m_wallet->m_scanning_start = GetTimeMillis();
|
||||
m_wallet->m_scanning_progress = 0;
|
||||
m_wallet->fScanningWallet = true;
|
||||
m_could_reserve = true;
|
||||
return true;
|
||||
}
|
||||
@ -1350,7 +1347,6 @@ public:
|
||||
|
||||
~WalletRescanReserver()
|
||||
{
|
||||
std::lock_guard<std::mutex> lock(m_wallet->mutexScanning);
|
||||
if (m_could_reserve) {
|
||||
m_wallet->fScanningWallet = false;
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user