Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bitcoin 0.16 locking PRs #5017

Merged
merged 17 commits into from
Apr 17, 2021
Merged

Bitcoin 0.16 locking PRs #5017

merged 17 commits into from
Apr 17, 2021

Commits on Apr 12, 2021

  1. Acquire cs_main lock before cs_wallet during wallet initialization 

    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
    @ryanofsky @LarryRuane
    ryanofsky authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    e0a1266 View commit details
    Browse the repository at this point in the history

  2. keystore GetKeys(): return result instead of writing to reference 

    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
    @LarryRuane
    Marko Bencun authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    2fd8809 View commit details
    Browse the repository at this point in the history

  3. Make fUseCrypto atomic 

    zcash: cherry picked from commit c626dcb
zcash: bitcoin/bitcoin#11107
    @meshcollider @LarryRuane
    meshcollider authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    ea9bcc4 View commit details
    Browse the repository at this point in the history

  4. rpc: work-around an upstream libevent bug 

    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
    @theuni @LarryRuane
    theuni authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    92d8f1d View commit details
    Browse the repository at this point in the history

  5. rpc: further constrain the libevent workaround 

    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
    @theuni @LarryRuane
    theuni authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    af79142 View commit details
    Browse the repository at this point in the history

  6. addrman: Add missing lock in Clear() (CAddrMan) 

    The variable vRandom is guarded by the mutex cs.

zcash: cherry picked from commit 3ab545d
zcash: bitcoin/bitcoin#11585
    @practicalswift @LarryRuane
    practicalswift authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    29eb55c View commit details
    Browse the repository at this point in the history

  7. Add missing cs_main locks when calling blockToJSON/blockheaderToJSON 

    zcash: cherry picked from commit a9b6ba0
zcash: bitcoin/bitcoin#11618
    @practicalswift @LarryRuane
    practicalswift authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    d892626 View commit details
    Browse the repository at this point in the history

  8. Add ability to assert a lock is not held in DEBUG_LOCKORDER 

    zcash: cherry picked from commit 2b4b345
zcash: bitcoin/bitcoin#10286
    @TheBlueMatt @LarryRuane
    TheBlueMatt authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    756a33a View commit details
    Browse the repository at this point in the history

  9. Remove redundant pwallet nullptr check 

    zcash: cherry picked from commit 89f0312
zcash: bitcoin/bitcoin#10286
    @TheBlueMatt @LarryRuane
    TheBlueMatt authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    3145dce View commit details
    Browse the repository at this point in the history

  10. wallet: Remove unnecessary mempool lock in ReacceptWalletTransactions 

    zcash: cherry picked from commit 5b25293
zcash: bitcoin/bitcoin#11870
    @promag @LarryRuane
    promag authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    3d29bb2 View commit details
    Browse the repository at this point in the history

  11. [rpc] Reduce scope of cs_main and cs_wallet locks in listtransactions 

    zcash: cherry picked from commit c409b1a
zcash: bitcoin/bitcoin#12330
    @promag @LarryRuane
    promag authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    25b8cdc View commit details
    Browse the repository at this point in the history

  12. http: Join worker threads before deleting work queue 

    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
    @laanwj @LarryRuane
    laanwj authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    0dcbfc9 View commit details
    Browse the repository at this point in the history

  13. http: Remove WaitExit from WorkQueue 

    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
    @laanwj @LarryRuane
    laanwj authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    c733ded View commit details
    Browse the repository at this point in the history

  14. http: Remove numThreads and ThreadCounter 

    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
    @laanwj @LarryRuane
    laanwj authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    cbd1113 View commit details
    Browse the repository at this point in the history

  15. Hold mempool.cs for the duration of ATMP. 

    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
    @TheBlueMatt @LarryRuane
    TheBlueMatt authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    a4730c8 View commit details
    Browse the repository at this point in the history

  16. Add braces to meet code style on line-after-the-one-changed. 

    zcash: cherry picked from commit 02fc886
zcash: bitcoin/bitcoin#12368
    @TheBlueMatt @LarryRuane
    TheBlueMatt authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    829e801 View commit details
    Browse the repository at this point in the history

  17. [wallet] Make CWallet::ListCoins atomic 

    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
    @promag @LarryRuane
    promag authored and LarryRuane committed Apr 12, 2021
    Configuration menu
    Copy the full SHA
    5b2f7a6 View commit details
    Browse the repository at this point in the history