Page MenuHomePhabricator

gui: Fix race in WalletModel::pollBalanceChanged
ClosedPublic

Authored by PiRK on Dec 30 2020, 17:13.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABC527cee36c452: gui: Fix race in WalletModel::pollBalanceChanged
Summary

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#r378452145
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 PR17954 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 <falke.marco@gmail.com> 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 <john@johnnewbery.com> for finding this bug this while
reviewing PR17954.

This is a backport of Core PR18123

Test Plan

ninja && src/qt/bitcoin-qt

Diff Detail

Repository
rABC Bitcoin ABC
Branch
pr18123
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 14535
Build 29032: Build Diffbuild-debug
Build 29031: arc lint + arc unit

Event Timeline

PiRK requested review of this revision.Dec 30 2020, 17:13
This revision is now accepted and ready to land.Dec 30 2020, 20:57
This revision was landed with ongoing or failed builds.Jan 2 2021, 08:36
This revision was automatically updated to reflect the committed changes.