Skip to content

Commit

Permalink
Merge #5990: refactor: minimize locking in ChainLocks Cleanup
Browse files Browse the repository at this point in the history
04ba164 refactor: immediately update lastCleanupTime (pasta)
d0d2641 refactor: minimize locking of cs_main in chainlocks.cpp (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Simple changes, just look at the two commits: first we minimize scope of cs_main to what actually needs it. Then we change where we update the `lastCleanupTime` to right after we check it to minimize any chance of two threads entering at the same time.

  ## What was done?

  ## How Has This Been Tested?
  Built, ran for a bit

  ## Breaking Changes
  None

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

Top commit has no ACKs.

Tree-SHA512: 63432ccc16a67e6478e578c96fe809cf33d08e5068285c21e65ccc175c0c9e82c23519d57f0c4ebac162a094bfc20eb44df30cbe85b26815d02eaac06ceb66ad
  • Loading branch information
PastaPastaPasta committed May 3, 2024
2 parents 765ad20 + 04ba164 commit d44b0d5
Showing 1 changed file with 11 additions and 10 deletions.
21 changes: 11 additions & 10 deletions src/llmq/chainlocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -622,19 +622,22 @@ void CChainLocksHandler::Cleanup()
if (GetTimeMillis() - lastCleanupTime < CLEANUP_INTERVAL) {
return;
}
lastCleanupTime = GetTimeMillis();

{
LOCK(cs);
for (auto it = seenChainLocks.begin(); it != seenChainLocks.end(); ) {
if (GetTimeMillis() - it->second >= CLEANUP_SEEN_TIMEOUT) {
it = seenChainLocks.erase(it);
} else {
++it;
}
}
}
// need mempool.cs due to GetTransaction calls
LOCK2(cs_main, mempool.cs);
LOCK(cs);

for (auto it = seenChainLocks.begin(); it != seenChainLocks.end(); ) {
if (GetTimeMillis() - it->second >= CLEANUP_SEEN_TIMEOUT) {
it = seenChainLocks.erase(it);
} else {
++it;
}
}

for (auto it = blockTxs.begin(); it != blockTxs.end(); ) {
const auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(it->first);
if (InternalHasChainLock(pindex->nHeight, pindex->GetBlockHash())) {
Expand Down Expand Up @@ -666,8 +669,6 @@ void CChainLocksHandler::Cleanup()
++it;
}
}

lastCleanupTime = GetTimeMillis();
}

bool AreChainLocksEnabled(const CSporkManager& sporkman)
Expand Down

0 comments on commit d44b0d5

Please sign in to comment.