Don't hold CDeterministicMNManager::cs while calling signals (#2608)

This is causing potential deadlocks due to governance calling back into
CDeterministicMNManager but with a differnent lock order.
This commit is contained in:
Alexander Block 2019-01-03 13:24:28 +01:00 committed by UdjinM6
parent 968eb3fc5d
commit cdc8ae9436

View File

@ -448,31 +448,36 @@ CDeterministicMNManager::CDeterministicMNManager(CEvoDB& _evoDb) :
bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockIndex* pindex, CValidationState& _state) bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockIndex* pindex, CValidationState& _state)
{ {
LOCK(cs); CDeterministicMNList oldList, newList;
CDeterministicMNListDiff diff;
int nHeight = pindex->nHeight; int nHeight = pindex->nHeight;
CDeterministicMNList newList; {
if (!BuildNewListFromBlock(block, pindex->pprev, _state, newList, true)) { LOCK(cs);
return false;
} if (!BuildNewListFromBlock(block, pindex->pprev, _state, newList, true)) {
return false;
if (newList.GetHeight() == -1) { }
newList.SetHeight(nHeight);
} if (newList.GetHeight() == -1) {
newList.SetHeight(nHeight);
newList.SetBlockHash(block.GetHash()); }
CDeterministicMNList oldList = GetListForBlock(pindex->pprev->GetBlockHash()); newList.SetBlockHash(block.GetHash());
CDeterministicMNListDiff diff = oldList.BuildDiff(newList);
oldList = GetListForBlock(pindex->pprev->GetBlockHash());
evoDb.Write(std::make_pair(DB_LIST_DIFF, diff.blockHash), diff); diff = oldList.BuildDiff(newList);
if ((nHeight % SNAPSHOT_LIST_PERIOD) == 0) {
evoDb.Write(std::make_pair(DB_LIST_SNAPSHOT, diff.blockHash), newList); evoDb.Write(std::make_pair(DB_LIST_DIFF, diff.blockHash), diff);
LogPrintf("CDeterministicMNManager::%s -- Wrote snapshot. nHeight=%d, mapCurMNs.allMNsCount=%d\n", if ((nHeight % SNAPSHOT_LIST_PERIOD) == 0) {
__func__, nHeight, newList.GetAllMNsCount()); evoDb.Write(std::make_pair(DB_LIST_SNAPSHOT, diff.blockHash), newList);
LogPrintf("CDeterministicMNManager::%s -- Wrote snapshot. nHeight=%d, mapCurMNs.allMNsCount=%d\n",
__func__, nHeight, newList.GetAllMNsCount());
}
} }
// Don't hold cs while calling signals
if (!diff.addedMNs.empty() || !diff.removedMns.empty()) { if (!diff.addedMNs.empty() || !diff.removedMns.empty()) {
GetMainSignals().NotifyMasternodeListChanged(newList); GetMainSignals().NotifyMasternodeListChanged(newList);
} }
@ -487,6 +492,7 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
LogPrintf("CDeterministicMNManager::%s -- DIP3 is active now. nHeight=%d\n", __func__, nHeight); LogPrintf("CDeterministicMNManager::%s -- DIP3 is active now. nHeight=%d\n", __func__, nHeight);
} }
LOCK(cs);
CleanupCache(nHeight); CleanupCache(nHeight);
return true; return true;
@ -494,17 +500,19 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
bool CDeterministicMNManager::UndoBlock(const CBlock& block, const CBlockIndex* pindex) bool CDeterministicMNManager::UndoBlock(const CBlock& block, const CBlockIndex* pindex)
{ {
LOCK(cs);
int nHeight = pindex->nHeight; int nHeight = pindex->nHeight;
uint256 blockHash = block.GetHash(); uint256 blockHash = block.GetHash();
CDeterministicMNListDiff diff; CDeterministicMNListDiff diff;
evoDb.Read(std::make_pair(DB_LIST_DIFF, blockHash), diff); {
LOCK(cs);
evoDb.Read(std::make_pair(DB_LIST_DIFF, blockHash), diff);
evoDb.Erase(std::make_pair(DB_LIST_DIFF, blockHash)); evoDb.Erase(std::make_pair(DB_LIST_DIFF, blockHash));
evoDb.Erase(std::make_pair(DB_LIST_SNAPSHOT, blockHash)); evoDb.Erase(std::make_pair(DB_LIST_SNAPSHOT, blockHash));
mnListsCache.erase(blockHash);
mnListsCache.erase(blockHash);
}
if (!diff.addedMNs.empty() || !diff.removedMns.empty()) { if (!diff.addedMNs.empty() || !diff.removedMns.empty()) {
auto prevList = GetListForBlock(pindex->pprev->GetBlockHash()); auto prevList = GetListForBlock(pindex->pprev->GetBlockHash());