Page MenuHomePhabricator

Acquire cs_main lock before cs_wallet during wallet initialization
ClosedPublic

Authored by Fabien on Thu, Feb 28, 16:15.

Details

Summary
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/

Backport of core PR11126

Test Plan
../configure --enable-debug
make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien created this revision.Thu, Feb 28, 16:15
Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Feb 28, 16:15
Herald added a subscriber: schancel. · View Herald Transcript
deadalnix accepted this revision.Thu, Feb 28, 16:36
This revision is now accepted and ready to land.Thu, Feb 28, 16:36
Closed by commit rABCd415ec10755d: Acquire cs_main lock before cs_wallet during wallet initialization (authored by Russell Yanofsky <russ@yanofsky.org>, committed by Fabien). · Explain WhyThu, Feb 28, 16:40
This revision was automatically updated to reflect the committed changes.