Bitcoin 0.14 locking PRs #5009
Merged
Bitcoin 0.14 locking PRs #5009
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
| @@ -328,8 +335,7 @@ const fs::path GetExportDir() | |||
|
|
|||
| const fs::path &GetDataDir(bool fNetSpecific) | |||
| { | |||
|
|
|||
| LOCK(csPathCached); | |||
| LOCK2(cs_args, csPathCached); | |||
There was a problem hiding this comment.
Upstream seems to not have included the lock here; but it seems safe to.
There was a problem hiding this comment.
Yes, I think it's safe, but also necessary, because this function accesses mapArgs which is protected by cs_args. The upstream commit didn't change this function because it had already been updated to call gArgs.GetArg() (instead of accessing mapArgs directly), and gArgs.GetArg() calls GetSetting(), which acquires cs_args. We probably should pick up these argument-handling changes someday, but that's a big project.
There was a problem hiding this comment.
Yep, I've wanted to get these in for years (and have slowly been unblocking them).
zcash: cherry picked from commit 33d15a3 zcash: bitcoin/bitcoin#8472
This makes sure that cs_filter is never held while taking cs_main or CNode::cs_vSend. zcash: cherry picked from commit 144ed76 zcash: bitcoin/bitcoin#8606
See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78552 zcash: cherry picked from commit 507145d zcash: bitcoin/bitcoin#9230
zcash: cherry picked from commit 4e04814 zcash: bitcoin/bitcoin#9243
zcash: cherry picked from commit 3c37dc4 zcash: bitcoin/bitcoin#9626
This removes a "race" between Interrupt() and Run(), though it should not effect any of our supported platforms. zcash: cherry picked from commit 7b2d96b zcash: bitcoin/bitcoin#9679
zcash: cherry picked from commit d63ff62 zcash: bitcoin/bitcoin#9227
zcash: cherry picked from commit 45e2e08 zcash: bitcoin/bitcoin#9698
zcash: cherry picked from commit 9a0b784 zcash: bitcoin/bitcoin#9698
These are (afaik) all long-standing races or concurrent accesses. Going forward, we can clean these up so that they're not all individual atomic accesses. - Reintroduce cs_vRecv to guard receive-specific vars - Lock vRecv/vSend for CNodeStats - Make some vars atomic. - Only set the connection time in CNode's constructor so that it doesn't change zcash: cherry picked from commit 321d0fc zcash: bitcoin/bitcoin#9708
zcash: cherry picked from commit 644f123 zcash: bitcoin/bitcoin#9708
zcash: cherry picked from commit ae683c1 zcash: bitcoin/bitcoin#9708
LarryRuane
force-pushed
the
upstream-locking-0.14
branch
from
b9d7f27 to
7b4fe98
Compare
|
Force-pushed to rebase onto latest master (which now includes #4999). |
zcash: cherry picked from commit 512731b zcash: bitcoin/bitcoin#9708
zcash: cherry picked from commit 96f42d8 zcash: bitcoin/bitcoin#9708
zcash: cherry picked from commit 0f31872 zcash: bitcoin/bitcoin#9708
zcash: cherry picked from commit 22b4966 zcash: bitcoin/bitcoin#9708
zcash: cherry picked from commit d8f2b8a zcash: bitcoin/bitcoin#9708
zcash: cherry picked from commit 036073b zcash: bitcoin/bitcoin#9708
zcash: cherry picked from commit db2dc7a zcash: bitcoin/bitcoin#9708
A new AssertLockHeld(cs_wallet) call was added in commit a58370e "Dedup nTimeFirstKey update logic" (part of PR #9108). The lock held assertion will fail when loading prexisting wallets files from before the #9108 merge that have watch-only keys. zcash: cherry picked from commit 07afcd6 zcash: bitcoin/bitcoin#9771
str4d
force-pushed
the
upstream-locking-0.14
branch
from
7b4fe98 to
4fd7387
Compare
str4d
approved these changes
Apr 1, 2021
|
@zkbot r+ |
|
📌 Commit 4fd7387 has been approved by |
|
⌛ Testing commit 4fd7387 with merge 1b5f17c900ccb344f3952cf5f10a7cd7a7c48bbb... |
|
💔 Test failed - pr-merge |
|
Transient failure. @zkbot retry |
This was referenced Apr 2, 2021
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.14, oldest to newest (when merged to the master branch).
Each commit also includes a reference both to the PR and the upstream commit.
LogTimestampStranymore).IsArgSetlocking (we haven't pulled that function in yet).CConnman::PushMessagein second commit (which we don't have yet).