Page MenuHomePhabricator

qt: lock cs_main, m_cached_tip_mutex in that order
AbandonedPublic

Authored by PiRK on Feb 22 2021, 15:04.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

Always lock the mutexes cs_main and m_cached_tip_mutex in
the same order: cs_main, m_cached_tip_mutex. Otherwise we may end up
in a deadlock.

ClientModel::m_cached_tip_blocks is protected by
ClientModel::m_cached_tip_mutex. There are two access paths that
lock the two mutexes in opposite order:

validation.cpp:2868 CChainState::ActivateBestChain(): lock cs_main
validation.cpp:2916 CChainState::ActivateBestChain(): call uiInterface.NotifyBlockTip()
ui_interface.cpp:52 CClientUIInterface::NotifyBlockTip(): go deep in boost
...
qt/clientmodel.cpp:255 BlockTipChanged(): lock m_cached_tip_mutex

and

qt/clientmodel.cpp:119 ClientModel::getBestBlockHash(): lock m_cached_tip_mutex
qt/clientmodel.cpp:121 ClientModel::getBestBlockHash(): call m_node.getBestBlockHash()
interfaces/node.cpp:200 NodeImpl::getBestBlockHash(): lock cs_main

From debug.log:

POTENTIAL DEADLOCK DETECTED
Previous lock order was:
 m_cs_chainstate validation.cpp:2851
 (1) cs_main validation.cpp:2868
 ::mempool.cs validation.cpp:2868
 (2) clientmodel->m_cached_tip_mutex qt/clientmodel.cpp:255
Current lock order is:
 (2) m_cached_tip_mutex qt/clientmodel.cpp:119
 (1) ::cs_main interfaces/node.cpp:200

The possible deadlock was introduced in
https://github.com/bitcoin/bitcoin/pull/17993

This is a backport of Core PR19132
Depends on D9254

Test Plan
ninja && src/qt/bitcoin-qt