From f8dcd5ca6f55ad49807cf7491c1f153f6158400e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 6 Apr 2012 18:39:12 +0200 Subject: [PATCH] Use scoped locks instead of CRITICAL_BLOCK --- src/addrman.h | 20 +++--- src/bitcoinrpc.cpp | 7 +- src/db.cpp | 23 +++--- src/init.cpp | 9 ++- src/keystore.cpp | 28 +++++--- src/keystore.h | 18 +++-- src/main.cpp | 72 +++++++++++-------- src/net.cpp | 116 +++++++++++++++++++++---------- src/net.h | 26 +++++-- src/qt/addresstablemodel.cpp | 14 ++-- src/qt/transactiondesc.cpp | 6 +- src/qt/transactiontablemodel.cpp | 12 ++-- src/qt/walletmodel.cpp | 10 +-- src/rpcdump.cpp | 4 +- src/test/util_tests.cpp | 7 +- src/util.cpp | 2 +- src/util.h | 15 ++-- src/wallet.cpp | 81 +++++++++++---------- src/wallet.h | 6 +- 19 files changed, 286 insertions(+), 190 deletions(-) diff --git a/src/addrman.h b/src/addrman.h index 91e1f87f..7652df66 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -266,8 +266,8 @@ public: // // This format is more complex, but significantly smaller (at most 1.5 MiB), and supports // changes to the ADDRMAN_ parameters without breaking the on-disk structure. - CRITICAL_BLOCK(cs) { + LOCK(cs); unsigned char nVersion = 0; READWRITE(nVersion); READWRITE(nKey); @@ -398,8 +398,8 @@ public: void Check() { #ifdef DEBUG_ADDRMAN - CRITICAL_BLOCK(cs) { + LOCK(cs); int err; if ((err=Check_())) printf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err); @@ -411,8 +411,8 @@ public: bool Add(const CAddress &addr, const CNetAddr& source, int64 nTimePenalty = 0) { bool fRet = false; - CRITICAL_BLOCK(cs) { + LOCK(cs); Check(); fRet |= Add_(addr, source, nTimePenalty); Check(); @@ -426,8 +426,8 @@ public: bool Add(const std::vector &vAddr, const CNetAddr& source, int64 nTimePenalty = 0) { int nAdd = 0; - CRITICAL_BLOCK(cs) { + LOCK(cs); Check(); for (std::vector::const_iterator it = vAddr.begin(); it != vAddr.end(); it++) nAdd += Add_(*it, source, nTimePenalty) ? 1 : 0; @@ -441,8 +441,8 @@ public: // Mark an entry as accessible. void Good(const CService &addr, int64 nTime = GetAdjustedTime()) { - CRITICAL_BLOCK(cs) { + LOCK(cs); Check(); Good_(addr, nTime); Check(); @@ -452,8 +452,8 @@ public: // Mark an entry as connection attempted to. void Attempt(const CService &addr, int64 nTime = GetAdjustedTime()) { - CRITICAL_BLOCK(cs) { + LOCK(cs); Check(); Attempt_(addr, nTime); Check(); @@ -465,8 +465,8 @@ public: CAddress Select(int nUnkBias = 50) { CAddress addrRet; - CRITICAL_BLOCK(cs) { + LOCK(cs); Check(); addrRet = Select_(nUnkBias); Check(); @@ -479,8 +479,10 @@ public: { Check(); std::vector vAddr; - CRITICAL_BLOCK(cs) + { + LOCK(cs); GetAddr_(vAddr); + } Check(); return vAddr; } @@ -488,8 +490,8 @@ public: // Mark an entry as currently-connected-to. void Connected(const CService &addr, int64 nTime = GetAdjustedTime()) { - CRITICAL_BLOCK(cs) { + LOCK(cs); Check(); Connected_(addr, nTime); Check(); diff --git a/src/bitcoinrpc.cpp b/src/bitcoinrpc.cpp index 8cd47503..20010517 100644 --- a/src/bitcoinrpc.cpp +++ b/src/bitcoinrpc.cpp @@ -1648,8 +1648,8 @@ Value walletlock(const Array& params, bool fHelp) if (!pwalletMain->IsCrypted()) throw JSONRPCError(-15, "Error: running with an unencrypted wallet, but walletlock was called."); - CRITICAL_BLOCK(cs_nWalletUnlockTime) { + LOCK(cs_nWalletUnlockTime); pwalletMain->Lock(); nWalletUnlockTime = 0; } @@ -2498,9 +2498,10 @@ void ThreadRPCServer2(void* parg) { // Execute Value result; - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(pwalletMain->cs_wallet) + { + LOCK2(cs_main, pwalletMain->cs_wallet); result = (*(*mi).second)(params, false); + } // Send reply string strReply = JSONRPCReply(result, Value::null, id); diff --git a/src/db.cpp b/src/db.cpp index b86a56e4..52db39a3 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -72,8 +72,8 @@ CDB::CDB(const char* pszFile, const char* pszMode) : pdb(NULL) if (fCreate) nFlags |= DB_CREATE; - CRITICAL_BLOCK(cs_db) { + LOCK(cs_db); if (!fDbEnvInit) { if (fShutdown) @@ -126,8 +126,10 @@ CDB::CDB(const char* pszFile, const char* pszMode) : pdb(NULL) { delete pdb; pdb = NULL; - CRITICAL_BLOCK(cs_db) + { + LOCK(cs_db); --mapFileUseCount[strFile]; + } strFile = ""; throw runtime_error(strprintf("CDB() : can't open database file %s, error %d", pszFile, ret)); } @@ -165,14 +167,16 @@ void CDB::Close() dbenv.txn_checkpoint(nMinutes ? GetArg("-dblogsize", 100)*1024 : 0, nMinutes, 0); - CRITICAL_BLOCK(cs_db) + { + LOCK(cs_db); --mapFileUseCount[strFile]; + } } void static CloseDb(const string& strFile) { - CRITICAL_BLOCK(cs_db) { + LOCK(cs_db); if (mapDb[strFile] != NULL) { // Close the database handle @@ -188,8 +192,8 @@ bool CDB::Rewrite(const string& strFile, const char* pszSkip) { while (!fShutdown) { - CRITICAL_BLOCK(cs_db) { + LOCK(cs_db); if (!mapFileUseCount.count(strFile) || mapFileUseCount[strFile] == 0) { // Flush log data to the dat file @@ -286,8 +290,8 @@ void DBFlush(bool fShutdown) printf("DBFlush(%s)%s\n", fShutdown ? "true" : "false", fDbEnvInit ? "" : " db not started"); if (!fDbEnvInit) return; - CRITICAL_BLOCK(cs_db) { + LOCK(cs_db); map::iterator mi = mapFileUseCount.begin(); while (mi != mapFileUseCount.end()) { @@ -877,8 +881,8 @@ int CWalletDB::LoadWallet(CWallet* pwallet) bool fIsEncrypted = false; //// todo: shouldn't we catch exceptions and try to recover and continue? - CRITICAL_BLOCK(pwallet->cs_wallet) { + LOCK(pwallet->cs_wallet); int nMinVersion = 0; if (Read((string)"minversion", nMinVersion)) { @@ -1119,7 +1123,8 @@ void ThreadFlushWalletDB(void* parg) if (nLastFlushed != nWalletDBUpdated && GetTime() - nLastWalletUpdate >= 2) { - TRY_CRITICAL_BLOCK(cs_db) + TRY_LOCK(cs_db,lockDb); + if (lockDb) { // Don't do this if any databases are in use int nRefCount = 0; @@ -1160,8 +1165,8 @@ bool BackupWallet(const CWallet& wallet, const string& strDest) return false; while (!fShutdown) { - CRITICAL_BLOCK(cs_db) { + LOCK(cs_db); if (!mapFileUseCount.count(wallet.strWalletFile) || mapFileUseCount[wallet.strWalletFile] == 0) { // Flush log data to the dat file diff --git a/src/init.cpp b/src/init.cpp index 3f900fc8..a1e45b1c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -40,10 +40,13 @@ void Shutdown(void* parg) static CCriticalSection cs_Shutdown; static bool fTaken; bool fFirstThread = false; - TRY_CRITICAL_BLOCK(cs_Shutdown) { - fFirstThread = !fTaken; - fTaken = true; + TRY_LOCK(cs_Shutdown, lockShutdown); + if (lockShutdown) + { + fFirstThread = !fTaken; + fTaken = true; + } } static bool fExit; if (fFirstThread) diff --git a/src/keystore.cpp b/src/keystore.cpp index 23f9e32f..7b46f6b0 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -21,31 +21,37 @@ bool CBasicKeyStore::AddKey(const CKey& key) { bool fCompressed = false; CSecret secret = key.GetSecret(fCompressed); - CRITICAL_BLOCK(cs_KeyStore) + { + LOCK(cs_KeyStore); mapKeys[CBitcoinAddress(key.GetPubKey())] = make_pair(secret, fCompressed); + } return true; } bool CBasicKeyStore::AddCScript(const CScript& redeemScript) { - CRITICAL_BLOCK(cs_KeyStore) + { + LOCK(cs_KeyStore); mapScripts[Hash160(redeemScript)] = redeemScript; + } return true; } bool CBasicKeyStore::HaveCScript(const uint160& hash) const { bool result; - CRITICAL_BLOCK(cs_KeyStore) + { + LOCK(cs_KeyStore); result = (mapScripts.count(hash) > 0); + } return result; } bool CBasicKeyStore::GetCScript(const uint160 &hash, CScript& redeemScriptOut) const { - CRITICAL_BLOCK(cs_KeyStore) { + LOCK(cs_KeyStore); ScriptMap::const_iterator mi = mapScripts.find(hash); if (mi != mapScripts.end()) { @@ -58,8 +64,8 @@ bool CBasicKeyStore::GetCScript(const uint160 &hash, CScript& redeemScriptOut) c bool CCryptoKeyStore::SetCrypted() { - CRITICAL_BLOCK(cs_KeyStore) { + LOCK(cs_KeyStore); if (fUseCrypto) return true; if (!mapKeys.empty()) @@ -71,8 +77,8 @@ bool CCryptoKeyStore::SetCrypted() bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn) { - CRITICAL_BLOCK(cs_KeyStore) { + LOCK(cs_KeyStore); if (!SetCrypted()) return false; @@ -100,8 +106,8 @@ bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn) bool CCryptoKeyStore::AddKey(const CKey& key) { - CRITICAL_BLOCK(cs_KeyStore) { + LOCK(cs_KeyStore); if (!IsCrypted()) return CBasicKeyStore::AddKey(key); @@ -123,8 +129,8 @@ bool CCryptoKeyStore::AddKey(const CKey& key) bool CCryptoKeyStore::AddCryptedKey(const std::vector &vchPubKey, const std::vector &vchCryptedSecret) { - CRITICAL_BLOCK(cs_KeyStore) { + LOCK(cs_KeyStore); if (!SetCrypted()) return false; @@ -135,8 +141,8 @@ bool CCryptoKeyStore::AddCryptedKey(const std::vector &vchPubKey, bool CCryptoKeyStore::GetKey(const CBitcoinAddress &address, CKey& keyOut) const { - CRITICAL_BLOCK(cs_KeyStore) { + LOCK(cs_KeyStore); if (!IsCrypted()) return CBasicKeyStore::GetKey(address, keyOut); @@ -160,8 +166,8 @@ bool CCryptoKeyStore::GetKey(const CBitcoinAddress &address, CKey& keyOut) const bool CCryptoKeyStore::GetPubKey(const CBitcoinAddress &address, std::vector& vchPubKeyOut) const { - CRITICAL_BLOCK(cs_KeyStore) { + LOCK(cs_KeyStore); if (!IsCrypted()) return CKeyStore::GetPubKey(address, vchPubKeyOut); @@ -177,8 +183,8 @@ bool CCryptoKeyStore::GetPubKey(const CBitcoinAddress &address, std::vector 0); + } return result; } void GetKeys(std::set &setAddress) const { setAddress.clear(); - CRITICAL_BLOCK(cs_KeyStore) { + LOCK(cs_KeyStore); KeyMap::const_iterator mi = mapKeys.begin(); while (mi != mapKeys.end()) { @@ -73,8 +75,8 @@ public: } bool GetKey(const CBitcoinAddress &address, CKey &keyOut) const { - CRITICAL_BLOCK(cs_KeyStore) { + LOCK(cs_KeyStore); KeyMap::const_iterator mi = mapKeys.find(address); if (mi != mapKeys.end()) { @@ -129,8 +131,10 @@ public: if (!IsCrypted()) return false; bool result; - CRITICAL_BLOCK(cs_KeyStore) + { + LOCK(cs_KeyStore); result = vMasterKey.empty(); + } return result; } @@ -139,8 +143,10 @@ public: if (!SetCrypted()) return false; - CRITICAL_BLOCK(cs_KeyStore) + { + LOCK(cs_KeyStore); vMasterKey.clear(); + } return true; } @@ -149,8 +155,8 @@ public: bool AddKey(const CKey& key); bool HaveKey(const CBitcoinAddress &address) const { - CRITICAL_BLOCK(cs_KeyStore) { + LOCK(cs_KeyStore); if (!IsCrypted()) return CBasicKeyStore::HaveKey(address); return mapCryptedKeys.count(address) > 0; diff --git a/src/main.cpp b/src/main.cpp index 7c49316b..dea60f03 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -75,16 +75,16 @@ int64 nTransactionFee = 0; void RegisterWallet(CWallet* pwalletIn) { - CRITICAL_BLOCK(cs_setpwalletRegistered) { + LOCK(cs_setpwalletRegistered); setpwalletRegistered.insert(pwalletIn); } } void UnregisterWallet(CWallet* pwalletIn) { - CRITICAL_BLOCK(cs_setpwalletRegistered) { + LOCK(cs_setpwalletRegistered); setpwalletRegistered.erase(pwalletIn); } } @@ -478,9 +478,11 @@ bool CTransaction::AcceptToMemoryPool(CTxDB& txdb, bool fCheckInputs, bool* pfMi // Do we already have it? uint256 hash = GetHash(); - CRITICAL_BLOCK(cs_mapTransactions) + { + LOCK(cs_mapTransactions); if (mapTransactions.count(hash)) return false; + } if (fCheckInputs) if (txdb.ContainsTx(hash)) return false; @@ -552,8 +554,8 @@ bool CTransaction::AcceptToMemoryPool(CTxDB& txdb, bool fCheckInputs, bool* pfMi static int64 nLastTime; int64 nNow = GetTime(); - CRITICAL_BLOCK(cs) { + LOCK(cs); // Use an exponentially decaying ~10-minute window: dFreeCount *= pow(1.0 - 1.0/600.0, (double)(nNow - nLastTime)); nLastTime = nNow; @@ -576,8 +578,8 @@ bool CTransaction::AcceptToMemoryPool(CTxDB& txdb, bool fCheckInputs, bool* pfMi } // Store transaction in memory - CRITICAL_BLOCK(cs_mapTransactions) { + LOCK(cs_mapTransactions); if (ptxOld) { printf("AcceptToMemoryPool() : replacing tx %s with new version\n", ptxOld->GetHash().ToString().c_str()); @@ -608,8 +610,8 @@ bool CTransaction::AddToMemoryPoolUnchecked() printf("AcceptToMemoryPoolUnchecked(): size %lu\n", mapTransactions.size()); // Add to memory pool without checking anything. Don't call this directly, // call AcceptToMemoryPool to properly check the transaction first. - CRITICAL_BLOCK(cs_mapTransactions) { + LOCK(cs_mapTransactions); uint256 hash = GetHash(); mapTransactions[hash] = *this; for (int i = 0; i < vin.size(); i++) @@ -624,8 +626,8 @@ bool CTransaction::AddToMemoryPoolUnchecked() bool CTransaction::RemoveFromMemoryPool() { // Remove transaction from memory pool - CRITICAL_BLOCK(cs_mapTransactions) { + LOCK(cs_mapTransactions); uint256 hash = GetHash(); if (mapTransactions.count(hash)) { @@ -702,8 +704,9 @@ bool CMerkleTx::AcceptToMemoryPool() bool CWalletTx::AcceptWalletTransaction(CTxDB& txdb, bool fCheckInputs) { - CRITICAL_BLOCK(cs_mapTransactions) + { + LOCK(cs_mapTransactions); // Add previous supporting transactions first BOOST_FOREACH(CMerkleTx& tx, vtxPrev) { @@ -1024,8 +1027,8 @@ bool CTransaction::FetchInputs(CTxDB& txdb, const map& mapTes if (!fFound || txindex.pos == CDiskTxPos(1,1,1)) { // Get prev tx from single transactions in memory - CRITICAL_BLOCK(cs_mapTransactions) { + LOCK(cs_mapTransactions); if (!mapTransactions.count(prevout.hash)) return error("FetchInputs() : %s mapTransactions prev not found %s", GetHash().ToString().substr(0,10).c_str(), prevout.hash.ToString().substr(0,10).c_str()); txPrev = mapTransactions[prevout.hash]; @@ -1190,8 +1193,8 @@ bool CTransaction::ClientConnectInputs() return false; // Take over previous transactions' spent pointers - CRITICAL_BLOCK(cs_mapTransactions) { + LOCK(cs_mapTransactions); int64 nValueIn = 0; for (int i = 0; i < vin.size(); i++) { @@ -1707,10 +1710,12 @@ bool CBlock::AcceptBlock() // Relay inventory, but don't relay old inventory during initial block download int nBlockEstimate = Checkpoints::GetTotalBlocksEstimate(); if (hashBestChain == hash) - CRITICAL_BLOCK(cs_vNodes) - BOOST_FOREACH(CNode* pnode, vNodes) - if (nBestHeight > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : nBlockEstimate)) - pnode->PushInventory(CInv(MSG_BLOCK, hash)); + { + LOCK(cs_vNodes); + BOOST_FOREACH(CNode* pnode, vNodes) + if (nBestHeight > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : nBlockEstimate)) + pnode->PushInventory(CInv(MSG_BLOCK, hash)); + } return true; } @@ -2052,8 +2057,8 @@ string GetWarnings(string strFor) } // Alerts - CRITICAL_BLOCK(cs_mapAlerts) { + LOCK(cs_mapAlerts); BOOST_FOREACH(PAIRTYPE(const uint256, CAlert)& item, mapAlerts) { const CAlert& alert = item.second; @@ -2080,8 +2085,8 @@ bool CAlert::ProcessAlert() if (!IsInEffect()) return false; - CRITICAL_BLOCK(cs_mapAlerts) { + LOCK(cs_mapAlerts); // Cancel previous alerts for (map::iterator mi = mapAlerts.begin(); mi != mapAlerts.end();) { @@ -2260,9 +2265,11 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) } // Relay alerts - CRITICAL_BLOCK(cs_mapAlerts) + { + LOCK(cs_mapAlerts); BOOST_FOREACH(PAIRTYPE(const uint256, CAlert)& item, mapAlerts) item.second.RelayTo(pfrom); + } pfrom->fSuccessfullyConnected = true; @@ -2316,8 +2323,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) if (addr.nTime > nSince && !pfrom->fGetAddr && vAddr.size() <= 10 && addr.IsRoutable()) { // Relay to a limited number of other nodes - CRITICAL_BLOCK(cs_vNodes) { + LOCK(cs_vNodes); // Use deterministic randomness to send to the same nodes for 24 hours // at a time so the setAddrKnowns of the chosen nodes prevent repeats static uint256 hashSalt; @@ -2428,8 +2435,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) else if (inv.IsKnownType()) { // Send stream from relay memory - CRITICAL_BLOCK(cs_mapRelay) { + LOCK(cs_mapRelay); map::iterator mi = mapRelay.find(inv); if (mi != mapRelay.end()) pfrom->PushMessage(inv.GetCommand(), (*mi).second); @@ -2634,8 +2641,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) vRecv >> hashReply; CRequestTracker tracker; - CRITICAL_BLOCK(pfrom->cs_mapRequests) { + LOCK(pfrom->cs_mapRequests); map::iterator mi = pfrom->mapRequests.find(hashReply); if (mi != pfrom->mapRequests.end()) { @@ -2662,9 +2669,11 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) { // Relay pfrom->setKnown.insert(alert.GetHash()); - CRITICAL_BLOCK(cs_vNodes) + { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) alert.RelayTo(pnode); + } } } @@ -2763,8 +2772,10 @@ bool ProcessMessages(CNode* pfrom) bool fRet = false; try { - CRITICAL_BLOCK(cs_main) + { + LOCK(cs_main); fRet = ProcessMessage(pfrom, strCommand, vMsg); + } if (fShutdown) return true; } @@ -2802,8 +2813,8 @@ bool ProcessMessages(CNode* pfrom) bool SendMessages(CNode* pto, bool fSendTrickle) { - CRITICAL_BLOCK(cs_main) { + LOCK(cs_main); // Don't send anything until we get their version message if (pto->nVersion == 0) return true; @@ -2819,8 +2830,8 @@ bool SendMessages(CNode* pto, bool fSendTrickle) static int64 nLastRebroadcast; if (!IsInitialBlockDownload() && (GetTime() - nLastRebroadcast > 24 * 60 * 60)) { - CRITICAL_BLOCK(cs_vNodes) { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) { // Periodically clear setAddrKnown to allow refresh broadcasts @@ -2871,8 +2882,8 @@ bool SendMessages(CNode* pto, bool fSendTrickle) // vector vInv; vector vInvWait; - CRITICAL_BLOCK(pto->cs_inventory) { + LOCK(pto->cs_inventory); vInv.reserve(pto->vInventoryToSend.size()); vInvWait.reserve(pto->vInventoryToSend.size()); BOOST_FOREACH(const CInv& inv, pto->vInventoryToSend) @@ -3087,9 +3098,8 @@ CBlock* CreateNewBlock(CReserveKey& reservekey) // Collect memory pool transactions into the block int64 nFees = 0; - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(cs_mapTransactions) { + LOCK2(cs_main, cs_mapTransactions); CTxDB txdb("r"); // Priority order to process transactions @@ -3317,8 +3327,8 @@ bool CheckWork(CBlock* pblock, CWallet& wallet, CReserveKey& reservekey) printf("generated %s\n", FormatMoney(pblock->vtx[0].vout[0].nValue).c_str()); // Found a solution - CRITICAL_BLOCK(cs_main) { + LOCK(cs_main); if (pblock->hashPrevBlock != hashBestChain) return error("BitcoinMiner : generated block is stale"); @@ -3326,8 +3336,10 @@ bool CheckWork(CBlock* pblock, CWallet& wallet, CReserveKey& reservekey) reservekey.KeepKey(); // Track how many getdata requests this block gets - CRITICAL_BLOCK(wallet.cs_wallet) + { + LOCK(wallet.cs_wallet); wallet.mapRequestCount[pblock->GetHash()] = 0; + } // Process this block the same as if we had received it from another node if (!ProcessBlock(NULL, pblock)) @@ -3443,8 +3455,8 @@ void static BitcoinMiner(CWallet *pwallet) if (GetTimeMillis() - nHPSTimerStart > 4000) { static CCriticalSection cs; - CRITICAL_BLOCK(cs) { + LOCK(cs); if (GetTimeMillis() - nHPSTimerStart > 4000) { dHashesPerSec = 1000.0 * nHashCounter / (GetTimeMillis() - nHPSTimerStart); diff --git a/src/net.cpp b/src/net.cpp index 59bace41..8272b255 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -272,9 +272,11 @@ void ThreadGetMyExternalIP(void* parg) // setAddrKnown automatically filters any duplicate sends. CAddress addr(addrLocalHost); addr.nTime = GetAdjustedTime(); - CRITICAL_BLOCK(cs_vNodes) + { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) pnode->PushAddress(addr); + } } } } @@ -296,8 +298,8 @@ void AddressCurrentlyConnected(const CService& addr) CNode* FindNode(const CNetAddr& ip) { - CRITICAL_BLOCK(cs_vNodes) { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) if ((CNetAddr)pnode->addr == ip) return (pnode); @@ -307,8 +309,8 @@ CNode* FindNode(const CNetAddr& ip) CNode* FindNode(const CService& addr) { - CRITICAL_BLOCK(cs_vNodes) { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) if ((CService)pnode->addr == addr) return (pnode); @@ -362,10 +364,14 @@ CNode* ConnectNode(CAddress addrConnect, int64 nTimeout) pnode->AddRef(nTimeout); else pnode->AddRef(); - CRITICAL_BLOCK(cs_vNodes) + { + LOCK(cs_vNodes); vNodes.push_back(pnode); - WAITABLE_CRITICAL_BLOCK(csOutbound) + } + { + WAITABLE_LOCK(csOutbound); nOutbound++; + } pnode->nTimeConnected = GetTime(); return pnode; @@ -421,8 +427,8 @@ void CNode::ClearBanned() bool CNode::IsBanned(CNetAddr ip) { bool fResult = false; - CRITICAL_BLOCK(cs_setBanned) { + LOCK(cs_setBanned); std::map::iterator i = setBanned.find(ip); if (i != setBanned.end()) { @@ -446,9 +452,11 @@ bool CNode::Misbehaving(int howmuch) if (nMisbehavior >= GetArg("-banscore", 100)) { int64 banTime = GetTime()+GetArg("-bantime", 60*60*24); // Default 24-hour ban - CRITICAL_BLOCK(cs_setBanned) + { + LOCK(cs_setBanned); if (setBanned[addr] < banTime) setBanned[addr] = banTime; + } CloseSocketDisconnect(); printf("Disconnected %s for misbehavior (score=%d)\n", addr.ToString().c_str(), nMisbehavior); return true; @@ -497,8 +505,8 @@ void ThreadSocketHandler2(void* parg) // // Disconnect nodes // - CRITICAL_BLOCK(cs_vNodes) { + LOCK(cs_vNodes); // Disconnect unused nodes vector vNodesCopy = vNodes; BOOST_FOREACH(CNode* pnode, vNodesCopy) @@ -510,8 +518,8 @@ void ThreadSocketHandler2(void* parg) vNodes.erase(remove(vNodes.begin(), vNodes.end(), pnode), vNodes.end()); if (!pnode->fInbound) - WAITABLE_CRITICAL_BLOCK(csOutbound) { + WAITABLE_LOCK(csOutbound); nOutbound--; // Connection slot(s) were removed, notify connection creator(s) @@ -538,11 +546,23 @@ void ThreadSocketHandler2(void* parg) if (pnode->GetRefCount() <= 0) { bool fDelete = false; - TRY_CRITICAL_BLOCK(pnode->cs_vSend) - TRY_CRITICAL_BLOCK(pnode->cs_vRecv) - TRY_CRITICAL_BLOCK(pnode->cs_mapRequests) - TRY_CRITICAL_BLOCK(pnode->cs_inventory) - fDelete = true; + { + TRY_LOCK(pnode->cs_vSend, lockSend); + if (lockSend) + { + TRY_LOCK(pnode->cs_vRecv, lockRecv); + if (lockRecv) + { + TRY_LOCK(pnode->cs_mapRequests, lockReq); + if (lockReq) + { + TRY_LOCK(pnode->cs_inventory, lockInv); + if (lockInv) + fDelete = true; + } + } + } + } if (fDelete) { vNodesDisconnected.remove(pnode); @@ -576,8 +596,8 @@ void ThreadSocketHandler2(void* parg) if(hListenSocket != INVALID_SOCKET) FD_SET(hListenSocket, &fdsetRecv); hSocketMax = max(hSocketMax, hListenSocket); - CRITICAL_BLOCK(cs_vNodes) { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) { if (pnode->hSocket == INVALID_SOCKET) @@ -585,9 +605,11 @@ void ThreadSocketHandler2(void* parg) FD_SET(pnode->hSocket, &fdsetRecv); FD_SET(pnode->hSocket, &fdsetError); hSocketMax = max(hSocketMax, pnode->hSocket); - TRY_CRITICAL_BLOCK(pnode->cs_vSend) - if (!pnode->vSend.empty()) + { + TRY_LOCK(pnode->cs_vSend, lockSend); + if (lockSend && !pnode->vSend.empty()) FD_SET(pnode->hSocket, &fdsetSend); + } } } @@ -625,10 +647,12 @@ void ThreadSocketHandler2(void* parg) if (hSocket != INVALID_SOCKET) addr = CAddress(sockaddr); - CRITICAL_BLOCK(cs_vNodes) + { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) - if (pnode->fInbound) - nInbound++; + if (pnode->fInbound) + nInbound++; + } if (hSocket == INVALID_SOCKET) { @@ -637,9 +661,11 @@ void ThreadSocketHandler2(void* parg) } else if (nInbound >= GetArg("-maxconnections", 125) - MAX_OUTBOUND_CONNECTIONS) { - CRITICAL_BLOCK(cs_setservAddNodeAddresses) + { + LOCK(cs_setservAddNodeAddresses); if (!setservAddNodeAddresses.count(addr)) closesocket(hSocket); + } } else if (CNode::IsBanned(addr)) { @@ -651,8 +677,10 @@ void ThreadSocketHandler2(void* parg) printf("accepted connection %s\n", addr.ToString().c_str()); CNode* pnode = new CNode(hSocket, addr, true); pnode->AddRef(); - CRITICAL_BLOCK(cs_vNodes) + { + LOCK(cs_vNodes); vNodes.push_back(pnode); + } } } @@ -661,8 +689,8 @@ void ThreadSocketHandler2(void* parg) // Service each socket // vector vNodesCopy; - CRITICAL_BLOCK(cs_vNodes) { + LOCK(cs_vNodes); vNodesCopy = vNodes; BOOST_FOREACH(CNode* pnode, vNodesCopy) pnode->AddRef(); @@ -679,7 +707,8 @@ void ThreadSocketHandler2(void* parg) continue; if (FD_ISSET(pnode->hSocket, &fdsetRecv) || FD_ISSET(pnode->hSocket, &fdsetError)) { - TRY_CRITICAL_BLOCK(pnode->cs_vRecv) + TRY_LOCK(pnode->cs_vRecv, lockRecv); + if (lockRecv) { CDataStream& vRecv = pnode->vRecv; unsigned int nPos = vRecv.size(); @@ -728,7 +757,8 @@ void ThreadSocketHandler2(void* parg) continue; if (FD_ISSET(pnode->hSocket, &fdsetSend)) { - TRY_CRITICAL_BLOCK(pnode->cs_vSend) + TRY_LOCK(pnode->cs_vSend, lockSend); + if (lockSend) { CDataStream& vSend = pnode->vSend; if (!vSend.empty()) @@ -782,8 +812,8 @@ void ThreadSocketHandler2(void* parg) } } } - CRITICAL_BLOCK(cs_vNodes) { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodesCopy) pnode->Release(); } @@ -1195,8 +1225,10 @@ void ThreadOpenConnections2(void* parg) // Limit outbound connections int nMaxOutbound = min(MAX_OUTBOUND_CONNECTIONS, (int)GetArg("-maxconnections", 125)); vnThreadsRunning[THREAD_OPENCONNECTIONS]--; - WAITABLE_CRITICAL_BLOCK(csOutbound) + { + WAITABLE_LOCK(csOutbound); WAIT(condOutbound, fShutdown || nOutbound < nMaxOutbound); + } vnThreadsRunning[THREAD_OPENCONNECTIONS]++; if (fShutdown) return; @@ -1233,9 +1265,11 @@ void ThreadOpenConnections2(void* parg) // Only connect to one address per a.b.?.? range. // Do this here so we don't have to critsect vNodes inside mapAddresses critsect. set > setConnected; - CRITICAL_BLOCK(cs_vNodes) + { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) setConnected.insert(pnode->addr.GetGroup()); + } int64 nANow = GetAdjustedTime(); @@ -1301,9 +1335,11 @@ void ThreadOpenAddedConnections2(void* parg) if(Lookup(strAddNode.c_str(), vservNode, GetDefaultPort(), fAllowDNS, 0)) { vservAddressesToAdd.push_back(vservNode); - CRITICAL_BLOCK(cs_setservAddNodeAddresses) + { + LOCK(cs_setservAddNodeAddresses); BOOST_FOREACH(CService& serv, vservNode) setservAddNodeAddresses.insert(serv); + } } } loop @@ -1311,7 +1347,8 @@ void ThreadOpenAddedConnections2(void* parg) vector > vservConnectAddresses = vservAddressesToAdd; // Attempt to connect to each IP for each addnode entry until at least one is successful per addnode entry // (keeping in mind that addnode entries can have many IPs if fAllowDNS) - CRITICAL_BLOCK(cs_vNodes) + { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) for (vector >::iterator it = vservConnectAddresses.begin(); it != vservConnectAddresses.end(); it++) BOOST_FOREACH(CService& addrNode, *(it)) @@ -1321,6 +1358,7 @@ void ThreadOpenAddedConnections2(void* parg) it--; break; } + } BOOST_FOREACH(vector& vserv, vservConnectAddresses) { OpenNetworkConnection(CAddress(*(vserv.begin()))); @@ -1394,8 +1432,8 @@ void ThreadMessageHandler2(void* parg) while (!fShutdown) { vector vNodesCopy; - CRITICAL_BLOCK(cs_vNodes) { + LOCK(cs_vNodes); vNodesCopy = vNodes; BOOST_FOREACH(CNode* pnode, vNodesCopy) pnode->AddRef(); @@ -1408,20 +1446,26 @@ void ThreadMessageHandler2(void* parg) BOOST_FOREACH(CNode* pnode, vNodesCopy) { // Receive messages - TRY_CRITICAL_BLOCK(pnode->cs_vRecv) - ProcessMessages(pnode); + { + TRY_LOCK(pnode->cs_vRecv, lockRecv); + if (lockRecv) + ProcessMessages(pnode); + } if (fShutdown) return; // Send messages - TRY_CRITICAL_BLOCK(pnode->cs_vSend) - SendMessages(pnode, pnode == pnodeTrickle); + { + TRY_LOCK(pnode->cs_vSend, lockSend); + if (lockSend) + SendMessages(pnode, pnode == pnodeTrickle); + } if (fShutdown) return; } - CRITICAL_BLOCK(cs_vNodes) { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodesCopy) pnode->Release(); } diff --git a/src/net.h b/src/net.h index cd707e73..3c84650c 100644 --- a/src/net.h +++ b/src/net.h @@ -247,15 +247,19 @@ public: void AddInventoryKnown(const CInv& inv) { - CRITICAL_BLOCK(cs_inventory) + { + LOCK(cs_inventory); setInventoryKnown.insert(inv); + } } void PushInventory(const CInv& inv) { - CRITICAL_BLOCK(cs_inventory) + { + LOCK(cs_inventory); if (!setInventoryKnown.count(inv)) vInventoryToSend.push_back(inv); + } } void AskFor(const CInv& inv) @@ -519,8 +523,10 @@ public: uint256 hashReply; RAND_bytes((unsigned char*)&hashReply, sizeof(hashReply)); - CRITICAL_BLOCK(cs_mapRequests) + { + LOCK(cs_mapRequests); mapRequests[hashReply] = CRequestTracker(fn, param1); + } PushMessage(pszCommand, hashReply); } @@ -532,8 +538,10 @@ public: uint256 hashReply; RAND_bytes((unsigned char*)&hashReply, sizeof(hashReply)); - CRITICAL_BLOCK(cs_mapRequests) + { + LOCK(cs_mapRequests); mapRequests[hashReply] = CRequestTracker(fn, param1); + } PushMessage(pszCommand, hashReply, a1); } @@ -545,8 +553,10 @@ public: uint256 hashReply; RAND_bytes((unsigned char*)&hashReply, sizeof(hashReply)); - CRITICAL_BLOCK(cs_mapRequests) + { + LOCK(cs_mapRequests); mapRequests[hashReply] = CRequestTracker(fn, param1); + } PushMessage(pszCommand, hashReply, a1, a2); } @@ -592,9 +602,11 @@ public: inline void RelayInventory(const CInv& inv) { // Put on lists to offer to the other nodes - CRITICAL_BLOCK(cs_vNodes) + { + LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) pnode->PushInventory(inv); + } } template @@ -609,8 +621,8 @@ void RelayMessage(const CInv& inv, const T& a) template<> inline void RelayMessage<>(const CInv& inv, const CDataStream& ss) { - CRITICAL_BLOCK(cs_mapRelay) { + LOCK(cs_mapRelay); // Expire old relay messages while (!vRelayExpiration.empty() && vRelayExpiration.front().first < GetTime()) { diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index 198a857b..05f3a816 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -39,8 +39,8 @@ struct AddressTablePriv { cachedAddressTable.clear(); - CRITICAL_BLOCK(wallet->cs_wallet) { + LOCK(wallet->cs_wallet); BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, std::string)& item, wallet->mapAddressBook) { const CBitcoinAddress& address = item.first; @@ -169,8 +169,8 @@ bool AddressTableModel::setData(const QModelIndex & index, const QVariant & valu // Double-check that we're not overwriting a receiving address if(rec->type == AddressTableEntry::Sending) { - CRITICAL_BLOCK(wallet->cs_wallet) { + LOCK(wallet->cs_wallet); // Remove old entry wallet->DelAddressBookName(rec->address.toStdString()); // Add new entry with new address @@ -254,8 +254,8 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con return QString(); } // Check for duplicate addresses - CRITICAL_BLOCK(wallet->cs_wallet) { + LOCK(wallet->cs_wallet); if(wallet->mapAddressBook.count(strAddress)) { editStatus = DUPLICATE_ADDRESS; @@ -286,8 +286,10 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con return QString(); } // Add entry - CRITICAL_BLOCK(wallet->cs_wallet) + { + LOCK(wallet->cs_wallet); wallet->SetAddressBookName(strAddress, strLabel); + } return QString::fromStdString(strAddress); } @@ -301,8 +303,8 @@ bool AddressTableModel::removeRows(int row, int count, const QModelIndex & paren // Also refuse to remove receiving addresses. return false; } - CRITICAL_BLOCK(wallet->cs_wallet) { + LOCK(wallet->cs_wallet); wallet->DelAddressBookName(rec->address.toStdString()); } return true; @@ -312,8 +314,8 @@ bool AddressTableModel::removeRows(int row, int count, const QModelIndex & paren */ QString AddressTableModel::labelForAddress(const QString &address) const { - CRITICAL_BLOCK(wallet->cs_wallet) { + LOCK(wallet->cs_wallet); CBitcoinAddress address_parsed(address.toStdString()); std::map::iterator mi = wallet->mapAddressBook.find(address_parsed); if (mi != wallet->mapAddressBook.end()) diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index c32a006f..dd7dd613 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -34,8 +34,9 @@ QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx) QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx) { QString strHTML; - CRITICAL_BLOCK(wallet->cs_wallet) + { + LOCK(wallet->cs_wallet); strHTML.reserve(4000); strHTML += ""; @@ -243,8 +244,9 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx) strHTML += "
Inputs:"; strHTML += "
    "; - CRITICAL_BLOCK(wallet->cs_wallet) + { + LOCK(wallet->cs_wallet); BOOST_FOREACH(const CTxIn& txin, wtx.vin) { COutPoint prevout = txin.prevout; diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 480d4ac2..aa11df97 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -69,8 +69,8 @@ struct TransactionTablePriv qDebug() << "refreshWallet"; #endif cachedWallet.clear(); - CRITICAL_BLOCK(wallet->cs_wallet) { + LOCK(wallet->cs_wallet); for(std::map::iterator it = wallet->mapWallet.begin(); it != wallet->mapWallet.end(); ++it) { cachedWallet.append(TransactionRecord::decomposeTransaction(wallet, it->second)); @@ -95,8 +95,8 @@ struct TransactionTablePriv QList updated_sorted = updated; qSort(updated_sorted); - CRITICAL_BLOCK(wallet->cs_wallet) { + LOCK(wallet->cs_wallet); for(int update_idx = updated_sorted.size()-1; update_idx >= 0; --update_idx) { const uint256 &hash = updated_sorted.at(update_idx); @@ -171,8 +171,8 @@ struct TransactionTablePriv // simply re-use the cached status. if(rec->statusUpdateNeeded()) { - CRITICAL_BLOCK(wallet->cs_wallet) { + LOCK(wallet->cs_wallet); std::map::iterator mi = wallet->mapWallet.find(rec->hash); if(mi != wallet->mapWallet.end()) @@ -191,8 +191,8 @@ struct TransactionTablePriv QString describe(TransactionRecord *rec) { - CRITICAL_BLOCK(wallet->cs_wallet) { + LOCK(wallet->cs_wallet); std::map::iterator mi = wallet->mapWallet.find(rec->hash); if(mi != wallet->mapWallet.end()) { @@ -229,9 +229,9 @@ void TransactionTableModel::update() QList updated; // Check if there are changes to wallet map - TRY_CRITICAL_BLOCK(wallet->cs_wallet) { - if(!wallet->vWalletUpdated.empty()) + TRY_LOCK(wallet->cs_wallet, lockWallet); + if (lockWallet && !wallet->vWalletUpdated.empty()) { BOOST_FOREACH(uint256 hash, wallet->vWalletUpdated) { diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 6cc02379..9c28a8ab 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -32,8 +32,8 @@ qint64 WalletModel::getUnconfirmedBalance() const int WalletModel::getNumTransactions() const { int numTransactions = 0; - CRITICAL_BLOCK(wallet->cs_wallet) { + LOCK(wallet->cs_wallet); numTransactions = wallet->mapWallet.size(); } return numTransactions; @@ -115,9 +115,9 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(const QListcs_wallet) { + LOCK2(cs_main, wallet->cs_wallet); + // Sendmany std::vector > vecSend; foreach(const SendCoinsRecipient &rcp, recipients) @@ -155,8 +155,8 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(const QListcs_wallet) { + LOCK(wallet->cs_wallet); if (!wallet->mapAddressBook.count(strAddress)) wallet->SetAddressBookName(strAddress, rcp.label.toStdString()); } @@ -227,8 +227,8 @@ bool WalletModel::setWalletLocked(bool locked, const SecureString &passPhrase) bool WalletModel::changePassphrase(const SecureString &oldPass, const SecureString &newPass) { bool retval; - CRITICAL_BLOCK(wallet->cs_wallet) { + LOCK(wallet->cs_wallet); wallet->Lock(); // Make sure wallet is locked before attempting pass change retval = wallet->ChangeWalletPassphrase(oldPass, newPass); } diff --git a/src/rpcdump.cpp b/src/rpcdump.cpp index 8180aadf..5bb4789c 100644 --- a/src/rpcdump.cpp +++ b/src/rpcdump.cpp @@ -60,9 +60,9 @@ Value importprivkey(const Array& params, bool fHelp) key.SetSecret(secret, fCompressed); CBitcoinAddress vchAddress = CBitcoinAddress(key.GetPubKey()); - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(pwalletMain->cs_wallet) { + LOCK2(cs_main, pwalletMain->cs_wallet); + pwalletMain->MarkDirty(); pwalletMain->SetAddressBookName(vchAddress, strLabel); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 94c0c77a..db31f498 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -15,14 +15,15 @@ BOOST_AUTO_TEST_CASE(util_criticalsection) CCriticalSection cs; do { - CRITICAL_BLOCK(cs) - break; + LOCK(cs); + break; BOOST_ERROR("break was swallowed!"); } while(0); do { - TRY_CRITICAL_BLOCK(cs) + TRY_LOCK(cs, lockTest); + if (lockTest) break; BOOST_ERROR("break was swallowed!"); diff --git a/src/util.cpp b/src/util.cpp index d55e7ae1..fd4847b1 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -195,8 +195,8 @@ inline int OutputDebugStringF(const char* pszFormat, ...) static CCriticalSection cs_OutputDebugStringF; // accumulate a line at a time - CRITICAL_BLOCK(cs_OutputDebugStringF) { + LOCK(cs_OutputDebugStringF); static char pszBuffer[50000]; static char* pend; if (pend == NULL) diff --git a/src/util.h b/src/util.h index 635790b7..7027e62b 100644 --- a/src/util.h +++ b/src/util.h @@ -282,13 +282,10 @@ typedef boost::interprocess::interprocess_condition CConditionVariable; #define NOTIFY_ALL(name) \ do { (name).notify_all(); } while(0) -#define CRITICAL_BLOCK(cs) \ - for (bool fcriticalblockonce=true; fcriticalblockonce; assert(("break caught by CRITICAL_BLOCK!" && !fcriticalblockonce)), fcriticalblockonce=false) \ - for (CCriticalBlock criticalblock(cs, #cs, __FILE__, __LINE__); fcriticalblockonce; fcriticalblockonce=false) - -#define WAITABLE_CRITICAL_BLOCK(cs) \ - for (bool fcriticalblockonce=true; fcriticalblockonce; assert(("break caught by WAITABLE_CRITICAL_BLOCK!" && !fcriticalblockonce)), fcriticalblockonce=false) \ - for (CWaitableCriticalBlock waitablecriticalblock(cs, #cs, __FILE__, __LINE__); fcriticalblockonce; fcriticalblockonce=false) +#define LOCK(cs) CCriticalBlock criticalblock(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 WAITABLE_LOCK(cs) CWaitableCriticalBlock waitablecriticalblock(cs, #cs, __FILE__, __LINE__) #define ENTER_CRITICAL_SECTION(cs) \ { \ @@ -302,10 +299,6 @@ typedef boost::interprocess::interprocess_condition CConditionVariable; LeaveCritical(); \ } -#define TRY_CRITICAL_BLOCK(cs) \ - for (bool fcriticalblockonce=true; fcriticalblockonce; assert(("break caught by TRY_CRITICAL_BLOCK!" && !fcriticalblockonce)), fcriticalblockonce=false) \ - for (CCriticalBlock criticalblock(cs, #cs, __FILE__, __LINE__, true); fcriticalblockonce && (fcriticalblockonce = criticalblock); fcriticalblockonce=false) - // This is exactly like std::string, but with a custom allocator. // (secure_allocator<> is defined in serialize.h) diff --git a/src/wallet.cpp b/src/wallet.cpp index e444e5f6..97ed6aa5 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -49,8 +49,8 @@ bool CWallet::AddCryptedKey(const vector &vchPubKey, const vector return false; if (!fFileBacked) return true; - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); if (pwalletdbEncryption) return pwalletdbEncryption->WriteCryptedKey(vchPubKey, vchCryptedSecret); else @@ -76,7 +76,8 @@ bool CWallet::Unlock(const SecureString& strWalletPassphrase) CCrypter crypter; CKeyingMaterial vMasterKey; - CRITICAL_BLOCK(cs_wallet) + { + LOCK(cs_wallet); BOOST_FOREACH(const MasterKeyMap::value_type& pMasterKey, mapMasterKeys) { if(!crypter.SetKeyFromPassphrase(strWalletPassphrase, pMasterKey.second.vchSalt, pMasterKey.second.nDeriveIterations, pMasterKey.second.nDerivationMethod)) @@ -86,6 +87,7 @@ bool CWallet::Unlock(const SecureString& strWalletPassphrase) if (CCryptoKeyStore::Unlock(vMasterKey)) return true; } + } return false; } @@ -93,8 +95,8 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, { bool fWasLocked = IsLocked(); - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); Lock(); CCrypter crypter; @@ -228,8 +230,8 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) if (!crypter.Encrypt(vMasterKey, kMasterKey.vchCryptedKey)) return false; - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); mapMasterKeys[++nMasterKeyMaxID] = kMasterKey; if (fFileBacked) { @@ -275,8 +277,8 @@ void CWallet::WalletUpdateSpent(const CTransaction &tx) // Anytime a signature is successfully verified, it's proof the outpoint is spent. // Update the wallet spent flag if it doesn't know due to wallet.dat being // restored from backup or the user making copies of wallet.dat. - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); BOOST_FOREACH(const CTxIn& txin, tx.vin) { map::iterator mi = mapWallet.find(txin.prevout.hash); @@ -297,8 +299,8 @@ void CWallet::WalletUpdateSpent(const CTransaction &tx) void CWallet::MarkDirty() { - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); BOOST_FOREACH(PAIRTYPE(const uint256, CWalletTx)& item, mapWallet) item.second.MarkDirty(); } @@ -307,8 +309,8 @@ void CWallet::MarkDirty() bool CWallet::AddToWallet(const CWalletTx& wtxIn) { uint256 hash = wtxIn.GetHash(); - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); // Inserts only if not already there, returns tx inserted or tx found pair::iterator, bool> ret = mapWallet.insert(make_pair(hash, wtxIn)); CWalletTx& wtx = (*ret.first).second; @@ -382,8 +384,8 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn) bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate, bool fFindBlock) { uint256 hash = tx.GetHash(); - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); bool fExisted = mapWallet.count(hash); if (fExisted && !fUpdate) return false; if (fExisted || IsMine(tx) || IsFromMe(tx)) @@ -404,8 +406,8 @@ bool CWallet::EraseFromWallet(uint256 hash) { if (!fFileBacked) return false; - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); if (mapWallet.erase(hash)) CWalletDB(strWalletFile).EraseTx(hash); } @@ -415,8 +417,8 @@ bool CWallet::EraseFromWallet(uint256 hash) bool CWallet::IsMine(const CTxIn &txin) const { - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); map::const_iterator mi = mapWallet.find(txin.prevout.hash); if (mi != mapWallet.end()) { @@ -431,8 +433,8 @@ bool CWallet::IsMine(const CTxIn &txin) const int64 CWallet::GetDebit(const CTxIn &txin) const { - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); map::const_iterator mi = mapWallet.find(txin.prevout.hash); if (mi != mapWallet.end()) { @@ -457,9 +459,11 @@ bool CWallet::IsChange(const CTxOut& txout) const // 'the change' will need to be implemented (maybe extend CWalletTx to remember // which output, if any, was change). if (ExtractAddress(txout.scriptPubKey, address) && HaveKey(address)) - CRITICAL_BLOCK(cs_wallet) - if (!mapAddressBook.count(address)) - return true; + { + LOCK(cs_wallet); + if (!mapAddressBook.count(address)) + return true; + } return false; } @@ -472,8 +476,8 @@ int CWalletTx::GetRequestCount() const { // Returns -1 if it wasn't being tracked int nRequests = -1; - CRITICAL_BLOCK(pwallet->cs_wallet) { + LOCK(pwallet->cs_wallet); if (IsCoinBase()) { // Generated block @@ -577,8 +581,8 @@ void CWalletTx::GetAccountAmounts(const string& strAccount, int64& nGenerated, i nSent += s.second; nFee = allFee; } - CRITICAL_BLOCK(pwallet->cs_wallet) { + LOCK(pwallet->cs_wallet); BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress,int64)& r, listReceived) { if (pwallet->mapAddressBook.count(r.first)) @@ -607,8 +611,8 @@ void CWalletTx::AddSupportingTransactions(CTxDB& txdb) vWorkQueue.push_back(txin.prevout.hash); // This critsect is OK because txdb is already open - CRITICAL_BLOCK(pwallet->cs_wallet) { + LOCK(pwallet->cs_wallet); map mapWalletPrev; set setAlreadyDone; for (int i = 0; i < vWorkQueue.size(); i++) @@ -666,8 +670,8 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) int ret = 0; CBlockIndex* pindex = pindexStart; - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); while (pindex) { CBlock block; @@ -696,8 +700,9 @@ void CWallet::ReacceptWalletTransactions() { CTxDB txdb("r"); bool fRepeat = true; - while (fRepeat) CRITICAL_BLOCK(cs_wallet) + while (fRepeat) { + LOCK(cs_wallet); fRepeat = false; vector vMissingTx; BOOST_FOREACH(PAIRTYPE(const uint256, CWalletTx)& item, mapWallet) @@ -799,8 +804,8 @@ void CWallet::ResendWalletTransactions() // Rebroadcast any of our txes that aren't in a block yet printf("ResendWalletTransactions()\n"); CTxDB txdb("r"); - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); // Sort them in chronological order multimap mapSorted; BOOST_FOREACH(PAIRTYPE(const uint256, CWalletTx)& item, mapWallet) @@ -833,8 +838,8 @@ void CWallet::ResendWalletTransactions() int64 CWallet::GetBalance() const { int64 nTotal = 0; - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); for (map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) { const CWalletTx* pcoin = &(*it).second; @@ -850,8 +855,8 @@ int64 CWallet::GetBalance() const int64 CWallet::GetUnconfirmedBalance() const { int64 nTotal = 0; - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); for (map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) { const CWalletTx* pcoin = &(*it).second; @@ -875,8 +880,8 @@ bool CWallet::SelectCoinsMinConf(int64 nTargetValue, int nConfMine, int nConfThe vector > > vValue; int64 nTotalLower = 0; - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); vector vCoins; vCoins.reserve(mapWallet.size()); for (map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) @@ -1032,9 +1037,8 @@ bool CWallet::CreateTransaction(const vector >& vecSend, CW wtxNew.BindWallet(this); - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(cs_wallet) { + LOCK2(cs_main, cs_wallet); // txdb must be opened before the mapWallet lock CTxDB txdb("r"); { @@ -1146,9 +1150,8 @@ bool CWallet::CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& w // Call after CreateTransaction unless you want to abort bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey) { - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(cs_wallet) { + LOCK2(cs_main, cs_wallet); printf("CommitTransaction:\n%s", wtxNew.ToString().c_str()); { // This is only to keep the database open to defeat the auto-flush for the @@ -1297,8 +1300,8 @@ bool CWallet::DelAddressBookName(const CBitcoinAddress& address) void CWallet::PrintWallet(const CBlock& block) { - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); if (mapWallet.count(block.vtx[0].GetHash())) { CWalletTx& wtx = mapWallet[block.vtx[0].GetHash()]; @@ -1310,8 +1313,8 @@ void CWallet::PrintWallet(const CBlock& block) bool CWallet::GetTransaction(const uint256 &hashTx, CWalletTx& wtx) { - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); map::iterator mi = mapWallet.find(hashTx); if (mi != mapWallet.end()) { @@ -1347,8 +1350,8 @@ bool GetWalletFile(CWallet* pwallet, string &strWalletFileOut) // bool CWallet::NewKeyPool() { - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); CWalletDB walletdb(strWalletFile); BOOST_FOREACH(int64 nIndex, setKeyPool) walletdb.ErasePool(nIndex); @@ -1371,8 +1374,9 @@ bool CWallet::NewKeyPool() bool CWallet::TopUpKeyPool() { - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); + if (IsLocked()) return false; @@ -1398,8 +1402,9 @@ void CWallet::ReserveKeyFromKeyPool(int64& nIndex, CKeyPool& keypool) { nIndex = -1; keypool.vchPubKey.clear(); - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); + if (!IsLocked()) TopUpKeyPool(); @@ -1422,9 +1427,8 @@ void CWallet::ReserveKeyFromKeyPool(int64& nIndex, CKeyPool& keypool) int64 CWallet::AddReserveKey(const CKeyPool& keypool) { - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(cs_wallet) { + LOCK2(cs_main, cs_wallet); CWalletDB walletdb(strWalletFile); int64 nIndex = 1 + *(--setKeyPool.end()); @@ -1450,8 +1454,10 @@ void CWallet::KeepKey(int64 nIndex) void CWallet::ReturnKey(int64 nIndex) { // Return to key pool - CRITICAL_BLOCK(cs_wallet) + { + LOCK(cs_wallet); setKeyPool.insert(nIndex); + } printf("keypool return %"PRI64d"\n", nIndex); } @@ -1459,8 +1465,8 @@ bool CWallet::GetKeyFromPool(vector& result, bool fAllowReuse) { int64 nIndex = 0; CKeyPool keypool; - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); ReserveKeyFromKeyPool(nIndex, keypool); if (nIndex == -1) { @@ -1530,8 +1536,7 @@ void CWallet::GetAllReserveAddresses(set& setAddress) CWalletDB walletdb(strWalletFile); - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(cs_wallet) + LOCK2(cs_main, cs_wallet); BOOST_FOREACH(const int64& id, setKeyPool) { CKeyPool keypool; diff --git a/src/wallet.h b/src/wallet.h index e1065cff..f864370a 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -211,16 +211,18 @@ public: void UpdatedTransaction(const uint256 &hashTx) { - CRITICAL_BLOCK(cs_wallet) + { + LOCK(cs_wallet); vWalletUpdated.push_back(hashTx); + } } void PrintWallet(const CBlock& block); void Inventory(const uint256 &hash) { - CRITICAL_BLOCK(cs_wallet) { + LOCK(cs_wallet); std::map::iterator mi = mapRequestCount.find(hash); if (mi != mapRequestCount.end()) (*mi).second++;