diff options
author | Jonas Schnelli <dev@jonasschnelli.ch> | 2020-02-13 08:44:33 +0100 |
---|---|---|
committer | Jonas Schnelli <dev@jonasschnelli.ch> | 2020-02-13 08:44:36 +0100 |
commit | b6a16fa44ef27cb597dae7e264a7028352740bd5 (patch) | |
tree | ef28e49c033b9b20949bcfba30e5440d107005af | |
parent | 2bdc476d4d23256d8396bb9051a511f540d87392 (diff) | |
parent | bf36a3ccc212ad4d7c5cb8f26d7a22e279fe3cec (diff) |
Merge #18123: gui: Fix race in WalletModel::pollBalanceChanged
bf36a3ccc212ad4d7c5cb8f26d7a22e279fe3cec gui: Fix race in WalletModel::pollBalanceChanged (Russell Yanofsky)
Pull request description:
Poll function was wrongly setting cached height to the current chain height instead of the chain height at the time of polling.
This bug could cause balances to appear out of date, and was first introduced https://github.com/bitcoin/bitcoin/pull/10244/commits/a0704a8996bb950ae3c4d5b5a30e9dfe34cde1d3#diff-2e3836af182cfb375329c3463ffd91f8L117. Before that commit, there wasn't a problem because cs_main was held during the poll update.
Currently, the problem should be rare. But if 8937d99ce81a27ae5e1012a28323c0e26d89c50b from #17954 were merged, the problem would get worse, because the wrong cachedNumBlocks value would be set if the wallet was polled in the interval between a block being connected and it processing the BlockConnected notification.
MarcoFalke also points out that a0704a8996b could lead to GUI hangs as well, because previously the pollBalanceChanged method, which runs on the GUI thread, would only make a nonblocking TRY_LOCK(cs_main) call, but after could make blocking LOCK(cs_main) calls, potentially locking up the GUI.
Thanks to John Newbery for finding this bug this while reviewing https://github.com/bitcoin/bitcoin/pull/17954.
ACKs for top commit:
Empact:
utACK https://github.com/bitcoin/bitcoin/pull/18123/commits/bf36a3ccc212ad4d7c5cb8f26d7a22e279fe3cec
jonasschnelli:
utACK bf36a3c
Tree-SHA512: 1f4f229fa70a6d1fcf7be3806dca3252e86bc1755168fb421258389eb95aae67f863cb1216e6dc086b596c33560d1136215a4c87b5ff890abc8baaa3333b47f4
-rw-r--r-- | src/qt/walletmodel.cpp | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 6c3a06f3a2..8a84a8c168 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -82,12 +82,12 @@ void WalletModel::pollBalanceChanged() return; } - if(fForceCheckBalanceChanged || m_node.getNumBlocks() != cachedNumBlocks) + if(fForceCheckBalanceChanged || numBlocks != cachedNumBlocks) { fForceCheckBalanceChanged = false; // Balance and number of transactions might have changed - cachedNumBlocks = m_node.getNumBlocks(); + cachedNumBlocks = numBlocks; checkBalanceChanged(new_balances); if(transactionTableModel) |