From 992b075d9e3976437089f5ea9e10d3f8ee2c8560 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Fri, 29 Mar 2024 10:28:24 -0400 Subject: [PATCH] Reversion of the removal of cntPartsRcvd decrement and increment Also includes other fixes to address premature marking of manifests complete that was being masked by decrementing an unsigned integer that was already zero. --- src/gridcoin/scraper/scraper.cpp | 7 ++++--- src/gridcoin/scraper/scraper_net.cpp | 12 ++++++++---- src/gridcoin/scraper/scraper_net.h | 7 ++++++- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/gridcoin/scraper/scraper.cpp b/src/gridcoin/scraper/scraper.cpp index 9a65484b78..d654f88d4a 100755 --- a/src/gridcoin/scraper/scraper.cpp +++ b/src/gridcoin/scraper/scraper.cpp @@ -4479,7 +4479,7 @@ EXCLUSIVE_LOCKS_REQUIRED(cs_StructScraperFileManifest, CScraperManifest::cs_mapM CDataStream part(std::move(vchData), SER_NETWORK, 1); - manifest->addPartData(std::move(part)); + manifest->addPartData(std::move(part), true); iPartNum++; @@ -4512,7 +4512,7 @@ EXCLUSIVE_LOCKS_REQUIRED(cs_StructScraperFileManifest, CScraperManifest::cs_mapM part << ScraperVerifiedBeacons.mVerifiedMap; - manifest->addPartData(std::move(part)); + manifest->addPartData(std::move(part), true); iPartNum++; } @@ -4592,7 +4592,7 @@ EXCLUSIVE_LOCKS_REQUIRED(cs_StructScraperFileManifest, CScraperManifest::cs_mapM CDataStream part(vchData, SER_NETWORK, 1); - manifest->addPartData(std::move(part)); + manifest->addPartData(std::move(part), true); iPartNum++; } @@ -4602,6 +4602,7 @@ EXCLUSIVE_LOCKS_REQUIRED(cs_StructScraperFileManifest, CScraperManifest::cs_mapM LOCK(CSplitBlob::cs_mapParts); + // addManifest will also call Complete() after signing to send the manifest over the network. bool bAddManifestSuccessful = CScraperManifest::addManifest(manifest, Key); if (bAddManifestSuccessful) diff --git a/src/gridcoin/scraper/scraper_net.cpp b/src/gridcoin/scraper/scraper_net.cpp index e00fb4d9e4..ccb46dc9f2 100644 --- a/src/gridcoin/scraper/scraper_net.cpp +++ b/src/gridcoin/scraper/scraper_net.cpp @@ -97,7 +97,7 @@ bool CSplitBlob::RecvPart(CNode* pfrom, CDataStream& vRecv) bool CSplitBlob::isComplete() const EXCLUSIVE_LOCKS_REQUIRED(CSplitBlob::cs_manifest) { - return cntPartsRcvd == vParts.size(); + return (!m_publish_in_progress && cntPartsRcvd == vParts.size()); } void CSplitBlob::addPart(const uint256& ihash) @@ -119,10 +119,12 @@ void CSplitBlob::addPart(const uint256& ihash) part.refs.emplace(this, n); } -int CSplitBlob::addPartData(CDataStream&& vData) +int CSplitBlob::addPartData(CDataStream&& vData, const bool& publish_in_progress) { LOCK2(cs_mapParts, cs_manifest); + m_publish_in_progress = publish_in_progress; + uint256 hash(Hash(vData)); auto it = mapParts.emplace(hash, CPart(hash)); @@ -138,9 +140,9 @@ int CSplitBlob::addPartData(CDataStream&& vData) if (!part.present()) { /* missing data; use the supplied data */ - /* prevent calling the Complete callback FIXME: make this look better */ CSplitBlob::RecvPart(nullptr, vData); } + return n; } @@ -800,7 +802,7 @@ EXCLUSIVE_LOCKS_REQUIRED(CScraperManifest::cs_mapManifest, cs_mapParts) if (it.second == false) return false; - // Release lock on cs_manifest before taking a lonk on cs_ConvergedScraperStatsCache to avoid potential deadlocks. + // Release lock on cs_manifest before taking a lock on cs_ConvergedScraperStatsCache to avoid potential deadlocks. { CScraperManifest& manifest = *it.first->second; @@ -834,6 +836,8 @@ EXCLUSIVE_LOCKS_REQUIRED(CScraperManifest::cs_mapManifest, cs_mapParts) void CScraperManifest::Complete() EXCLUSIVE_LOCKS_REQUIRED(CSplitBlob::cs_manifest, CSplitBlob::cs_mapParts) { + m_publish_in_progress = false; + // Notify peers that we have a new manifest LogPrint(BCLog::LogFlags::MANIFEST, "manifest %s complete with %u parts", phash->GetHex(), (unsigned)vParts.size()); { diff --git a/src/gridcoin/scraper/scraper_net.h b/src/gridcoin/scraper/scraper_net.h index 3959a6e889..ed2b1681a9 100755 --- a/src/gridcoin/scraper/scraper_net.h +++ b/src/gridcoin/scraper/scraper_net.h @@ -65,7 +65,7 @@ class CSplitBlob void addPart(const uint256& ihash); /** Create a part from specified data and add reference to it into vParts. */ - int addPartData(CDataStream&& vData); + int addPartData(CDataStream&& vData, const bool &publish_in_progress = false); /** Unref all parts referenced by this. Removes parts with no references */ virtual ~CSplitBlob(); @@ -86,6 +86,11 @@ class CSplitBlob std::vector vParts GUARDED_BY(cs_manifest); size_t cntPartsRcvd GUARDED_BY(cs_manifest) = 0; + + //! + //! \brief Used by the scraper when building manifests part by part to prevent triggering Complete() prematurely. + //! + bool m_publish_in_progress GUARDED_BY(cs_manifest) = false; }; /** An objects holding info about the scraper data file we have or are downloading. */