Permalink
Browse files

Merge pull request #6534

9493803 Revert "Revert "Assert on probable deadlocks if the second lock isnt try_lock"" (Cory Fields)
605433f locks: don't lock cs_wallet before making rpc calls (Cory Fields)
  • Loading branch information...
laanwj committed Aug 10, 2015
2 parents ce37a04 + 9493803 commit 6c1c7fdfd6531a2abe92219af81dd0e599c2de7a
Showing with 59 additions and 29 deletions.
  1. +39 −10 src/sync.cpp
  2. +20 −19 src/test/rpc_wallet_tests.cpp
View
@@ -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> lockstack;
static void potential_deadlock_detected(const std::pair<void*, void*>& 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<void*, void*> 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()
@@ -27,8 +27,6 @@ BOOST_FIXTURE_TEST_SUITE(rpc_wallet_tests, TestingSetup)
BOOST_AUTO_TEST_CASE(rpc_addmultisig)
{
LOCK(pwalletMain->cs_wallet);
rpcfn_type addmultisig = tableRPC["addmultisigaddress"]->actor;
// old, 65-byte-long:
@@ -68,25 +66,28 @@ BOOST_AUTO_TEST_CASE(rpc_wallet)
{
// Test RPC calls for various wallet statistics
UniValue r;
LOCK2(cs_main, pwalletMain->cs_wallet);
CPubKey demoPubkey = pwalletMain->GenerateNewKey();
CBitcoinAddress demoAddress = CBitcoinAddress(CTxDestination(demoPubkey.GetID()));
CPubKey demoPubkey;
CBitcoinAddress demoAddress;
UniValue retValue;
string strAccount = "walletDemoAccount";
string strPurpose = "receive";
BOOST_CHECK_NO_THROW({ /*Initialize Wallet with an account */
CWalletDB walletdb(pwalletMain->strWalletFile);
CAccount account;
account.vchPubKey = demoPubkey;
pwalletMain->SetAddressBook(account.vchPubKey.GetID(), strAccount, strPurpose);
walletdb.WriteAccount(strAccount, account);
});
CPubKey setaccountDemoPubkey = pwalletMain->GenerateNewKey();
CBitcoinAddress setaccountDemoAddress = CBitcoinAddress(CTxDestination(setaccountDemoPubkey.GetID()));
CBitcoinAddress setaccountDemoAddress;
{
LOCK(pwalletMain->cs_wallet);
demoPubkey = pwalletMain->GenerateNewKey();
demoAddress = CBitcoinAddress(CTxDestination(demoPubkey.GetID()));
string strPurpose = "receive";
BOOST_CHECK_NO_THROW({ /*Initialize Wallet with an account */
CWalletDB walletdb(pwalletMain->strWalletFile);
CAccount account;
account.vchPubKey = demoPubkey;
pwalletMain->SetAddressBook(account.vchPubKey.GetID(), strAccount, strPurpose);
walletdb.WriteAccount(strAccount, account);
});
CPubKey setaccountDemoPubkey = pwalletMain->GenerateNewKey();
setaccountDemoAddress = CBitcoinAddress(CTxDestination(setaccountDemoPubkey.GetID()));
}
/*********************************
* setaccount
*********************************/

0 comments on commit 6c1c7fd

Please sign in to comment.