Skip to content

Commit

Permalink
Merge pull request #2755 from jamescowens/fix_scraper_net
Browse files Browse the repository at this point in the history
scraper: Corrections to scraper_net after removal of cntPartsRcvd decrement and increment
  • Loading branch information
jamescowens authored Mar 31, 2024
2 parents e33e41d + 5817745 commit 9324bd1
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 9324bd1

Please sign in to comment.