mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-11-12 23:18:14 +01:00
Merge #16033: Hold cs_main when reading chainActive via getTipLocator(). Remove assumeLocked().
9402ef0739Remove temporary method assumeLocked(). Remove LockingStateImpl. Remove redundant cs_main locks. (practicalswift)593a8e8a2cwallet: Use chain.lock() instead of temporary chain.assumeLocked() (practicalswift) Pull request description: Fixes #16028. Problem description: `LockAnnotation lock(::cs_main)` is a guarantee to the compiler thread analysis that `::cs_main` is locked (when it couldn't be determined otherwise). Despite being annotated with the locking guarantee ...65526fc866/src/interfaces/chain.cpp (L134-L138)... `getTipLocator()` reads `chainActive` (via `::ChainActive()`) without holding `cs_main`. This can be verified by adding the following `AssertLockHeld(cs_main)`: ``` $ git diff diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 59623284d..9fc693a0f 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -134,6 +134,7 @@ class LockImpl : public Chain::Lock CBlockLocator getTipLocator() override { LockAnnotation lock(::cs_main); + AssertLockHeld(::cs_main); return ::ChainActive().GetLocator(); } Optional<int> findLocatorFork(const CBlockLocator& locator) override $ make check ../build-aux/test-driver: line 107: 12881 Aborted "$@" > $log_file 2>&1 FAIL: qt/test/test_bitcoin-qt ``` ACKs for commit 9402ef: MarcoFalke: utACK9402ef0739ryanofsky: utACK9402ef0739. Changes are consolidating commits and removing redundant lock2 cs_main calls Tree-SHA512: 0a030bf0c07eb53194ecc246f973ef389dd42a0979f51932bf94bdf7e90c52473ae03be49718ee1629582b05dd8e0dc020b5a210318c93378ea4ace90c0f9f72
This commit is contained in:
@@ -138,11 +138,6 @@ public:
|
||||
//! unlocked when the returned interface is freed.
|
||||
virtual std::unique_ptr<Lock> lock(bool try_lock = false) = 0;
|
||||
|
||||
//! Return Lock interface assuming chain is already locked. This
|
||||
//! method is temporary and is only used in a few places to avoid changing
|
||||
//! behavior while code is transitioned to use the Chain::Lock interface.
|
||||
virtual std::unique_ptr<Lock> assumeLocked() = 0;
|
||||
|
||||
//! Return whether node has the block and optionally return block metadata
|
||||
//! or contents.
|
||||
//!
|
||||
|
||||
Reference in New Issue
Block a user