diff --git a/qa/rpc-tests/test_framework/comptool.py b/qa/rpc-tests/test_framework/comptool.py index 24ae05807..b945f1bf2 100755 --- a/qa/rpc-tests/test_framework/comptool.py +++ b/qa/rpc-tests/test_framework/comptool.py @@ -140,8 +140,8 @@ class TestNode(NodeConnCB): # or false, then only the last tx is tested against outcome.) class TestInstance(object): - def __init__(self, objects=[], sync_every_block=True, sync_every_tx=False): - self.blocks_and_transactions = objects + def __init__(self, objects=None, sync_every_block=True, sync_every_tx=False): + self.blocks_and_transactions = objects if objects else [] self.sync_every_block = sync_every_block self.sync_every_tx = sync_every_tx diff --git a/src/init.cpp b/src/init.cpp index 5d7f4d3f1..705dc1572 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -985,6 +985,9 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) fAlerts = GetBoolArg("-alerts", DEFAULT_ALERTS); + // Option to startup with mocktime set (used for regression testing): + SetMockTime(GetArg("-mocktime", 0)); // SetMockTime(0) is a no-op + #ifdef ENABLE_MINING if (mapArgs.count("-mineraddress")) { CBitcoinAddress addr; diff --git a/src/main.cpp b/src/main.cpp index 4e9230c51..52e8ca4c6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -79,9 +79,9 @@ struct COrphanTx { CTransaction tx; NodeId fromPeer; }; -map mapOrphanTransactions; -map > mapOrphanTransactionsByPrev; -void EraseOrphansFor(NodeId peer); +map mapOrphanTransactions GUARDED_BY(cs_main);; +map > mapOrphanTransactionsByPrev GUARDED_BY(cs_main);; +void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** * Returns true if there are nRequired or more blocks of minVersion or above @@ -554,7 +554,7 @@ CBlockTreeDB *pblocktree = NULL; // mapOrphanTransactions // -bool AddOrphanTx(const CTransaction& tx, NodeId peer) +bool AddOrphanTx(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { uint256 hash = tx.GetHash(); if (mapOrphanTransactions.count(hash)) @@ -584,7 +584,7 @@ bool AddOrphanTx(const CTransaction& tx, NodeId peer) return true; } -void static EraseOrphanTx(uint256 hash) +void static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { map::iterator it = mapOrphanTransactions.find(hash); if (it == mapOrphanTransactions.end()) @@ -618,7 +618,7 @@ void EraseOrphansFor(NodeId peer) } -unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) +unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { unsigned int nEvicted = 0; while (mapOrphanTransactions.size() > nMaxOrphans) @@ -4131,7 +4131,7 @@ std::string GetWarnings(const std::string& strFor) // -bool static AlreadyHave(const CInv& inv) +bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { switch (inv.type) { diff --git a/src/rpcmisc.cpp b/src/rpcmisc.cpp index 38610903b..e7f8d0427 100644 --- a/src/rpcmisc.cpp +++ b/src/rpcmisc.cpp @@ -452,10 +452,19 @@ UniValue setmocktime(const UniValue& params, bool fHelp) if (!Params().MineBlocksOnDemand()) throw runtime_error("setmocktime for regression testing (-regtest mode) only"); - LOCK(cs_main); + // cs_vNodes is locked and node send/receive times are updated + // atomically with the time change to prevent peers from being + // disconnected because we think we haven't communicated with them + // in a long time. + LOCK2(cs_main, cs_vNodes); RPCTypeCheck(params, boost::assign::list_of(UniValue::VNUM)); SetMockTime(params[0].get_int64()); + uint64_t t = GetTime(); + BOOST_FOREACH(CNode* pnode, vNodes) { + pnode->nLastSend = pnode->nLastRecv = t; + } + return NullUniValue; } diff --git a/src/sync.cpp b/src/sync.cpp index a42293996..1837e8d53 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -33,20 +33,22 @@ void PrintLockContention(const char* pszName, const char* pszFile, int nLine) // struct CLockLocation { - CLockLocation(const char* pszName, const char* pszFile, int nLine) + CLockLocation(const char* pszName, const char* pszFile, int nLine, bool fTryIn) { mutexName = pszName; sourceFile = pszFile; sourceLine = nLine; + fTry = fTryIn; } std::string ToString() const { - return mutexName + " " + sourceFile + ":" + itostr(sourceLine); + return mutexName + " " + sourceFile + ":" + itostr(sourceLine) + (fTry ? " (TRY)" : ""); } std::string MutexName() const { return mutexName; } + bool fTry; private: std::string mutexName; std::string sourceFile; @@ -62,23 +64,52 @@ static boost::thread_specific_ptr lockstack; static void potential_deadlock_detected(const std::pair& mismatch, const LockStack& s1, const LockStack& s2) { + // We attempt to not assert on probably-not deadlocks by assuming that + // a try lock will immediately have otherwise bailed if it had + // failed to get the lock + // We do this by, for the locks which triggered the potential deadlock, + // in either lockorder, checking that the second of the two which is locked + // is only a TRY_LOCK, ignoring locks if they are reentrant. + bool firstLocked = false; + bool secondLocked = false; + bool onlyMaybeDeadlock = false; + LogPrintf("POTENTIAL DEADLOCK DETECTED\n"); LogPrintf("Previous lock order was:\n"); BOOST_FOREACH (const PAIRTYPE(void*, CLockLocation) & i, s2) { - if (i.first == mismatch.first) + if (i.first == mismatch.first) { LogPrintf(" (1)"); - if (i.first == mismatch.second) + if (!firstLocked && secondLocked && i.second.fTry) + onlyMaybeDeadlock = true; + firstLocked = true; + } + if (i.first == mismatch.second) { LogPrintf(" (2)"); + if (!secondLocked && firstLocked && i.second.fTry) + onlyMaybeDeadlock = true; + secondLocked = true; + } LogPrintf(" %s\n", i.second.ToString()); } + firstLocked = false; + secondLocked = false; LogPrintf("Current lock order is:\n"); BOOST_FOREACH (const PAIRTYPE(void*, CLockLocation) & i, s1) { - if (i.first == mismatch.first) + if (i.first == mismatch.first) { LogPrintf(" (1)"); - if (i.first == mismatch.second) + if (!firstLocked && secondLocked && i.second.fTry) + onlyMaybeDeadlock = true; + firstLocked = true; + } + if (i.first == mismatch.second) { LogPrintf(" (2)"); + if (!secondLocked && firstLocked && i.second.fTry) + onlyMaybeDeadlock = true; + secondLocked = true; + } LogPrintf(" %s\n", i.second.ToString()); } + assert(onlyMaybeDeadlock); } static void push_lock(void* c, const CLockLocation& locklocation, bool fTry) @@ -101,10 +132,8 @@ static void push_lock(void* c, const CLockLocation& locklocation, bool fTry) lockorders[p1] = (*lockstack); std::pair p2 = std::make_pair(c, i.first); - if (lockorders.count(p2)) { + if (lockorders.count(p2)) potential_deadlock_detected(p1, lockorders[p2], lockorders[p1]); - break; - } } } dd_mutex.unlock(); @@ -119,7 +148,7 @@ static void pop_lock() void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry) { - push_lock(cs, CLockLocation(pszName, pszFile, nLine), fTry); + push_lock(cs, CLockLocation(pszName, pszFile, nLine, fTry), fTry); } void LeaveCritical() diff --git a/src/sync.h b/src/sync.h index 5c1516534..68a944308 100644 --- a/src/sync.h +++ b/src/sync.h @@ -101,7 +101,7 @@ void PrintLockContention(const char* pszName, const char* pszFile, int nLine); /** Wrapper around boost::unique_lock */ template -class CMutexLock +class SCOPED_LOCKABLE CMutexLock { private: boost::unique_lock lock; @@ -129,7 +129,7 @@ private: } public: - CMutexLock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) : lock(mutexIn, boost::defer_lock) + CMutexLock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : lock(mutexIn, boost::defer_lock) { if (fTry) TryEnter(pszName, pszFile, nLine); @@ -137,7 +137,7 @@ public: Enter(pszName, pszFile, nLine); } - CMutexLock(Mutex* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) + CMutexLock(Mutex* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn) { if (!pmutexIn) return; @@ -148,7 +148,7 @@ public: Enter(pszName, pszFile, nLine); } - ~CMutexLock() + ~CMutexLock() UNLOCK_FUNCTION() { if (lock.owns_lock()) LeaveCritical(); diff --git a/src/test/data/bitcoin-util-test.json b/src/test/data/bitcoin-util-test.json index e30c53759..c23befe23 100644 --- a/src/test/data/bitcoin-util-test.json +++ b/src/test/data/bitcoin-util-test.json @@ -52,7 +52,7 @@ ["-create", "in=4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59485:0", "set=privatekeys:[\"5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreAnchuDf\"]", - "set=prevtxs:[{\"txid\":\"4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59485\",\"vout\":0,\"scriptPubKey\":\"4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59485\"}]", + "set=prevtxs:[{\"txid\":\"4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59485\",\"vout\":0,\"scriptPubKey\":\"76a91491b24bf9f5288532960ac687abb035127b1d28a588ac\"}]", "sign=ALL", "outaddr=0.001:t1Ruz6gK4QPZoPPGpHaieupnnh62mktjQE7"], "output_cmp": "txcreatesign.hex" diff --git a/src/test/data/txcreatesign.hex b/src/test/data/txcreatesign.hex index 56ce28a86..a46fcc88c 100644 --- a/src/test/data/txcreatesign.hex +++ b/src/test/data/txcreatesign.hex @@ -1 +1 @@ -01000000018594c5bdcaec8f06b78b596f31cd292a294fd031e24eec716f43dac91ea7494d0000000000ffffffff01a0860100000000001976a9145834479edbbe0539b31ffd3a8f8ebadc2165ed0188ac00000000 +01000000018594c5bdcaec8f06b78b596f31cd292a294fd031e24eec716f43dac91ea7494d000000008b48304502210096a75056c9e2cc62b7214777b3d2a592cfda7092520126d4ebfcd6d590c99bd8022051bb746359cf98c0603f3004477eac68701132380db8facba19c89dc5ab5c5e201410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ffffffff01a0860100000000001976a9145834479edbbe0539b31ffd3a8f8ebadc2165ed0188ac00000000