From eb4e6a32ddfddd0fd101f424482d2a767c846d79 Mon Sep 17 00:00:00 2001
From: Tim Flynn <tgflynn@users.noreply.github.com>
Date: Wed, 8 Mar 2017 17:36:40 -0500
Subject: [PATCH] Fix deadlocks on cs_vSend in RequestGovernanceObject (#1387)

---
 src/governance.cpp | 69 +++++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 31 deletions(-)

diff --git a/src/governance.cpp b/src/governance.cpp
index 0d8b22276e..d107c3dc85 100644
--- a/src/governance.cpp
+++ b/src/governance.cpp
@@ -1036,6 +1036,7 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH
     filter.clear();
 
     if(fUseFilter) {
+        LOCK(cs);
         CGovernanceObject* pObj = FindGovernanceObject(nHash);
 
         if(pObj) {
@@ -1064,14 +1065,13 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector<CNode*>&
 
     if(vNodesCopy.empty()) return -1;
 
-    LOCK2(cs_main, cs);
-
-    if(mapObjects.empty()) return -2;
-
     int64_t nNow = GetTime();
     int nTimeout = 60 * 60;
     size_t nPeersPerHashMax = 3;
 
+    std::vector<CGovernanceObject*> vpGovObjsTmp;
+    std::vector<CGovernanceObject*> vpGovObjsTriggersTmp;
+
     // This should help us to get some idea about an impact this can bring once deployed on mainnet.
     // Testnet is ~40 times smaller in masternode count, but only ~1000 masternodes usually vote,
     // so 1 obj on mainnet == ~10 objs or ~1000 votes on testnet. However we want to test a higher
@@ -1083,25 +1083,28 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector<CNode*>&
         nMaxObjRequestsPerNode = std::max(1, int(nProjectedVotes / std::max(1, mnodeman.size())));
     }
 
-    std::vector<CGovernanceObject*> vpGovObjsTmp;
-    std::vector<CGovernanceObject*> vpGovObjsTriggersTmp;
+    {
+        LOCK2(cs_main, cs);
 
-    for(object_m_it it = mapObjects.begin(); it != mapObjects.end(); ++it) {
-        if(mapAskedRecently.count(it->first)) {
-            std::map<CService, int64_t>::iterator it1 = mapAskedRecently[it->first].begin();
-            while(it1 != mapAskedRecently[it->first].end()) {
-                if(it1->second < nNow) {
-                    mapAskedRecently[it->first].erase(it1++);
-                } else {
-                    ++it1;
+        if(mapObjects.empty()) return -2;
+
+        for(object_m_it it = mapObjects.begin(); it != mapObjects.end(); ++it) {
+            if(mapAskedRecently.count(it->first)) {
+                std::map<CService, int64_t>::iterator it1 = mapAskedRecently[it->first].begin();
+                while(it1 != mapAskedRecently[it->first].end()) {
+                    if(it1->second < nNow) {
+                        mapAskedRecently[it->first].erase(it1++);
+                    } else {
+                        ++it1;
+                    }
                 }
+                if(mapAskedRecently[it->first].size() >= nPeersPerHashMax) continue;
+            }
+            if(it->second.nObjectType == GOVERNANCE_OBJECT_TRIGGER) {
+                vpGovObjsTriggersTmp.push_back(&(it->second));
+            } else {
+                vpGovObjsTmp.push_back(&(it->second));
             }
-            if(mapAskedRecently[it->first].size() >= nPeersPerHashMax) continue;
-        }
-        if(it->second.nObjectType == GOVERNANCE_OBJECT_TRIGGER) {
-            vpGovObjsTriggersTmp.push_back(&(it->second));
-        } else {
-            vpGovObjsTmp.push_back(&(it->second));
         }
     }
 
@@ -1301,24 +1304,28 @@ void CGovernanceManager::RequestOrphanObjects()
 {
     std::vector<CNode*> vNodesCopy = CopyNodeVector();
 
+    std::vector<uint256> vecHashesFiltered;
     {
-        LOCK(cs);
         std::vector<uint256> vecHashes;
+        LOCK(cs);
         mapOrphanVotes.GetKeys(vecHashes);
-
-        LogPrint("gobject", "CGovernanceObject::RequestOrphanObjects -- number objects = %d\n", vecHashes.size());
         for(size_t i = 0; i < vecHashes.size(); ++i) {
             const uint256& nHash = vecHashes[i];
-            if(mapObjects.find(nHash) != mapObjects.end()) {
+            if(mapObjects.find(nHash) == mapObjects.end()) {
+                vecHashesFiltered.push_back(nHash);
+            }
+        }
+    }
+
+    LogPrint("gobject", "CGovernanceObject::RequestOrphanObjects -- number objects = %d\n", vecHashesFiltered.size());
+    for(size_t i = 0; i < vecHashesFiltered.size(); ++i) {
+        const uint256& nHash = vecHashesFiltered[i];
+        for(size_t j = 0; j < vNodesCopy.size(); ++j) {
+            CNode* pnode = vNodesCopy[j];
+            if(pnode->fMasternode) {
                 continue;
             }
-            for(size_t j = 0; j < vNodesCopy.size(); ++j) {
-                CNode* pnode = vNodesCopy[j];
-                if(pnode->fMasternode) {
-                    continue;
-                }
-                RequestGovernanceObject(pnode, nHash);
-            }
+            RequestGovernanceObject(pnode, nHash);
         }
     }