diff options
author | MarcoFalke <falke.marco@gmail.com> | 2019-05-17 07:16:41 -0400 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2019-05-17 07:17:41 -0400 |
commit | f3d27d126bbf50e0ba63f295bfa44e4554d48343 (patch) | |
tree | db264bc3dd21938a72fbd31ed01860932a169c97 /src/wallet/wallet.cpp | |
parent | 8f2f17f79a10ab3db13f95291d302f5eae3e3a91 (diff) | |
parent | 9402ef0739fdcd8e989c07c0595095e9608b243c (diff) |
Merge #16033: Hold cs_main when reading chainActive via getTipLocator(). Remove assumeLocked().
9402ef0739 Remove temporary method assumeLocked(). Remove LockingStateImpl. Remove redundant cs_main locks. (practicalswift)
593a8e8a2c wallet: 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 ...
https://github.com/bitcoin/bitcoin/blob/65526fc8666fef35ef908dbc225f706bef642c7e/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:
utACK 9402ef0739fdcd8e989c07c0595095e9608b243c
ryanofsky:
utACK 9402ef0739fdcd8e989c07c0595095e9608b243c. Changes are consolidating commits and removing redundant lock2 cs_main calls
Tree-SHA512: 0a030bf0c07eb53194ecc246f973ef389dd42a0979f51932bf94bdf7e90c52473ae03be49718ee1629582b05dd8e0dc020b5a210318c93378ea4ace90c0f9f72
Diffstat (limited to 'src/wallet/wallet.cpp')
-rw-r--r-- | src/wallet/wallet.cpp | 2 |
1 files changed, 1 insertions, 1 deletions
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7038eff4bb..260617c666 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4074,7 +4074,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, return nullptr; } - auto locked_chain = chain.assumeLocked(); // Temporary. Removed in upcoming lock cleanup + auto locked_chain = chain.lock(); walletInstance->ChainStateFlushed(locked_chain->getTipLocator()); } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) { // Make it impossible to disable private keys after creation |