aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--doc/tor.md7
-rw-r--r--src/Makefile.test.include1
-rw-r--r--src/httpserver.cpp8
-rw-r--r--src/init.cpp12
-rw-r--r--src/net.cpp6
-rw-r--r--src/net.h2
-rw-r--r--src/random.cpp7
-rw-r--r--src/rpc/blockchain.cpp8
-rw-r--r--src/rpc/blockchain.h3
-rw-r--r--src/rpc/mining.cpp2
-rw-r--r--src/sync.cpp8
-rw-r--r--src/sync.h112
-rw-r--r--src/test/scheduler_tests.cpp6
-rw-r--r--src/test/sync_tests.cpp52
-rw-r--r--src/threadinterrupt.cpp6
-rw-r--r--src/threadinterrupt.h4
-rw-r--r--src/threadsafety.h2
-rw-r--r--src/validation.cpp6
-rw-r--r--src/validation.h4
-rw-r--r--src/wallet/test/wallet_tests.cpp16
-rw-r--r--src/wallet/wallet.cpp12
-rw-r--r--src/wallet/wallet.h2
-rwxr-xr-xtest/functional/rpc_help.py10
-rwxr-xr-xtest/functional/test_framework/test_framework.py9
-rwxr-xr-xtest/lint/lint-assertions.sh23
25 files changed, 218 insertions, 110 deletions
diff --git a/doc/tor.md b/doc/tor.md
index 2d0676c89a..dc0b88618a 100644
--- a/doc/tor.md
+++ b/doc/tor.md
@@ -93,7 +93,7 @@ API, to create and destroy 'ephemeral' hidden services programmatically.
Bitcoin Core has been updated to make use of this.
This means that if Tor is running (and proper authentication has been configured),
-Bitcoin Core automatically creates a hidden service to listen on. This will positively
+Bitcoin Core automatically creates a hidden service to listen on. This will positively
affect the number of available .onion nodes.
This new feature is enabled by default if Bitcoin Core is listening (`-listen`), and
@@ -102,8 +102,9 @@ and, if not disabled, configured using the `-torcontrol` and `-torpassword` sett
To show verbose debugging information, pass `-debug=tor`.
Connecting to Tor's control socket API requires one of two authentication methods to be
-configured. For cookie authentication the user running bitcoind must have write access
-to the `CookieAuthFile` specified in Tor configuration. In some cases, this is
+configured. It also requires the control socket to be enabled, e.g. put `ControlPort 9051`
+in `torrc` config file. For cookie authentication the user running bitcoind must have read
+access to the `CookieAuthFile` specified in Tor configuration. In some cases this is
preconfigured and the creation of a hidden service is automatic. If permission problems
are seen with `-debug=tor` they can be resolved by adding both the user running Tor and
the user running bitcoind to the same group and setting permissions appropriately. On
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index abfd66efad..019799eb0b 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -83,6 +83,7 @@ BITCOIN_TESTS =\
test/sigopcount_tests.cpp \
test/skiplist_tests.cpp \
test/streams_tests.cpp \
+ test/sync_tests.cpp \
test/timedata_tests.cpp \
test/torcontrol_tests.cpp \
test/transaction_tests.cpp \
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 0fdc3e5ff3..326f7f6b64 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -69,7 +69,7 @@ class WorkQueue
{
private:
/** Mutex protects entire object */
- std::mutex cs;
+ Mutex cs;
std::condition_variable cond;
std::deque<std::unique_ptr<WorkItem>> queue;
bool running;
@@ -88,7 +88,7 @@ public:
/** Enqueue a work item */
bool Enqueue(WorkItem* item)
{
- std::unique_lock<std::mutex> lock(cs);
+ LOCK(cs);
if (queue.size() >= maxDepth) {
return false;
}
@@ -102,7 +102,7 @@ public:
while (true) {
std::unique_ptr<WorkItem> i;
{
- std::unique_lock<std::mutex> lock(cs);
+ WAIT_LOCK(cs, lock);
while (running && queue.empty())
cond.wait(lock);
if (!running)
@@ -116,7 +116,7 @@ public:
/** Interrupt and exit loops */
void Interrupt()
{
- std::unique_lock<std::mutex> lock(cs);
+ LOCK(cs);
running = false;
cond.notify_all();
}
diff --git a/src/init.cpp b/src/init.cpp
index bd330459f6..27966319d6 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -561,17 +561,17 @@ static void BlockNotifyCallback(bool initialSync, const CBlockIndex *pBlockIndex
}
static bool fHaveGenesis = false;
-static CWaitableCriticalSection cs_GenesisWait;
-static CConditionVariable condvar_GenesisWait;
+static Mutex g_genesis_wait_mutex;
+static std::condition_variable g_genesis_wait_cv;
static void BlockNotifyGenesisWait(bool, const CBlockIndex *pBlockIndex)
{
if (pBlockIndex != nullptr) {
{
- WaitableLock lock_GenesisWait(cs_GenesisWait);
+ LOCK(g_genesis_wait_mutex);
fHaveGenesis = true;
}
- condvar_GenesisWait.notify_all();
+ g_genesis_wait_cv.notify_all();
}
}
@@ -1661,12 +1661,12 @@ bool AppInitMain()
// Wait for genesis block to be processed
{
- WaitableLock lock(cs_GenesisWait);
+ WAIT_LOCK(g_genesis_wait_mutex, lock);
// We previously could hang here if StartShutdown() is called prior to
// ThreadImport getting started, so instead we just wait on a timer to
// check ShutdownRequested() regularly.
while (!fHaveGenesis && !ShutdownRequested()) {
- condvar_GenesisWait.wait_for(lock, std::chrono::milliseconds(500));
+ g_genesis_wait_cv.wait_for(lock, std::chrono::milliseconds(500));
}
uiInterface.NotifyBlockTip_disconnect(BlockNotifyGenesisWait);
}
diff --git a/src/net.cpp b/src/net.cpp
index df2cb54b7a..e9e9cf4c92 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1227,7 +1227,7 @@ void CConnman::ThreadSocketHandler()
if(vNodesSize != nPrevNodeCount) {
nPrevNodeCount = vNodesSize;
if(clientInterface)
- clientInterface->NotifyNumConnectionsChanged(nPrevNodeCount);
+ clientInterface->NotifyNumConnectionsChanged(vNodesSize);
}
//
@@ -2065,7 +2065,7 @@ void CConnman::ThreadMessageHandler()
pnode->Release();
}
- std::unique_lock<std::mutex> lock(mutexMsgProc);
+ WAIT_LOCK(mutexMsgProc, lock);
if (!fMoreWork) {
condMsgProc.wait_until(lock, std::chrono::steady_clock::now() + std::chrono::milliseconds(100), [this] { return fMsgProcWake; });
}
@@ -2347,7 +2347,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
flagInterruptMsgProc = false;
{
- std::unique_lock<std::mutex> lock(mutexMsgProc);
+ LOCK(mutexMsgProc);
fMsgProcWake = false;
}
diff --git a/src/net.h b/src/net.h
index edd2f8cf0d..9f6c426ab7 100644
--- a/src/net.h
+++ b/src/net.h
@@ -427,7 +427,7 @@ private:
bool fMsgProcWake;
std::condition_variable condMsgProc;
- std::mutex mutexMsgProc;
+ Mutex mutexMsgProc;
std::atomic<bool> flagInterruptMsgProc;
CThreadInterrupt interruptNet;
diff --git a/src/random.cpp b/src/random.cpp
index 6c5ad5ac96..503d5b3636 100644
--- a/src/random.cpp
+++ b/src/random.cpp
@@ -12,6 +12,7 @@
#include <wincrypt.h>
#endif
#include <logging.h> // for LogPrint()
+#include <sync.h> // for WAIT_LOCK
#include <utiltime.h> // for GetTime()
#include <stdlib.h>
@@ -295,7 +296,7 @@ void RandAddSeedSleep()
}
-static std::mutex cs_rng_state;
+static Mutex cs_rng_state;
static unsigned char rng_state[32] = {0};
static uint64_t rng_counter = 0;
@@ -305,7 +306,7 @@ static void AddDataToRng(void* data, size_t len) {
hasher.Write((const unsigned char*)data, len);
unsigned char buf[64];
{
- std::unique_lock<std::mutex> lock(cs_rng_state);
+ WAIT_LOCK(cs_rng_state, lock);
hasher.Write(rng_state, sizeof(rng_state));
hasher.Write((const unsigned char*)&rng_counter, sizeof(rng_counter));
++rng_counter;
@@ -337,7 +338,7 @@ void GetStrongRandBytes(unsigned char* out, int num)
// Combine with and update state
{
- std::unique_lock<std::mutex> lock(cs_rng_state);
+ WAIT_LOCK(cs_rng_state, lock);
hasher.Write(rng_state, sizeof(rng_state));
hasher.Write((const unsigned char*)&rng_counter, sizeof(rng_counter));
++rng_counter;
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index a77fea8ea8..f0d767bbfc 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -50,7 +50,7 @@ struct CUpdatedBlock
int height;
};
-static std::mutex cs_blockchange;
+static Mutex cs_blockchange;
static std::condition_variable cond_blockchange;
static CUpdatedBlock latestblock;
@@ -225,7 +225,7 @@ static UniValue waitfornewblock(const JSONRPCRequest& request)
CUpdatedBlock block;
{
- std::unique_lock<std::mutex> lock(cs_blockchange);
+ 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(); });
@@ -267,7 +267,7 @@ static UniValue waitforblock(const JSONRPCRequest& request)
CUpdatedBlock block;
{
- std::unique_lock<std::mutex> lock(cs_blockchange);
+ WAIT_LOCK(cs_blockchange, lock);
if(timeout)
cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&hash]{return latestblock.hash == hash || !IsRPCRunning();});
else
@@ -310,7 +310,7 @@ static UniValue waitforblockheight(const JSONRPCRequest& request)
CUpdatedBlock block;
{
- std::unique_lock<std::mutex> lock(cs_blockchange);
+ WAIT_LOCK(cs_blockchange, lock);
if(timeout)
cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&height]{return latestblock.height >= height || !IsRPCRunning();});
else
diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h
index 544bc62c36..add335eb8a 100644
--- a/src/rpc/blockchain.h
+++ b/src/rpc/blockchain.h
@@ -16,8 +16,7 @@ class UniValue;
static constexpr int NUM_GETBLOCKSTATS_PERCENTILES = 5;
/**
- * Get the difficulty of the net wrt to the given block index, or the chain tip if
- * not provided.
+ * Get the difficulty of the net wrt to the given block index.
*
* @return A floating point number that is a multiple of the main net minimum
* difficulty (4295032833 hashes).
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 623b0bd86a..9cc4e77768 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -470,7 +470,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
{
checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1);
- WaitableLock lock(g_best_block_mutex);
+ WAIT_LOCK(g_best_block_mutex, lock);
while (g_best_block == hashWatchedChain && IsRPCRunning())
{
if (g_best_block_cv.wait_until(lock, checktxtime) == std::cv_status::timeout)
diff --git a/src/sync.cpp b/src/sync.cpp
index 255eb4f00b..c9aa98dcd6 100644
--- a/src/sync.cpp
+++ b/src/sync.cpp
@@ -100,7 +100,11 @@ static void potential_deadlock_detected(const std::pair<void*, void*>& mismatch,
}
LogPrintf(" %s\n", i.second.ToString());
}
- assert(false);
+ if (g_debug_lockorder_abort) {
+ fprintf(stderr, "Assertion failed: detected inconsistent lock order at %s:%i, details in debug log.\n", __FILE__, __LINE__);
+ abort();
+ }
+ throw std::logic_error("potential deadlock detected");
}
static void push_lock(void* c, const CLockLocation& locklocation)
@@ -189,4 +193,6 @@ void DeleteLock(void* cs)
}
}
+bool g_debug_lockorder_abort = true;
+
#endif /* DEBUG_LOCKORDER */
diff --git a/src/sync.h b/src/sync.h
index 11c1253757..40709bdd7f 100644
--- a/src/sync.h
+++ b/src/sync.h
@@ -46,14 +46,42 @@ LEAVE_CRITICAL_SECTION(mutex); // no RAII
// //
///////////////////////////////
+#ifdef DEBUG_LOCKORDER
+void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false);
+void LeaveCritical();
+std::string LocksHeld();
+void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs);
+void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs);
+void DeleteLock(void* cs);
+
/**
- * Template mixin that adds -Wthread-safety locking
- * annotations to a subset of the mutex API.
+ * Call abort() if a potential lock order deadlock bug is detected, instead of
+ * just logging information and throwing a logic_error. Defaults to true, and
+ * set to false in DEBUG_LOCKORDER unit tests.
+ */
+extern bool g_debug_lockorder_abort;
+#else
+void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {}
+void static inline LeaveCritical() {}
+void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs) {}
+void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {}
+void static inline DeleteLock(void* cs) {}
+#endif
+#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs)
+#define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs)
+
+/**
+ * Template mixin that adds -Wthread-safety locking annotations and lock order
+ * checking to a subset of the mutex API.
*/
template <typename PARENT>
class LOCKABLE AnnotatedMixin : public PARENT
{
public:
+ ~AnnotatedMixin() {
+ DeleteLock((void*)this);
+ }
+
void lock() EXCLUSIVE_LOCK_FUNCTION()
{
PARENT::lock();
@@ -68,64 +96,36 @@ public:
{
return PARENT::try_lock();
}
-};
-#ifdef DEBUG_LOCKORDER
-void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false);
-void LeaveCritical();
-std::string LocksHeld();
-void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs);
-void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs);
-void DeleteLock(void* cs);
-#else
-void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {}
-void static inline LeaveCritical() {}
-void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs) {}
-void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {}
-void static inline DeleteLock(void* cs) {}
-#endif
-#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs)
-#define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs)
+ using UniqueLock = std::unique_lock<PARENT>;
+};
/**
* Wrapped mutex: supports recursive locking, but no waiting
* TODO: We should move away from using the recursive lock by default.
*/
-class CCriticalSection : public AnnotatedMixin<std::recursive_mutex>
-{
-public:
- ~CCriticalSection() {
- DeleteLock((void*)this);
- }
-};
+typedef AnnotatedMixin<std::recursive_mutex> CCriticalSection;
/** Wrapped mutex: supports waiting but not recursive locking */
-typedef AnnotatedMixin<std::mutex> CWaitableCriticalSection;
-
-/** Just a typedef for std::condition_variable, can be wrapped later if desired */
-typedef std::condition_variable CConditionVariable;
-
-/** Just a typedef for std::unique_lock, can be wrapped later if desired */
-typedef std::unique_lock<std::mutex> WaitableLock;
+typedef AnnotatedMixin<std::mutex> Mutex;
#ifdef DEBUG_LOCKCONTENTION
void PrintLockContention(const char* pszName, const char* pszFile, int nLine);
#endif
-/** Wrapper around std::unique_lock<CCriticalSection> */
-class SCOPED_LOCKABLE CCriticalBlock
+/** Wrapper around std::unique_lock style lock for Mutex. */
+template <typename Mutex, typename Base = typename Mutex::UniqueLock>
+class SCOPED_LOCKABLE UniqueLock : public Base
{
private:
- std::unique_lock<CCriticalSection> lock;
-
void Enter(const char* pszName, const char* pszFile, int nLine)
{
- EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()));
+ EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()));
#ifdef DEBUG_LOCKCONTENTION
- if (!lock.try_lock()) {
+ if (!Base::try_lock()) {
PrintLockContention(pszName, pszFile, nLine);
#endif
- lock.lock();
+ Base::lock();
#ifdef DEBUG_LOCKCONTENTION
}
#endif
@@ -133,15 +133,15 @@ private:
bool TryEnter(const char* pszName, const char* pszFile, int nLine)
{
- EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()), true);
- lock.try_lock();
- if (!lock.owns_lock())
+ EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()), true);
+ Base::try_lock();
+ if (!Base::owns_lock())
LeaveCritical();
- return lock.owns_lock();
+ return Base::owns_lock();
}
public:
- CCriticalBlock(CCriticalSection& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : lock(mutexIn, std::defer_lock)
+ UniqueLock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : Base(mutexIn, std::defer_lock)
{
if (fTry)
TryEnter(pszName, pszFile, nLine);
@@ -149,35 +149,41 @@ public:
Enter(pszName, pszFile, nLine);
}
- CCriticalBlock(CCriticalSection* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn)
+ UniqueLock(Mutex* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn)
{
if (!pmutexIn) return;
- lock = std::unique_lock<CCriticalSection>(*pmutexIn, std::defer_lock);
+ *static_cast<Base*>(this) = Base(*pmutexIn, std::defer_lock);
if (fTry)
TryEnter(pszName, pszFile, nLine);
else
Enter(pszName, pszFile, nLine);
}
- ~CCriticalBlock() UNLOCK_FUNCTION()
+ ~UniqueLock() UNLOCK_FUNCTION()
{
- if (lock.owns_lock())
+ if (Base::owns_lock())
LeaveCritical();
}
operator bool()
{
- return lock.owns_lock();
+ return Base::owns_lock();
}
};
+template<typename MutexArg>
+using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove_pointer<MutexArg>::type>::type>;
+
#define PASTE(x, y) x ## y
#define PASTE2(x, y) PASTE(x, y)
-#define LOCK(cs) CCriticalBlock PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__)
-#define LOCK2(cs1, cs2) CCriticalBlock criticalblock1(cs1, #cs1, __FILE__, __LINE__), criticalblock2(cs2, #cs2, __FILE__, __LINE__)
-#define TRY_LOCK(cs, name) CCriticalBlock name(cs, #cs, __FILE__, __LINE__, true)
+#define LOCK(cs) DebugLock<decltype(cs)> PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__)
+#define LOCK2(cs1, cs2) \
+ DebugLock<decltype(cs1)> criticalblock1(cs1, #cs1, __FILE__, __LINE__); \
+ DebugLock<decltype(cs2)> criticalblock2(cs2, #cs2, __FILE__, __LINE__);
+#define TRY_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__, true)
+#define WAIT_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__)
#define ENTER_CRITICAL_SECTION(cs) \
{ \
diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp
index 814459c2bc..12ec00ce02 100644
--- a/src/test/scheduler_tests.cpp
+++ b/src/test/scheduler_tests.cpp
@@ -138,11 +138,13 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered)
// the callbacks should run in exactly the order in which they were enqueued
for (int i = 0; i < 100; ++i) {
queue1.AddToProcessQueue([i, &counter1]() {
- assert(i == counter1++);
+ bool expectation = i == counter1++;
+ assert(expectation);
});
queue2.AddToProcessQueue([i, &counter2]() {
- assert(i == counter2++);
+ bool expectation = i == counter2++;
+ assert(expectation);
});
}
diff --git a/src/test/sync_tests.cpp b/src/test/sync_tests.cpp
new file mode 100644
index 0000000000..df0380546e
--- /dev/null
+++ b/src/test/sync_tests.cpp
@@ -0,0 +1,52 @@
+// Copyright (c) 2012-2017 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <sync.h>
+#include <test/test_bitcoin.h>
+
+#include <boost/test/unit_test.hpp>
+
+namespace {
+template <typename MutexType>
+void TestPotentialDeadLockDetected(MutexType& mutex1, MutexType& mutex2)
+{
+ {
+ LOCK2(mutex1, mutex2);
+ }
+ bool error_thrown = false;
+ try {
+ LOCK2(mutex2, mutex1);
+ } catch (const std::logic_error& e) {
+ BOOST_CHECK_EQUAL(e.what(), "potential deadlock detected");
+ error_thrown = true;
+ }
+ #ifdef DEBUG_LOCKORDER
+ BOOST_CHECK(error_thrown);
+ #else
+ BOOST_CHECK(!error_thrown);
+ #endif
+}
+} // namespace
+
+BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup)
+
+BOOST_AUTO_TEST_CASE(potential_deadlock_detected)
+{
+ #ifdef DEBUG_LOCKORDER
+ bool prev = g_debug_lockorder_abort;
+ g_debug_lockorder_abort = false;
+ #endif
+
+ CCriticalSection rmutex1, rmutex2;
+ TestPotentialDeadLockDetected(rmutex1, rmutex2);
+
+ Mutex mutex1, mutex2;
+ TestPotentialDeadLockDetected(mutex1, mutex2);
+
+ #ifdef DEBUG_LOCKORDER
+ g_debug_lockorder_abort = prev;
+ #endif
+}
+
+BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/threadinterrupt.cpp b/src/threadinterrupt.cpp
index d08d52e4a9..340106ed99 100644
--- a/src/threadinterrupt.cpp
+++ b/src/threadinterrupt.cpp
@@ -5,6 +5,8 @@
#include <threadinterrupt.h>
+#include <sync.h>
+
CThreadInterrupt::CThreadInterrupt() : flag(false) {}
CThreadInterrupt::operator bool() const
@@ -20,7 +22,7 @@ void CThreadInterrupt::reset()
void CThreadInterrupt::operator()()
{
{
- std::unique_lock<std::mutex> lock(mut);
+ LOCK(mut);
flag.store(true, std::memory_order_release);
}
cond.notify_all();
@@ -28,7 +30,7 @@ void CThreadInterrupt::operator()()
bool CThreadInterrupt::sleep_for(std::chrono::milliseconds rel_time)
{
- std::unique_lock<std::mutex> lock(mut);
+ WAIT_LOCK(mut, lock);
return !cond.wait_for(lock, rel_time, [this]() { return flag.load(std::memory_order_acquire); });
}
diff --git a/src/threadinterrupt.h b/src/threadinterrupt.h
index c30bd3d657..9c6fccfcde 100644
--- a/src/threadinterrupt.h
+++ b/src/threadinterrupt.h
@@ -5,6 +5,8 @@
#ifndef BITCOIN_THREADINTERRUPT_H
#define BITCOIN_THREADINTERRUPT_H
+#include <sync.h>
+
#include <atomic>
#include <chrono>
#include <condition_variable>
@@ -28,7 +30,7 @@ public:
private:
std::condition_variable cond;
- std::mutex mut;
+ Mutex mut;
std::atomic<bool> flag;
};
diff --git a/src/threadsafety.h b/src/threadsafety.h
index a2f45e5fce..47e6b2ea38 100644
--- a/src/threadsafety.h
+++ b/src/threadsafety.h
@@ -10,7 +10,7 @@
// TL;DR Add GUARDED_BY(mutex) to member variables. The others are
// rarely necessary. Ex: int nFoo GUARDED_BY(cs_foo);
//
-// See http://clang.llvm.org/docs/LanguageExtensions.html#threadsafety
+// See https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
// for documentation. The clang compiler can do advanced static analysis
// of locking when given the -Wthread-safety option.
#define LOCKABLE __attribute__((lockable))
diff --git a/src/validation.cpp b/src/validation.cpp
index f6257ffaed..a9eea5edd9 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -217,8 +217,8 @@ CCriticalSection cs_main;
BlockMap& mapBlockIndex = g_chainstate.mapBlockIndex;
CChain& chainActive = g_chainstate.chainActive;
CBlockIndex *pindexBestHeader = nullptr;
-CWaitableCriticalSection g_best_block_mutex;
-CConditionVariable g_best_block_cv;
+Mutex g_best_block_mutex;
+std::condition_variable g_best_block_cv;
uint256 g_best_block;
int nScriptCheckThreads = 0;
std::atomic_bool fImporting(false);
@@ -2239,7 +2239,7 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar
mempool.AddTransactionsUpdated(1);
{
- WaitableLock lock(g_best_block_mutex);
+ LOCK(g_best_block_mutex);
g_best_block = pindexNew->GetBlockHash();
g_best_block_cv.notify_all();
}
diff --git a/src/validation.h b/src/validation.h
index c4c9b8b5ba..3df6456eca 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -151,8 +151,8 @@ extern BlockMap& mapBlockIndex;
extern uint64_t nLastBlockTx;
extern uint64_t nLastBlockWeight;
extern const std::string strMessageMagic;
-extern CWaitableCriticalSection g_best_block_mutex;
-extern CConditionVariable g_best_block_cv;
+extern Mutex g_best_block_mutex;
+extern std::condition_variable g_best_block_cv;
extern uint256 g_best_block;
extern std::atomic_bool fImporting;
extern std::atomic_bool fReindex;
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index b436efc276..3a8e6f751a 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -320,7 +320,11 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
// Confirm ListCoins initially returns 1 coin grouped under coinbaseKey
// address.
- auto list = wallet->ListCoins();
+ std::map<CTxDestination, std::vector<COutput>> list;
+ {
+ LOCK2(cs_main, wallet->cs_wallet);
+ list = wallet->ListCoins();
+ }
BOOST_CHECK_EQUAL(list.size(), 1U);
BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress);
BOOST_CHECK_EQUAL(list.begin()->second.size(), 1U);
@@ -333,7 +337,10 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
// coinbaseKey pubkey, even though the change address has a different
// pubkey.
AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, false /* subtract fee */});
- list = wallet->ListCoins();
+ {
+ LOCK2(cs_main, wallet->cs_wallet);
+ list = wallet->ListCoins();
+ }
BOOST_CHECK_EQUAL(list.size(), 1U);
BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress);
BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U);
@@ -359,7 +366,10 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
}
// Confirm ListCoins still returns same result as before, despite coins
// being locked.
- list = wallet->ListCoins();
+ {
+ LOCK2(cs_main, wallet->cs_wallet);
+ list = wallet->ListCoins();
+ }
BOOST_CHECK_EQUAL(list.size(), 1U);
BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress);
BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index d3ccd2dfaa..6cbfbc1f90 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2252,20 +2252,12 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const
std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins() const
{
- // TODO: Add AssertLockHeld(cs_wallet) here.
- //
- // Because the return value from this function contains pointers to
- // CWalletTx objects, callers to this function really should acquire the
- // cs_wallet lock before calling it. However, the current caller doesn't
- // acquire this lock yet. There was an attempt to add the missing lock in
- // https://github.com/bitcoin/bitcoin/pull/10340, but that change has been
- // postponed until after https://github.com/bitcoin/bitcoin/pull/10244 to
- // avoid adding some extra complexity to the Qt code.
+ AssertLockHeld(cs_main);
+ AssertLockHeld(cs_wallet);
std::map<CTxDestination, std::vector<COutput>> result;
std::vector<COutput> availableCoins;
- LOCK2(cs_main, cs_wallet);
AvailableCoins(availableCoins);
for (auto& coin : availableCoins) {
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 05ae9a1bbf..da326517c0 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -764,7 +764,7 @@ public:
/**
* Return list of available coins and locked coins grouped by non-change output address.
*/
- std::map<CTxDestination, std::vector<COutput>> ListCoins() const;
+ std::map<CTxDestination, std::vector<COutput>> ListCoins() const EXCLUSIVE_LOCKS_REQUIRED(cs_main, cs_wallet);
/**
* Find non-change parent output.
diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py
index e878ded258..ceca40527f 100755
--- a/test/functional/rpc_help.py
+++ b/test/functional/rpc_help.py
@@ -4,7 +4,7 @@
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test RPC help output."""
-from test_framework.test_framework import BitcoinTestFramework
+from test_framework.test_framework import BitcoinTestFramework, is_zmq_enabled
from test_framework.util import assert_equal, assert_raises_rpc_error
class HelpRpcTest(BitcoinTestFramework):
@@ -25,7 +25,13 @@ class HelpRpcTest(BitcoinTestFramework):
# command titles
titles = [line[3:-3] for line in node.help().splitlines() if line.startswith('==')]
- assert_equal(titles, ['Blockchain', 'Control', 'Generating', 'Mining', 'Network', 'Rawtransactions', 'Util', 'Wallet', 'Zmq'])
+
+ components = ['Blockchain', 'Control', 'Generating', 'Mining', 'Network', 'Rawtransactions', 'Util', 'Wallet']
+
+ if is_zmq_enabled(self):
+ components.append('Zmq')
+
+ assert_equal(titles, components)
if __name__ == '__main__':
HelpRpcTest().main()
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index b876d9bd76..0e76b52570 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -488,8 +488,13 @@ def skip_if_no_py3_zmq():
def skip_if_no_bitcoind_zmq(test_instance):
"""Skip the running test if bitcoind has not been compiled with zmq support."""
+ if not is_zmq_enabled(test_instance):
+ raise SkipTest("bitcoind has not been built with zmq enabled.")
+
+
+def is_zmq_enabled(test_instance):
+ """Checks whether zmq is enabled or not."""
config = configparser.ConfigParser()
config.read_file(open(test_instance.options.configfile))
- if not config["components"].getboolean("ENABLE_ZMQ"):
- raise SkipTest("bitcoind has not been built with zmq enabled.")
+ return config["components"].getboolean("ENABLE_ZMQ")
diff --git a/test/lint/lint-assertions.sh b/test/lint/lint-assertions.sh
new file mode 100755
index 0000000000..5bbcae79eb
--- /dev/null
+++ b/test/lint/lint-assertions.sh
@@ -0,0 +1,23 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2018 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+#
+# Check for assertions with obvious side effects.
+
+export LC_ALL=C
+
+EXIT_CODE=0
+
+# PRE31-C (SEI CERT C Coding Standard):
+# "Assertions should not contain assignments, increment, or decrement operators."
+OUTPUT=$(git grep -E '[^_]assert\(.*(\+\+|\-\-|[^=!<>]=[^=!<>]).*\);' -- "*.cpp" "*.h")
+if [[ ${OUTPUT} != "" ]]; then
+ echo "Assertions should not have side effects:"
+ echo
+ echo "${OUTPUT}"
+ EXIT_CODE=1
+fi
+
+exit ${EXIT_CODE}
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-20 11:37:24 +0000