Merge bitcoin#24197: Replace lock with thread safety annotation in CBlockTreeDB::LoadBlockIndexGuts()

20276ca Replace lock with thread safety annotation in CBlockTreeDB::LoadBlockIndexGuts() (Jon Atack)

Pull request description:

  Following up on bitcoin#22932 (comment) by Marco Falke (good observation, thank you), we can replace a cs_main lock in `CBlockTreeDB::LoadBlockIndexGuts()` with a Clang thread safety annotation/assertion instead. The unlocked code is reverted to its original state before bitcoin#22932.

ACKs for top commit:
  hebasto:
    ACK 20276ca, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 2d91d1c962af0286d835d92a56396a27ea00e9d061b869eaff9421b448a083b0513828e1d4df7504396896057bf1e344e180a50271a5cfd1e1c7b6527155b2bb

Author: MarcoFalke

src/txdb.cpp

@@ -296,8 +296,8 @@ bool CBlockTreeDB::ReadFlag(const std::string &name, bool &fValue) {
 
 bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex)
 {
+    AssertLockHeld(::cs_main);
     std::unique_ptr<CDBIterator> pcursor(NewIterator());
-
     pcursor->Seek(std::make_pair(DB_BLOCK_INDEX, uint256()));
 
     // Load m_block_index
@@ -311,19 +311,16 @@ bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams,
                 CBlockIndex* pindexNew = insertBlockIndex(diskindex.GetBlockHash());
                 pindexNew->pprev          = insertBlockIndex(diskindex.hashPrev);
                 pindexNew->nHeight        = diskindex.nHeight;
+                pindexNew->nFile          = diskindex.nFile;
+                pindexNew->nDataPos       = diskindex.nDataPos;
+                pindexNew->nUndoPos       = diskindex.nUndoPos;
                 pindexNew->nVersion       = diskindex.nVersion;
                 pindexNew->hashMerkleRoot = diskindex.hashMerkleRoot;
                 pindexNew->nTime          = diskindex.nTime;
                 pindexNew->nBits          = diskindex.nBits;
                 pindexNew->nNonce         = diskindex.nNonce;
+                pindexNew->nStatus        = diskindex.nStatus;
                 pindexNew->nTx            = diskindex.nTx;
-                {
-                    LOCK(::cs_main);
-                    pindexNew->nFile = diskindex.nFile;
-                    pindexNew->nDataPos = diskindex.nDataPos;
-                    pindexNew->nUndoPos = diskindex.nUndoPos;
-                    pindexNew->nStatus = diskindex.nStatus;
-                }
 
                 if (!CheckProofOfWork(pindexNew->GetBlockHash(), pindexNew->nBits, consensusParams)) {
                     return error("%s: CheckProofOfWork failed: %s", __func__, pindexNew->ToString());

src/txdb.h

@@ -86,7 +86,8 @@ class CBlockTreeDB : public CDBWrapper
     void ReadReindexing(bool &fReindexing);
     bool WriteFlag(const std::string &name, bool fValue);
     bool ReadFlag(const std::string &name, bool &fValue);
-    bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex);
+    bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex)
+        EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 };
 
 std::optional<bilingual_str> CheckLegacyTxindex(CBlockTreeDB& block_tree_db);