From f9b22b61e68c4f17be265344a0597be0cf58a3fc Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Thu, 16 Sep 2021 16:53:47 +0200 Subject: [PATCH] Merge bitcoin/bitcoin#22895: consensus: don't call GetBlockPos in ReadBlockFromDisk without cs_main lock 350e034e64d175f3db4c85ddca42e76e279912f6 consensus: don't call GetBlockPos in ReadBlockFromDisk without lock (Jon Atack) Pull request description: Commit ccd8ef65 "Reduce cs_main lock in ReadBlockFromDisk, only read GetBlockPos under the lock" in #11281 moved the cs_main lock from caller to `ReadBlockFromDisk()` for calling `CBlockIndex::GetBlockPos()`, but the second invocation doesn't have the lock, and IIUC there is no guarantee the compiler can know if state has changed. Use the `blockPos` local variable instead, rename it to `block_pos`, and make it const. ACKs for top commit: laanwj: Code review ACK 350e034e64d175f3db4c85ddca42e76e279912f6 theStack: Code-review ACK 350e034e64d175f3db4c85ddca42e76e279912f6 promag: Code review ACK 350e034e64d175f3db4c85ddca42e76e279912f6. Tree-SHA512: 0df0614ab1876885c85f7b53c604a759a29008da8027e95503b4726d2b820ec6d27546020c613337ff954406e01cb5d191978ba4a12124052fed6e1b0e9a226f --- src/node/blockstorage.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 9d92daf199..b8c6f193f1 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -72,18 +72,14 @@ bool ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos, const Consensus::P bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams) { - FlatFilePos blockPos; - { - LOCK(cs_main); - blockPos = pindex->GetBlockPos(); - } + const FlatFilePos block_pos{WITH_LOCK(cs_main, return pindex->GetBlockPos())}; - if (!ReadBlockFromDisk(block, blockPos, consensusParams)) { + if (!ReadBlockFromDisk(block, block_pos, consensusParams)) { return false; } if (block.GetHash() != pindex->GetBlockHash()) { return error("ReadBlockFromDisk(CBlock&, CBlockIndex*): GetHash() doesn't match index for %s at %s", - pindex->ToString(), pindex->GetBlockPos().ToString()); + pindex->ToString(), block_pos.ToString()); } return true; }