Restrict cs names

src/blockgeneration/miner.cpp

@@ -150,7 +150,7 @@ std::unique_ptr<CBlockTemplate> CreateNewPoWBlock(CWallet *pwallet, const CScrip
     // Collect memory pool transactions into the block
     {
         LOCK(cs_main);
-        READLOCK(mempool.cs);
+        READLOCK(mempool.cs_txmempool);
         CBlockIndex *_pindexPrev = pnetMan->getChainActive()->chainActive.Tip();
         const int nHeight = _pindexPrev->nHeight + 1;
         pblock->nTime = GetAdjustedTime();
@@ -518,9 +518,9 @@ void EccMiner(CWallet *pwallet)
                 nHashCounter += nHashesDone;
             if (GetTimeMillis() - nHPSTimerStart > 4000)
             {
-                static CCriticalSection cs;
+                static CCriticalSection cs_miner;
                 {
-                    LOCK(cs);
+                    LOCK(cs_miner);
                     if (GetTimeMillis() - nHPSTimerStart > 4000)
                     {
                         dHashesPerSec = 1000.0 * nHashCounter / (GetTimeMillis() - nHPSTimerStart);

src/blockgeneration/minter.cpp

@@ -145,7 +145,7 @@ std::unique_ptr<CBlockTemplate> CreateNewPoSBlock(CWallet *pwallet, const CScrip
     // Collect memory pool transactions into the block
     {
         LOCK(cs_main);
-        READLOCK(mempool.cs);
+        READLOCK(mempool.cs_txmempool);
         CBlockIndex *_pindexPrev = pnetMan->getChainActive()->chainActive.Tip();
         const int nHeight = _pindexPrev->nHeight + 1;
         pblock->nTime = GetAdjustedTime();

src/httpserver.cpp

@@ -69,7 +69,7 @@ class WorkQueue
 {
 private:
     /** Mutex protects entire object */
-    CWaitableCriticalSection cs;
+    CWaitableCriticalSection cs_workqueue;
     CConditionVariable cond;
     /* XXX in C++11 we can use std::unique_ptr here and avoid manual cleanup */
     std::deque<WorkItem *> queue;
@@ -84,12 +84,12 @@ class WorkQueue
         WorkQueue &wq;
         ThreadCounter(WorkQueue &w) : wq(w)
         {
-            std::lock_guard<std::mutex> lock(wq.cs);
+            std::lock_guard<std::mutex> lock(wq.cs_workqueue);
             wq.numThreads += 1;
         }
         ~ThreadCounter()
         {
-            std::lock_guard<std::mutex> lock(wq.cs);
+            std::lock_guard<std::mutex> lock(wq.cs_workqueue);
             wq.numThreads -= 1;
             wq.cond.notify_all();
         }
@@ -111,7 +111,7 @@ class WorkQueue
     /** Enqueue a work item */
     bool Enqueue(WorkItem *item)
     {
-        std::unique_lock<std::mutex> lock(cs);
+        std::unique_lock<std::mutex> lock(cs_workqueue);
         if (queue.size() >= maxDepth)
         {
             return false;
@@ -128,7 +128,7 @@ class WorkQueue
         {
             WorkItem *i = 0;
             {
-                std::unique_lock<std::mutex> lock(cs);
+                std::unique_lock<std::mutex> lock(cs_workqueue);
                 while (running && queue.empty())
                     cond.wait(lock);
                 if (!running)
@@ -143,22 +143,22 @@ class WorkQueue
     /** Interrupt and exit loops */
     void Interrupt()
     {
-        std::unique_lock<std::mutex> lock(cs);
+        std::unique_lock<std::mutex> lock(cs_workqueue);
         running = false;
         cond.notify_all();
     }
     /** Wait for worker threads to exit */
     void WaitExit()
     {
-        std::unique_lock<std::mutex> lock(cs);
+        std::unique_lock<std::mutex> lock(cs_workqueue);
         while (numThreads > 0)
             cond.wait(lock);
     }
 
     /** Return current depth of queue */
     size_t Depth()
     {
-        std::unique_lock<std::mutex> lock(cs);
+        std::unique_lock<std::mutex> lock(cs_workqueue);
         return queue.size();
     }
 };

src/main.cpp

@@ -191,7 +191,7 @@ bool TestLockPointValidity(const LockPoints *lp)
 bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints *lp, bool useExistingLockPoints)
 {
     AssertLockHeld(cs_main);
-    AssertLockHeld(mempool.cs);
+    AssertLockHeld(mempool.cs_txmempool);
 
     CBlockIndex *tip = pnetMan->getChainActive()->chainActive.Tip();
     CBlockIndex index;
@@ -341,7 +341,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool &pool,
 
     // Check for conflicts with in-memory transactions
     {
-        READLOCK(pool.cs); // protect pool.mapNextTx
+        READLOCK(pool.cs_txmempool); // protect pool.mapNextTx
         for (auto const &txin : tx.vin)
         {
             auto itConflicting = pool.mapNextTx.find(txin.prevout);
@@ -360,7 +360,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool &pool,
         CAmount nValueIn = 0;
         LockPoints lp;
         {
-            WRITELOCK(pool.cs);
+            WRITELOCK(pool.cs_txmempool);
             CCoinsViewMemPool viewMemPool(pcoinsTip.get(), pool);
             view.SetBackend(viewMemPool);
 
@@ -406,7 +406,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool &pool,
             // Only accept BIP68 sequence locked transactions that can be mined in the next
             // block; we don't want our mempool filled up with transactions that can't
             // be mined yet.
-            // Must keep pool.cs for this unless we change CheckSequenceLocks to take a
+            // Must keep pool.cs_txmempool for this unless we change CheckSequenceLocks to take a
             // CoinsViewCache instead of create its own
             if (!CheckSequenceLocks(tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp))
             {
@@ -595,7 +595,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool &pool,
                 __func__, hash.ToString(), FormatStateMessage(state));
         }
         {
-            WRITELOCK(pool.cs);
+            WRITELOCK(pool.cs_txmempool);
             if (!pool._CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize,
                     nLimitDescendants, nLimitDescendantSize, errString))
             {
@@ -604,7 +604,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool &pool,
         }
 
         {
-            WRITELOCK(pool.cs);
+            WRITELOCK(pool.cs_txmempool);
             // Store transaction in memory
             pool.addUnchecked(hash, entry, setAncestors, !pnetMan->getChainActive()->IsInitialBlockDownload());
         }

src/net/addrman.h

@@ -189,7 +189,7 @@ class CAddrMan
 {
 private:
     //! critical section to protect the inner data structures
-    mutable CCriticalSection cs;
+    mutable CCriticalSection cs_addrman;
 
     //! last used nId
     int nIdCount;
@@ -311,7 +311,7 @@ class CAddrMan
     template <typename Stream>
     void Serialize(Stream &s) const
     {
-        LOCK(cs);
+        LOCK(cs_addrman);
 
         uint8_t nVersion = 1;
         s << nVersion;
@@ -371,7 +371,7 @@ class CAddrMan
     template <typename Stream>
     void Unserialize(Stream &s)
     {
-        LOCK(cs);
+        LOCK(cs_addrman);
 
         Clear();
 
@@ -530,7 +530,7 @@ class CAddrMan
     size_t size() const
     {
         // TODO: Cache this in an atomic to avoid this overhead
-        LOCK(cs);
+        LOCK(cs_addrman);
         return vRandom.size();
     }
 
@@ -539,7 +539,7 @@ class CAddrMan
     {
 #ifdef DEBUG_ADDRMAN
         {
-            LOCK(cs);
+            LOCK(cs_addrman);
             int err;
             if ((err = Check_()))
                 LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err);
@@ -550,7 +550,7 @@ class CAddrMan
     //! Add a single address.
     bool Add(const CAddress &addr, const CNetAddr &source, int64_t nTimePenalty = 0)
     {
-        LOCK(cs);
+        LOCK(cs_addrman);
         bool fRet = false;
         Check();
         fRet |= Add_(addr, source, nTimePenalty);
@@ -564,7 +564,7 @@ class CAddrMan
     //! Add multiple addresses.
     bool Add(const std::vector<CAddress> &vAddr, const CNetAddr &source, int64_t nTimePenalty = 0)
     {
-        LOCK(cs);
+        LOCK(cs_addrman);
         int nAdd = 0;
         Check();
         for (std::vector<CAddress>::const_iterator it = vAddr.begin(); it != vAddr.end(); it++)
@@ -579,7 +579,7 @@ class CAddrMan
     //! Mark an entry as accessible.
     void Good(const CService &addr, int64_t nTime = GetAdjustedTime())
     {
-        LOCK(cs);
+        LOCK(cs_addrman);
         Check();
         Good_(addr, nTime);
         Check();
@@ -588,7 +588,7 @@ class CAddrMan
     //! Mark an entry as connection attempted to.
     void Attempt(const CService &addr, bool fCountFailure, int64_t nTime = GetAdjustedTime())
     {
-        LOCK(cs);
+        LOCK(cs_addrman);
         Check();
         Attempt_(addr, fCountFailure, nTime);
         Check();
@@ -601,7 +601,7 @@ class CAddrMan
     {
         CAddrInfo addrRet;
         {
-            LOCK(cs);
+            LOCK(cs_addrman);
             Check();
             addrRet = Select_(newOnly);
             Check();
@@ -615,7 +615,7 @@ class CAddrMan
         Check();
         std::vector<CAddress> vAddr;
         {
-            LOCK(cs);
+            LOCK(cs_addrman);
             GetAddr_(vAddr);
         }
         Check();
@@ -625,15 +625,15 @@ class CAddrMan
     //! Mark an entry as currently-connected-to.
     void Connected(const CService &addr, int64_t nTime = GetAdjustedTime())
     {
-        LOCK(cs);
+        LOCK(cs_addrman);
         Check();
         Connected_(addr, nTime);
         Check();
     }
 
     void SetServices(const CService &addr, ServiceFlags nServices)
     {
-        LOCK(cs);
+        LOCK(cs_addrman);
         Check();
         SetServices_(addr, nServices);
         Check();

src/net/nodestate.cpp

@@ -20,13 +20,13 @@ CNodeState *CNodesStateManager::_GetNodeState(const NodeId id)
 
 void CNodesStateManager::InitializeNodeState(const CNode *pnode)
 {
-    LOCK(cs);
+    LOCK(cs_nodestateman);
     mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(pnode->GetId()),
         std::forward_as_tuple(pnode->addr, pnode->GetAddrName()));
 }
 
 void CNodesStateManager::RemoveNodeState(const NodeId id)
 {
-    LOCK(cs);
+    LOCK(cs_nodestateman);
     mapNodeState.erase(id);
 }

src/net/nodestate.h

@@ -91,7 +91,7 @@ struct CNodeState
 class CNodesStateManager
 {
 protected:
-    CCriticalSection cs;
+    CCriticalSection cs_nodestateman;
     std::map<NodeId, CNodeState> mapNodeState;
     friend class CNodeStateAccessor;
 
@@ -107,14 +107,14 @@ class CNodesStateManager
     /** Clear the entire nodestate map */
     void Clear()
     {
-        LOCK(cs);
+        LOCK(cs_nodestateman);
         mapNodeState.clear();
     }
 
     /** Is mapNodestate empty */
     bool Empty()
     {
-        LOCK(cs);
+        LOCK(cs_nodestateman);
         return mapNodeState.empty();
     }
 };
@@ -132,7 +132,7 @@ class CNodeStateAccessor
     }
     CNodeStateAccessor(CNodesStateManager &ns, const NodeId id)
     {
-        cs = &ns.cs;
+        cs = &ns.cs_nodestateman;
         EnterCritical("CNodeStateAccessor.cs", __FILE__, __LINE__, (void *)(&cs), LockType::RECURSIVE_MUTEX,
             OwnershipType::EXCLUSIVE);
         obj = ns._GetNodeState(id);

src/rpc/rpcblockchain.cpp

@@ -188,7 +188,7 @@ UniValue mempoolToJSON(bool fVerbose = false)
 {
     if (fVerbose)
     {
-        READLOCK(mempool.cs);
+        READLOCK(mempool.cs_txmempool);
         UniValue o(UniValue::VOBJ);
         for (auto const &e : mempool.mapTx)
         {
@@ -584,7 +584,7 @@ UniValue gettxout(const UniValue &params, bool fHelp)
     Coin coin;
     if (fMempool)
     {
-        READLOCK(mempool.cs);
+        READLOCK(mempool.cs_txmempool);
         CCoinsViewMemPool view(pcoinsTip.get(), mempool);
         if (!view.GetCoin(out, coin) || mempool.isSpent(out))
         {

src/rpc/rpcrawtransaction.cpp

@@ -670,7 +670,7 @@ UniValue signrawtransaction(const UniValue &params, bool fHelp)
     CCoinsView viewDummy;
     CCoinsViewCache view(&viewDummy);
     {
-        READLOCK(mempool.cs);
+        READLOCK(mempool.cs_txmempool);
         CCoinsViewCache &viewChain = *pcoinsTip;
         CCoinsViewMemPool viewMempool(&viewChain, mempool);
         view.SetBackend(viewMempool); // temporarily switch cache backend to db+mempool view

src/sync.h

@@ -298,6 +298,10 @@ class SCOPED_LOCKABLE CMutexLock
         bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn)
         : lock(mutexIn, std::defer_lock)
     {
+        assert(pszName != nullptr);
+        // we no longer allow naming critical sections cs, please name it something more meaningful
+        assert(std::string(pszName) != "cs");
+
         if (fTry)
             TryEnter(pszName, pszFile, nLine, type);
         else
@@ -314,6 +318,9 @@ class SCOPED_LOCKABLE CMutexLock
         if (!pmutexIn)
             return;
 
+        assert(pszName != nullptr);
+        // we no longer allow naming critical sections cs, please name it something more meaningful
+        assert(std::string(pszName) != "cs");
         lock = std::unique_lock<Mutex>(*pmutexIn, std::defer_lock);
         if (fTry)
             TryEnter(pszName, pszFile, nLine, type);
@@ -402,6 +409,10 @@ class SCOPED_LOCKABLE CMutexReadLock
         bool fTry = false) SHARED_LOCK_FUNCTION(mutexIn)
         : lock(mutexIn, std::defer_lock)
     {
+        assert(pszName != nullptr);
+        // we no longer allow naming critical sections cs, please name it something more meaningful
+        assert(std::string(pszName) != "cs");
+
         if (fTry)
             TryEnter(pszName, pszFile, nLine, type);
         else
@@ -418,6 +429,9 @@ class SCOPED_LOCKABLE CMutexReadLock
         if (!pmutexIn)
             return;
 
+        assert(pszName != nullptr);
+        // we no longer allow naming critical sections cs, please name it something more meaningful
+        assert(std::string(pszName) != "cs");
         lock = boost::shared_lock<Mutex>(*pmutexIn, boost::defer_lock);
         if (fTry)
             TryEnter(pszName, pszFile, nLine, type);

src/test/mempool_tests.cpp

@@ -211,7 +211,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
     std::vector<std::string> snapshotOrder;
     {
         CTxMemPool::setEntries setAncestorsCalculated;
-        WRITELOCK(pool.cs);
+        WRITELOCK(pool.cs_txmempool);
         BOOST_CHECK_EQUAL(pool._CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), setAncestorsCalculated, 100,
                               1000000, 1000, 1000000, dummy),
             true);