Fix (dead)locks

src/darksend.cpp

@@ -724,6 +724,8 @@ void CDarksendPool::ChargeFees()
         LogPrintf("CDarksendPool::ChargeFees -- found uncooperative node (didn't %s transaction), charging fees: %s\n",
                 (nState == POOL_STATE_SIGNING) ? "sign" : "send", vecOffendersCollaterals[0].ToString());
 
+        LOCK(cs_main);
+
         CValidationState state;
         bool fMissingInputs;
         if(!AcceptToMemoryPool(mempool, state, vecOffendersCollaterals[0], false, &fMissingInputs, false, true)) {
@@ -751,6 +753,8 @@ void CDarksendPool::ChargeRandomFees()
 {
     if(!fMasterNode) return;
 
+    LOCK(cs_main);
+
     BOOST_FOREACH(const CTransaction& txCollateral, vecSessionCollaterals) {
 
         if(GetRandInt(100) > 10) return;
@@ -773,13 +777,18 @@ void CDarksendPool::ChargeRandomFees()
 //
 void CDarksendPool::CheckTimeout()
 {
-    // check mixing queue objects for timeouts
-    std::vector<CDarksendQueue>::iterator it = vecDarksendQueue.begin();
-    while(it != vecDarksendQueue.end()) {
-        if((*it).IsExpired()) {
-            LogPrint("privatesend", "CDarksendPool::CheckTimeout -- Removing expired queue (%s)\n", (*it).ToString());
-            it = vecDarksendQueue.erase(it);
-        } else ++it;
+    {
+        TRY_LOCK(cs_darksend, lockDS);
+        if(!lockDS) return; // it's ok to fail here, we run this quite frequently
+
+        // check mixing queue objects for timeouts
+        std::vector<CDarksendQueue>::iterator it = vecDarksendQueue.begin();
+        while(it != vecDarksendQueue.end()) {
+            if((*it).IsExpired()) {
+                LogPrint("privatesend", "CDarksendPool::CheckTimeout -- Removing expired queue (%s)\n", (*it).ToString());
+                it = vecDarksendQueue.erase(it);
+            } else ++it;
+        }
     }
 
     if(!fEnablePrivateSend && !fMasterNode) return;
@@ -2335,11 +2344,22 @@ bool CDarksendQueue::CheckSignature(const CPubKey& pubKeyMasternode)
 
 bool CDarksendQueue::Relay()
 {
-    LOCK(cs_vNodes);
-    BOOST_FOREACH(CNode* pnode, vNodes)
+    std::vector<CNode*> vNodesCopy;
+    {
+        LOCK(cs_vNodes);
+        vNodesCopy = vNodes;
+        BOOST_FOREACH(CNode* pnode, vNodesCopy)
+            pnode->AddRef();
+    }
+    BOOST_FOREACH(CNode* pnode, vNodesCopy)
         if(pnode->nVersion >= MIN_PRIVATESEND_PEER_PROTO_VERSION)
             pnode->PushMessage(NetMsgType::DSQUEUE, (*this));
 
+    {
+        LOCK(cs_vNodes);
+        BOOST_FOREACH(CNode* pnode, vNodesCopy)
+            pnode->Release();
+    }
     return true;
 }
 
@@ -2463,6 +2483,8 @@ void ThreadCheckDarkSendPool()
             if(nTick % MASTERNODE_MIN_MNP_SECONDS == 1)
                 activeMasternode.ManageState();
 
+            mnodeman.Check();
+
             if(nTick % 60 == 0) {
                 mnodeman.CheckAndRemove();
                 mnodeman.ProcessMasternodeConnections();

src/governance.cpp

@@ -110,8 +110,6 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, std::string& strCommand, C
 
     if(pfrom->nVersion < MIN_GOVERNANCE_PEER_PROTO_VERSION) return;
 
-    LOCK(cs);
-
     // ANOTHER USER IS ASKING US TO HELP THEM SYNC GOVERNANCE OBJECT DATA
     if (strCommand == NetMsgType::MNGOVERNANCESYNC)
     {
@@ -164,6 +162,8 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, std::string& strCommand, C
             return;
         }
 
+        LOCK(cs);
+
         if(mapSeenGovernanceObjects.count(nHash)) {
             // TODO - print error code? what if it's GOVOBJ_ERROR_IMMATURE?
             LogPrint("gobject", "CGovernanceManager -- Received already seen object: %s\n", strHash);
@@ -845,6 +845,14 @@ void CGovernanceManager::AddCachedTriggers()
     }
 }
 
+void CGovernanceManager::InitOnLoad()
+{
+    LOCK(cs);
+    RebuildIndexes();
+    AddCachedTriggers();
+    ClearSeen();
+}
+
 std::string CGovernanceManager::ToString() const
 {
     std::ostringstream info;

src/governance.h

@@ -210,10 +210,6 @@ class CGovernanceManager
             Clear();
             return;
         }
-        if(ser_action.ForRead()) {
-            RebuildIndexes();
-            AddCachedTriggers();
-        }
     }
 
     void UpdatedBlockTip(const CBlockIndex *pindex);
@@ -250,6 +246,8 @@ class CGovernanceManager
         return fRateChecksEnabled;
     }
 
+    void InitOnLoad();
+
 private:
     void RequestGovernanceObject(CNode* pfrom, const uint256& nHash);
 

src/init.cpp

@@ -1875,7 +1875,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
     if(!flatdb3.Load(governance)) {
         return InitError("Failed to load governance cache from governance.dat");
     }
-    governance.ClearSeen();
+    governance.InitOnLoad();
 
     uiInterface.InitMessage(_("Loading fullfiled requests cache..."));
     CFlatDB<CNetFulfilledRequestManager> flatdb4("netfulfilled.dat", "magicFulfilledCache");

src/instantx.cpp

@@ -72,9 +72,12 @@ bool IsInstantSendTxValid(const CTransaction& txCandidate)
 {
     if(txCandidate.vout.size() < 1) return false;
 
-    if(!CheckFinalTx(txCandidate)) {
-        LogPrint("instantsend", "IsInstantSendTxValid -- Transaction is not final: txCandidate=%s", txCandidate.ToString());
-        return false;
+    {
+        LOCK(cs_main);
+        if(!CheckFinalTx(txCandidate)) {
+            LogPrint("instantsend", "IsInstantSendTxValid -- Transaction is not final: txCandidate=%s", txCandidate.ToString());
+            return false;
+        }
     }
 
     int64_t nValueIn = 0;
@@ -460,16 +463,16 @@ int64_t GetAverageMasternodeOrphanVoteTime()
 
 void CleanTxLockCandidates()
 {
-    LOCK(cs_instantsend);
-
-    std::map<uint256, CTxLockCandidate>::iterator it = mapTxLockCandidates.begin();
-
     int nHeight;
     {
         LOCK(cs_main);
         nHeight = chainActive.Height();
     }
 
+    LOCK(cs_instantsend);
+
+    std::map<uint256, CTxLockCandidate>::iterator it = mapTxLockCandidates.begin();
+
     while(it != mapTxLockCandidates.end()) {
         CTxLockCandidate &txLockCandidate = it->second;
         if(nHeight > txLockCandidate.nExpirationBlock) {

src/main.cpp

@@ -5057,14 +5057,16 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
             {
                 // Send stream from relay memory
                 bool pushed = false;
+                map<CInv, CDataStream>::iterator mi;
                 {
                     LOCK(cs_mapRelay);
-                    map<CInv, CDataStream>::iterator mi = mapRelay.find(inv);
+                    mi = mapRelay.find(inv);
                     if (mi != mapRelay.end()) {
-                        pfrom->PushMessage(inv.GetCommand(), (*mi).second);
                         pushed = true;
                     }
                 }
+                if(pushed)
+                    pfrom->PushMessage(inv.GetCommand(), (*mi).second);
 
                 if (!pushed && inv.type == MSG_TX) {
                     CTransaction tx;

src/masternode-payments.cpp

@@ -416,7 +416,7 @@ bool CMasternodePayments::AddPaymentVote(const CMasternodePaymentVote& vote)
     uint256 blockHash = uint256();
     if(!GetBlockHash(blockHash, vote.nBlockHeight - 101)) return false;
 
-    LOCK2(cs_mapMasternodePaymentVotes, cs_mapMasternodeBlocks);
+    LOCK2(cs_mapMasternodeBlocks, cs_mapMasternodePaymentVotes);
 
     if(mapMasternodePaymentVotes.count(vote.GetHash())) return false;
 
@@ -573,7 +573,7 @@ void CMasternodePayments::CheckAndRemove()
 {
     if(!pCurrentBlockIndex) return;
 
-    LOCK2(cs_mapMasternodePaymentVotes, cs_mapMasternodeBlocks);
+    LOCK2(cs_mapMasternodeBlocks, cs_mapMasternodePaymentVotes);
 
     int nLimit = GetStorageLimit();
 
@@ -781,7 +781,7 @@ void CMasternodePayments::RequestLowDataPaymentBlocks(CNode* pnode)
     // Old nodes can't process this
     if(pnode->nVersion < 70202) return;
 
-    LOCK(cs_mapMasternodeBlocks);
+    LOCK2(cs_main, cs_mapMasternodeBlocks);
 
     std::vector<CInv> vToFetch;
     std::map<int, CMasternodeBlockPayees>::iterator it = mapMasternodeBlocks.begin();

src/masternode-sync.cpp

@@ -162,6 +162,13 @@ void CMasternodeSync::ClearFulfilledRequests()
     }
 }
 
+void ReleaseNodes(const std::vector<CNode*> &vNodesCopy)
+{
+    LOCK(cs_vNodes);
+    BOOST_FOREACH(CNode* pnode, vNodesCopy)
+        pnode->Release();
+}
+
 void CMasternodeSync::ProcessTick()
 {
     static int nTick = 0;
@@ -210,15 +217,21 @@ void CMasternodeSync::ProcessTick()
         return;
     }
 
-    LOCK2(mnodeman.cs, cs_vNodes);
-
     if(nRequestedMasternodeAssets == MASTERNODE_SYNC_INITIAL ||
         (nRequestedMasternodeAssets == MASTERNODE_SYNC_SPORKS && IsBlockchainSynced()))
     {
         SwitchToNextAsset();
     }
 
-    BOOST_FOREACH(CNode* pnode, vNodes)
+    std::vector<CNode*> vNodesCopy;
+    {
+        LOCK(cs_vNodes);
+        vNodesCopy = vNodes;
+        BOOST_FOREACH(CNode* pnode, vNodesCopy)
+            pnode->AddRef();
+    }
+
+    BOOST_FOREACH(CNode* pnode, vNodesCopy)
     {
         // QUICK MODE (REGTEST ONLY!)
         if(Params().NetworkIDString() == CBaseChainParams::REGTEST)
@@ -236,6 +249,7 @@ void CMasternodeSync::ProcessTick()
                 nRequestedMasternodeAssets = MASTERNODE_SYNC_FINISHED;
             }
             nRequestedMasternodeAttempt++;
+            ReleaseNodes(vNodesCopy);
             return;
         }
 
@@ -271,9 +285,11 @@ void CMasternodeSync::ProcessTick()
                         LogPrintf("CMasternodeSync::ProcessTick -- ERROR: failed to sync %s\n", GetAssetName());
                         // there is no way we can continue without masternode list, fail here and try later
                         Fail();
+                        ReleaseNodes(vNodesCopy);
                         return;
                     }
                     SwitchToNextAsset();
+                    ReleaseNodes(vNodesCopy);
                     return;
                 }
 
@@ -288,6 +304,7 @@ void CMasternodeSync::ProcessTick()
                 if(nRequestedMasternodeAttempt > 1 && nMnCount > nMnCountEstimated) {
                     LogPrintf("CMasternodeSync::ProcessTick -- nTick %d nRequestedMasternodeAssets %d -- found enough data\n", nTick, nRequestedMasternodeAssets);
                     SwitchToNextAsset();
+                    ReleaseNodes(vNodesCopy);
                     return;
                 }
 
@@ -300,6 +317,7 @@ void CMasternodeSync::ProcessTick()
 
                 mnodeman.DsegUpdate(pnode);
 
+                ReleaseNodes(vNodesCopy);
                 return; //this will cause each peer to get one request each six seconds for the various assets we need
             }
 
@@ -316,9 +334,11 @@ void CMasternodeSync::ProcessTick()
                         LogPrintf("CMasternodeSync::ProcessTick -- ERROR: failed to sync %s\n", GetAssetName());
                         // probably not a good idea to proceed without winner list
                         Fail();
+                        ReleaseNodes(vNodesCopy);
                         return;
                     }
                     SwitchToNextAsset();
+                    ReleaseNodes(vNodesCopy);
                     return;
                 }
 
@@ -328,6 +348,7 @@ void CMasternodeSync::ProcessTick()
                 if(nRequestedMasternodeAttempt > 1 && mnpayments.IsEnoughData()) {
                     LogPrintf("CMasternodeSync::ProcessTick -- nTick %d nRequestedMasternodeAssets %d -- found enough data\n", nTick, nRequestedMasternodeAssets);
                     SwitchToNextAsset();
+                    ReleaseNodes(vNodesCopy);
                     return;
                 }
 
@@ -343,6 +364,7 @@ void CMasternodeSync::ProcessTick()
                 // ask node for missing pieces only (old nodes will not be asked)
                 mnpayments.RequestLowDataPaymentBlocks(pnode);
 
+                ReleaseNodes(vNodesCopy);
                 return; //this will cause each peer to get one request each six seconds for the various assets we need
             }
 
@@ -359,6 +381,7 @@ void CMasternodeSync::ProcessTick()
                         // it's kind of ok to skip this for now, hopefully we'll catch up later?
                     }
                     SwitchToNextAsset();
+                    ReleaseNodes(vNodesCopy);
                     return;
                 }
 
@@ -384,6 +407,7 @@ void CMasternodeSync::ProcessTick()
 
                 pnode->PushMessage(NetMsgType::MNGOVERNANCESYNC, uint256()); //sync masternode votes
 
+                ReleaseNodes(vNodesCopy);
                 return; //this will cause each peer to get one request each six seconds for the various assets we need
             }
         }