Skip to content

[pull] master from bitcoin:master#736

Merged
pull[bot] merged 6 commits intouniibu:masterfrom
bitcoin:master
May 1, 2020
Merged

[pull] master from bitcoin:master#736
pull[bot] merged 6 commits intouniibu:masterfrom
bitcoin:master

Conversation

@pull
Copy link
Copy Markdown

@pull pull bot commented May 1, 2020

See Commits and Changes for more details.


Created by pull[bot]. Want to support this open source service? Please star it : )

Antoine Riard and others added 6 commits April 30, 2020 14:31
Instead of calling getHeight, we rely on CWallet::m_last_block
processed_height where it's possible.
Add HaveChain to assert chain access for wallet-tool in LoadToWallet.
Remove findPruned and findFork, no more used after 17954.
This change is intended to make the bitcoin node and its rpc, network
and gui interfaces more responsive while the wallet is in use. Currently
because the node's cs_main mutex is always locked before the wallet's
cs_wallet mutex (to prevent deadlocks), cs_main currently stays locked
while the wallet does relatively slow things like creating and listing
transactions.

This commit only remmove chain lock tacking in wallet code, and invert
lock order from cs_main, cs_wallet to cs_wallet, cs_main.
must happen at once to avoid any deadlock. Previous commit were only
removing Chain::Lock methods to Chain interface and enforcing they
take cs_main.

Remove LockChain method from CWallet and Chain::Lock interface.
…n locking

6a72f26 [wallet] Remove locked_chain from CWallet, its RPCs and tests (Antoine Riard)
8411788 [wallet] Move methods from Chain::Lock interface to simple Chain (Antoine Riard)
0a76287 [wallet] Move getBlockHash from Chain::Lock interface to simple Chain (Antoine Riard)
de13363 [wallet] Move getBlockHeight from Chain::Lock interface to simple Chain (Antoine Riard)
b855592 [wallet] Move getHeight from Chain::Lock interface to simple Chain (Antoine Riard)

Pull request description:

  This change is intended to make the bitcoin node and its rpc, network and gui interfaces more responsive while the wallet is in use. Currently, because the node's `cs_main` mutex is always locked before the wallet's `cs_wallet` mutex (to prevent deadlocks), `cs_main` currently stays locked while the wallet does relatively slow things like creating and listing transactions.

  Switching the lock order so `cs_main` is acquired after `cs_wallet` allows `cs_main` to be only locked intermittently while the wallet is doing slow operations, so the node is not blocked waiting for the wallet.

  To review the present PR, most of getting right the move is ensuring any `LockAssertion` in `Chain::Lock` method is amended as a `LOCK(cs_main)`. And in final commit, check that any wallet code which was previously locking the chain is now calling a  method, enforcing the lock taking job. So far the only exception I found is `handleNotifications`, which should be corrected.

ACKs for top commit:
  MarcoFalke:
    re-ACK 6a72f26 🔏
  fjahr:
    re-ACK 6a72f26
  ryanofsky:
    Code review ACK 6a72f26. Only difference compared to the rebase I posted is reverting unneeded SetLastBlockProcessed change in wallet_disableprivkeys test

Tree-SHA512: 9168b3bf3432d4f8bc4d9fa9246ac057050848e673efc264c8f44345f243ba9697b05c22c809a79d1b51bf0de1c4ed317960e496480f8d71e584468d4dd1b0ad
@pull pull bot added the ⤵️ pull label May 1, 2020
@pull pull bot merged commit 608359b into uniibu:master May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants