Bitcoin 0.16 locking PRs #5017
Merged
Bitcoin 0.16 locking PRs #5017
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LarryRuane
added
C-upstream-port
|
☔ The latest upstream changes (presumably #5009) made this pull request unmergeable. Please resolve the merge conflicts. |
CWallet::MarkConflicted may acquire the cs_main lock after CWalletDB::LoadWallet acquires the cs_wallet lock during wallet initialization. (CWalletDB::LoadWallet calls ReadKeyValue which calls CWallet::LoadToWallet which calls CWallet::MarkConflicted). This is the opposite order that cs_main and cs_wallet locks are acquired in the rest of the code, and so leads to POTENTIAL DEADLOCK DETECTED errors if bitcoin is built with -DDEBUG_LOCKORDER. This commit changes CWallet::LoadWallet (which calls CWalletDB::LoadWallet) to acquire both locks in the standard order. It also fixes some tests that were acquiring wallet and main locks out of order and failed with the new locking in CWallet::LoadWallet. Error was reported by Luke Dashjr <luke-jr@utopios.org> in https://botbot.me/freenode/bitcoin-core-dev/msg/90244330/ zcash: cherry picked from commit de9a1db zcash: bitcoin/bitcoin#11126
Issue: #10905 By returning the result, a few useless lines can be removed. Return-value-optimization means there should be no copy. zcash: cherry picked from commit 5cb3da0 zcash: bitcoin/bitcoin#10916
zcash: cherry picked from commit c626dcb zcash: bitcoin/bitcoin#11107
A rare race condition may trigger while awaiting the body of a message, see upsteam commit 5ff8eb26371c4dc56f384b2de35bea2d87814779 for details. This may fix some reported rpc hangs/crashes. zcash: currently, we build with libevent-2.1.12, so this fix is needed zcash: cherry picked from commit 6b58360 zcash: bitcoin/bitcoin#11593
The bug was introduced in 2.1.6-beta, versions before that don't need the workaround. zcash: currently, we build with libevent-2.1.12, so this fix is needed zcash: cherry picked from commit 97932cd zcash: bitcoin/bitcoin#11593
The variable vRandom is guarded by the mutex cs. zcash: cherry picked from commit 3ab545d zcash: bitcoin/bitcoin#11585
zcash: cherry picked from commit a9b6ba0 zcash: bitcoin/bitcoin#11618
zcash: cherry picked from commit 2b4b345 zcash: bitcoin/bitcoin#10286
zcash: cherry picked from commit 89f0312 zcash: bitcoin/bitcoin#10286
zcash: cherry picked from commit 5b25293 zcash: bitcoin/bitcoin#11870
zcash: cherry picked from commit c409b1a zcash: bitcoin/bitcoin#12330
This prevents a potential race condition if control flow ends up in `ShutdownHTTPServer` before the thread gets to `queue->Run()`, deleting the work queue while workers are still going to use it. Meant to fix #12362. Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com> zcash: cherry picked from commit b1c2370 zcash: bitcoin/bitcoin#12366
This function, which waits for all threads to exit, is no longer needed now that threads are joined instead. Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com> zcash: cherry picked from commit f946654 zcash: bitcoin/bitcoin#12366
The HTTP worker thread counter, as well as the RAII object that was used to maintain it, is unused now, so can be removed. Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com> zcash: cherry picked from commit 11e0151 zcash: bitcoin/bitcoin#12366
This resolves an issue where getrawmempool() can race mempool notification signals. Intuitively we use mempool.cs as a "read lock" on the mempool with cs_main being the write lock, so holding the read lock intermittently while doing write operations is somewhat strange. This also avoids the introduction of cs_main in getrawmempool() which reviewers objected to in the previous fix in #12273 zcash: cherry picked from commit 85aa839 zcash: bitcoin/bitcoin#12368
zcash: cherry picked from commit 02fc886 zcash: bitcoin/bitcoin#12368
zcash: Not an actual locking fix; just eliminates unnecessary recursive zcash: locking (which is a long-term goal). zcash: cherry picked from commit 1beea7a zcash: bitcoin/bitcoin#12333
LarryRuane
force-pushed
the
upstream-locking-0.16
branch
from
f8ea721
to
5b2f7a6
Compare
therealyingtong
approved these changes
Apr 14, 2021
str4d
approved these changes
Apr 17, 2021
|
@zkbot r+ |
|
📌 Commit 5b2f7a6 has been approved by |
Merged
str4d
added a commit
to str4d/zcash
that referenced
this pull request
Apr 4, 2022
The comment on `view.SetBackend(dummy)` was removed when we backported upstream locking PRs in zcash#5017. The upstream commit in question removed a locking scope but did not remove the reference to that scope in the comment. Our backport removed the outdated comment, but should have modified it instead, because otherwise the existence of `view.SetBackend(dummy)` is very confusing (as it disconnects the cache from `pcoinsTip`, on the assumption that everything we need from it has been cached via calls to `CCoinsViewCache::HaveCoins` and `CCoinsViewCache::HaveShieldedRequirements`).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These are locking changes from upstream (bitcoin core) release 0.16 (Aug 14, 2017, to Feb 16, 2018), oldest to newest (when merged to the master branch).
Each commit also includes a reference both to the PR and the upstream commit.