From 8bbf2c7435f6cc02933da348ccdccc797d5ba782 Mon Sep 17 00:00:00 2001 From: Pasta Date: Sat, 24 Apr 2021 01:29:59 -0400 Subject: [PATCH] refactor: add better locking in mn sync --- src/masternode/masternode-sync.cpp | 43 +++++++++++++++++++++--------- src/masternode/masternode-sync.h | 24 ++++++++--------- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/masternode/masternode-sync.cpp b/src/masternode/masternode-sync.cpp index 827efc9c30..3385dc9184 100644 --- a/src/masternode/masternode-sync.cpp +++ b/src/masternode/masternode-sync.cpp @@ -18,12 +18,15 @@ void CMasternodeSync::Reset(bool fForce, bool fNotifyReset) { // Avoid resetting the sync process if we just "recently" received a new block if (fForce || (GetTime() - nTimeLastUpdateBlockTip > MASTERNODE_SYNC_RESET_SECONDS)) { - nCurrentAsset = MASTERNODE_SYNC_BLOCKCHAIN; - nTriedPeerCount = 0; - nTimeAssetSyncStarted = GetTime(); - nTimeLastBumped = GetTime(); - nTimeLastUpdateBlockTip = 0; - fReachedBestHeader = false; + { + LOCK(cs); + nCurrentAsset = MASTERNODE_SYNC_BLOCKCHAIN; + nTriedPeerCount = 0; + nTimeAssetSyncStarted = GetTime(); + nTimeLastBumped = GetTime(); + nTimeLastUpdateBlockTip = 0; + fReachedBestHeader = false; + } if (fNotifyReset) { uiInterface.NotifyAdditionalDataSyncProgressChanged(-1); } @@ -33,12 +36,14 @@ void CMasternodeSync::Reset(bool fForce, bool fNotifyReset) void CMasternodeSync::BumpAssetLastTime(const std::string& strFuncName) { if (IsSynced()) return; + LOCK(cs); nTimeLastBumped = GetTime(); LogPrint(BCLog::MNSYNC, "CMasternodeSync::BumpAssetLastTime -- %s\n", strFuncName); } std::string CMasternodeSync::GetAssetName() const { + LOCK(cs); switch(nCurrentAsset) { case(MASTERNODE_SYNC_BLOCKCHAIN): return "MASTERNODE_SYNC_BLOCKCHAIN"; @@ -50,6 +55,7 @@ std::string CMasternodeSync::GetAssetName() const void CMasternodeSync::SwitchToNextAsset(CConnman& connman) { + LOCK(cs); switch(nCurrentAsset) { case(MASTERNODE_SYNC_BLOCKCHAIN): @@ -300,6 +306,7 @@ void CMasternodeSync::SendGovernanceSyncRequest(CNode* pnode, CConnman& connman) void CMasternodeSync::AcceptedBlockHeader(const CBlockIndex *pindexNew) { LogPrint(BCLog::MNSYNC, "CMasternodeSync::AcceptedBlockHeader -- pindexNew->nHeight: %d\n", pindexNew->nHeight); + LOCK(cs); if (!IsBlockchainSynced()) { // Postpone timeout each time new block header arrives while we are still syncing blockchain @@ -324,10 +331,18 @@ void CMasternodeSync::UpdatedBlockTip(const CBlockIndex *pindexNew, bool fInitia { LogPrint(BCLog::MNSYNC, "CMasternodeSync::UpdatedBlockTip -- pindexNew->nHeight: %d fInitialDownload=%d\n", pindexNew->nHeight, fInitialDownload); - LOCK(cs); - nTimeLastUpdateBlockTip = GetAdjustedTime(); + { + LOCK(cs); + nTimeLastUpdateBlockTip = GetAdjustedTime(); + } - if (IsSynced() || !pindexBestHeader) + CBlockIndex* pindexTip; + { + LOCK(cs_main); + pindexTip = pindexBestHeader; + } + + if (IsSynced() || !pindexTip) return; if (!IsBlockchainSynced()) { @@ -346,7 +361,7 @@ void CMasternodeSync::UpdatedBlockTip(const CBlockIndex *pindexNew, bool fInitia } // Note: since we sync headers first, it should be ok to use this - bool fReachedBestHeaderNew = pindexNew->GetBlockHash() == pindexBestHeader->GetBlockHash(); + bool fReachedBestHeaderNew = pindexNew->GetBlockHash() == pindexTip->GetBlockHash(); if (fReachedBestHeader && !fReachedBestHeaderNew) { // Switching from true to false means that we previously stuck syncing headers for some reason, @@ -355,10 +370,12 @@ void CMasternodeSync::UpdatedBlockTip(const CBlockIndex *pindexNew, bool fInitia Reset(true); } - fReachedBestHeader = fReachedBestHeaderNew; - + { + LOCK(cs); + fReachedBestHeader = fReachedBestHeaderNew; + } LogPrint(BCLog::MNSYNC, "CMasternodeSync::UpdatedBlockTip -- pindexNew->nHeight: %d pindexBestHeader->nHeight: %d fInitialDownload=%d fReachedBestHeader=%d\n", - pindexNew->nHeight, pindexBestHeader->nHeight, fInitialDownload, fReachedBestHeader); + pindexNew->nHeight, pindexTip->nHeight, fInitialDownload, fReachedBestHeader); } void CMasternodeSync::DoMaintenance(CConnman &connman) diff --git a/src/masternode/masternode-sync.h b/src/masternode/masternode-sync.h index d800f0c948..9a6deb2377 100644 --- a/src/masternode/masternode-sync.h +++ b/src/masternode/masternode-sync.h @@ -31,35 +31,35 @@ extern CMasternodeSync masternodeSync; class CMasternodeSync { private: - CCriticalSection cs; + mutable CCriticalSection cs; // Keep track of current asset - int nCurrentAsset; + int nCurrentAsset GUARDED_BY(cs); // Count peers we've requested the asset from - int nTriedPeerCount; + int nTriedPeerCount GUARDED_BY(cs); // Time when current masternode asset sync started - int64_t nTimeAssetSyncStarted; + int64_t nTimeAssetSyncStarted GUARDED_BY(cs); // ... last bumped - int64_t nTimeLastBumped; + int64_t nTimeLastBumped GUARDED_BY(cs); /// Set to true if best header is reached in CMasternodeSync::UpdatedBlockTip - bool fReachedBestHeader{false}; + bool fReachedBestHeader GUARDED_BY(cs) {false}; /// Last time UpdateBlockTip has been called - int64_t nTimeLastUpdateBlockTip{0}; + int64_t nTimeLastUpdateBlockTip GUARDED_BY(cs) {0}; public: CMasternodeSync() { Reset(true, false); } static void SendGovernanceSyncRequest(CNode* pnode, CConnman& connman); - bool IsBlockchainSynced() const { return nCurrentAsset > MASTERNODE_SYNC_BLOCKCHAIN; } - bool IsSynced() const { return nCurrentAsset == MASTERNODE_SYNC_FINISHED; } + bool IsBlockchainSynced() const { LOCK(cs); return nCurrentAsset > MASTERNODE_SYNC_BLOCKCHAIN; } + bool IsSynced() const { LOCK(cs); return nCurrentAsset == MASTERNODE_SYNC_FINISHED; } - int GetAssetID() const { return nCurrentAsset; } - int GetAttempt() const { return nTriedPeerCount; } + int GetAssetID() const { LOCK(cs); return nCurrentAsset; } + int GetAttempt() const { LOCK(cs); return nTriedPeerCount; } void BumpAssetLastTime(const std::string& strFuncName); - int64_t GetAssetStartTime() const { return nTimeAssetSyncStarted; } + int64_t GetAssetStartTime() const { LOCK(cs); return nTimeAssetSyncStarted; } std::string GetAssetName() const; std::string GetSyncStatus() const;