Merge #19300: wallet: Handle concurrent wallet loading
9b009fae6e
qa: Test concurrent wallet loading (João Barbosa)b9971ae585
wallet: Handle concurrent wallet loading (João Barbosa) Pull request description: This PR handles concurrent wallet loading. This can be tested by running in parallel the following script a couple of times: ```sh for i in {1..10} do src/bitcoin-cli -regtest loadwallet foo src/bitcoin-cli -regtest unloadwallet foo done ``` Eventually the error occurs: ``` error code: -4 error message: Wallet already being loading. ``` For reference, loading and already loaded wallet gives: ``` error code: -4 error message: Wallet file verification failed. Error loading wallet w1. Duplicate -wallet filename specified. ``` Fixes #19232. ACKs for top commit: MarcoFalke: Concept ACK9b009fae6e
I have not reviewed the code hebasto: ACK9b009fae6e
, tested on Linux Mint 20 (x86_64): ryanofsky: Code review good-but-not-ideal ACK9b009fae6e
Tree-SHA512: 0ccd77b03c0926e4c4e51efb31e193b93cb4b9ffe8bac6bb018f7344c55dfd939b873b8cf5e657dca73e6202eb75aa672de2acb787cc133184b0b3b51e47b972
This commit is contained in:
commit
5c3c7cc50c
2 changed files with 47 additions and 2 deletions
|
@ -99,9 +99,11 @@ std::unique_ptr<interfaces::Handler> 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<std::string> g_unloading_wallet_set;
|
||||
static std::set<std::string> g_loading_wallet_set GUARDED_BY(g_loading_wallet_mutex);
|
||||
static std::set<std::string> g_unloading_wallet_set GUARDED_BY(g_wallet_release_mutex);
|
||||
|
||||
// Custom deleter for shared_ptr<CWallet>.
|
||||
static void ReleaseWallet(CWallet* wallet)
|
||||
|
@ -145,7 +147,8 @@ void UnloadWallet(std::shared_ptr<CWallet>&& wallet)
|
|||
}
|
||||
}
|
||||
|
||||
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& warnings)
|
||||
namespace {
|
||||
std::shared_ptr<CWallet> LoadWalletInternal(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& warnings)
|
||||
{
|
||||
try {
|
||||
if (!CWallet::Verify(chain, location, error, warnings)) {
|
||||
|
@ -166,6 +169,19 @@ std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocati
|
|||
return nullptr;
|
||||
}
|
||||
}
|
||||
} // namespace
|
||||
|
||||
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& 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<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings)
|
||||
{
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in a new issue