diff options
| -rw-r--r-- | src/wallet/context.h | 2 | ||||
| -rw-r--r-- | src/wallet/test/wallet_tests.cpp | 6 | ||||
| -rw-r--r-- | src/wallet/wallet.cpp | 6 |
3 files changed, 5 insertions, 9 deletions
diff --git a/src/wallet/context.h b/src/wallet/context.h index a382fb9021..dbd172e88e 100644 --- a/src/wallet/context.h +++ b/src/wallet/context.h @@ -34,6 +34,8 @@ using LoadWalletFn = std::function<void(std::unique_ptr<interfaces::Wallet> wall struct WalletContext { interfaces::Chain* chain{nullptr}; ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct + // It is unsafe to lock this after locking a CWallet::cs_wallet mutex because + // this could introduce inconsistent lock ordering and cause deadlocks. Mutex wallets_mutex; std::vector<std::shared_ptr<CWallet>> wallets GUARDED_BY(wallets_mutex); std::list<LoadWalletFn> wallet_load_fns GUARDED_BY(wallets_mutex); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 4499eb5903..6bf0d7b45b 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -783,18 +783,14 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // deadlock during the sync and simulates a new block notification happening // as soon as possible. addtx_count = 0; - auto handler = HandleLoadWallet(context, [&](std::unique_ptr<interfaces::Wallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->wallet()->cs_wallet, context.wallets_mutex) { + auto handler = HandleLoadWallet(context, [&](std::unique_ptr<interfaces::Wallet> wallet) { BOOST_CHECK(rescan_completed); m_coinbase_txns.push_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); block_tx = TestSimpleSpend(*m_coinbase_txns[2], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey())); m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); mempool_tx = TestSimpleSpend(*m_coinbase_txns[3], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey())); BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error)); - LEAVE_CRITICAL_SECTION(context.wallets_mutex); - LEAVE_CRITICAL_SECTION(wallet->wallet()->cs_wallet); SyncWithValidationInterfaceQueue(); - ENTER_CRITICAL_SECTION(wallet->wallet()->cs_wallet); - ENTER_CRITICAL_SECTION(context.wallets_mutex); }); wallet = TestLoadWallet(context); BOOST_CHECK_EQUAL(addtx_count, 4); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 155c27cfb6..6ecf51c2b7 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2759,8 +2759,6 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri // Try to top up keypool. No-op if the wallet is locked. walletInstance->TopUpKeyPool(); - LOCK(walletInstance->cs_wallet); - if (chain && !AttachChain(walletInstance, *chain, rescan_required, error, warnings)) { return nullptr; } @@ -2772,9 +2770,9 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri } } - walletInstance->SetBroadcastTransactions(args.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); - { + LOCK(walletInstance->cs_wallet); + walletInstance->SetBroadcastTransactions(args.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); walletInstance->WalletLogPrintf("setKeyPool.size() = %u\n", walletInstance->GetKeyPoolSize()); walletInstance->WalletLogPrintf("mapWallet.size() = %u\n", walletInstance->mapWallet.size()); walletInstance->WalletLogPrintf("m_address_book.size() = %u\n", walletInstance->m_address_book.size()); |