Merge #5990: refactor: minimize locking in ChainLocks Cleanup

04ba164064 refactor: immediately update lastCleanupTime (pasta)
d0d2641154 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
This commit is contained in:
pasta 2024-05-03 10:15:53 -05:00
commit d44b0d5dcb
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984

View File

@ -622,11 +622,10 @@ void CChainLocksHandler::Cleanup()
if (GetTimeMillis() - lastCleanupTime < CLEANUP_INTERVAL) { if (GetTimeMillis() - lastCleanupTime < CLEANUP_INTERVAL) {
return; return;
} }
lastCleanupTime = GetTimeMillis();
// need mempool.cs due to GetTransaction calls {
LOCK2(cs_main, mempool.cs);
LOCK(cs); LOCK(cs);
for (auto it = seenChainLocks.begin(); it != seenChainLocks.end(); ) { for (auto it = seenChainLocks.begin(); it != seenChainLocks.end(); ) {
if (GetTimeMillis() - it->second >= CLEANUP_SEEN_TIMEOUT) { if (GetTimeMillis() - it->second >= CLEANUP_SEEN_TIMEOUT) {
it = seenChainLocks.erase(it); it = seenChainLocks.erase(it);
@ -634,6 +633,10 @@ void CChainLocksHandler::Cleanup()
++it; ++it;
} }
} }
}
// need mempool.cs due to GetTransaction calls
LOCK2(cs_main, mempool.cs);
LOCK(cs);
for (auto it = blockTxs.begin(); it != blockTxs.end(); ) { for (auto it = blockTxs.begin(); it != blockTxs.end(); ) {
const auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(it->first); const auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(it->first);
@ -666,8 +669,6 @@ void CChainLocksHandler::Cleanup()
++it; ++it;
} }
} }
lastCleanupTime = GetTimeMillis();
} }
bool AreChainLocksEnabled(const CSporkManager& sporkman) bool AreChainLocksEnabled(const CSporkManager& sporkman)