diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 57eec9baf..fbb1a58ea 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -99,9 +99,11 @@ std::unique_ptr HandleLoadWallet(LoadWalletFn load_wallet) return interfaces::MakeHandler([it] { LOCK(cs_wallets); g_load_wallet_fns.erase(it); }); } +static Mutex g_loading_wallet_mutex; static Mutex g_wallet_release_mutex; static std::condition_variable g_wallet_release_cv; -static std::set g_unloading_wallet_set; +static std::set g_loading_wallet_set GUARDED_BY(g_loading_wallet_mutex); +static std::set g_unloading_wallet_set GUARDED_BY(g_wallet_release_mutex); // Custom deleter for shared_ptr. static void ReleaseWallet(CWallet* wallet) @@ -145,7 +147,8 @@ void UnloadWallet(std::shared_ptr&& wallet) } } -std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector& warnings) +namespace { +std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector& warnings) { try { if (!CWallet::Verify(chain, location, error, warnings)) { @@ -166,6 +169,19 @@ std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocati return nullptr; } } +} // namespace + +std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector& warnings) +{ + auto result = WITH_LOCK(g_loading_wallet_mutex, return g_loading_wallet_set.insert(location.GetName())); + if (!result.second) { + error = Untranslated("Wallet already being loading."); + return nullptr; + } + auto wallet = LoadWalletInternal(chain, location, error, warnings); + WITH_LOCK(g_loading_wallet_mutex, g_loading_wallet_set.erase(result.first)); + return wallet; +} std::shared_ptr LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector& warnings) { diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index ff9ff3418..88beef103 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -7,19 +7,36 @@ Verify that a bitcoind node can load multiple wallet files """ from decimal import Decimal +from threading import Thread import os import shutil import time +from test_framework.authproxy import JSONRPCException from test_framework.test_framework import BitcoinTestFramework from test_framework.test_node import ErrorMatch from test_framework.util import ( assert_equal, assert_raises_rpc_error, + get_rpc_proxy, ) FEATURE_LATEST = 169900 +got_loading_error = False +def test_load_unload(node, name): + global got_loading_error + for i in range(10): + if got_loading_error: + return + try: + node.loadwallet(name) + node.unloadwallet(name) + except JSONRPCException as e: + if e.error['code'] == -4 and 'Wallet already being loading' in e.error['message']: + got_loading_error = True + return + class MultiWalletTest(BitcoinTestFramework): def set_test_params(self): @@ -212,6 +229,18 @@ class MultiWalletTest(BitcoinTestFramework): w2 = node.get_wallet_rpc(wallet_names[1]) w2.getwalletinfo() + self.log.info("Concurrent wallet loading") + threads = [] + for _ in range(3): + n = node.cli if self.options.usecli else get_rpc_proxy(node.url, 1, timeout=600, coveragedir=node.coverage_dir) + t = Thread(target=test_load_unload, args=(n, wallet_names[2], )) + t.start() + threads.append(t) + for t in threads: + t.join() + global got_loading_error + assert_equal(got_loading_error, True) + self.log.info("Load remaining wallets") for wallet_name in wallet_names[2:]: loadwallet_name = self.nodes[0].loadwallet(wallet_name)