Skip to content

Commit

Permalink
Reversion of the removal of cntPartsRcvd decrement and increment
Browse files Browse the repository at this point in the history
Also includes other fixes to address premature marking of manifests
complete that was being masked by decrementing an unsigned integer that
was already zero.
  • Loading branch information
jamescowens committed Mar 30, 2024
1 parent 1963bd2 commit 992b075
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 8 deletions.
7 changes: 4 additions & 3 deletions src/gridcoin/scraper/scraper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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++;

Expand Down Expand Up @@ -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++;
}
Expand Down Expand Up @@ -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++;
}
Expand All @@ -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)
Expand Down
12 changes: 8 additions & 4 deletions src/gridcoin/scraper/scraper_net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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));
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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());
{
Expand Down
7 changes: 6 additions & 1 deletion src/gridcoin/scraper/scraper_net.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -86,6 +86,11 @@ class CSplitBlob

std::vector<CPart*> 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. */
Expand Down

0 comments on commit 992b075

Please sign in to comment.