Page MenuHomePhabricator

D5080.id15831.diff
No OneTemporary

D5080.id15831.diff

diff --git a/doc/release-notes.md b/doc/release-notes.md
--- a/doc/release-notes.md
+++ b/doc/release-notes.md
@@ -3,3 +3,5 @@
<https://download.bitcoinabc.org/0.20.12/>
This release includes the following features and fixes:
+ - The `unloadwallet` RPC is now synchronous, meaning that it blocks until the
+ wallet is fully unloaded.
diff --git a/src/init.cpp b/src/init.cpp
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -260,10 +260,10 @@
LogPrintf("%s: Unable to remove pidfile: %s\n", __func__, e.what());
}
#endif
+ interfaces.chain_clients.clear();
UnregisterAllValidationInterfaces();
GetMainSignals().UnregisterBackgroundSignalScheduler();
GetMainSignals().UnregisterWithMempoolSignals(g_mempool);
- interfaces.chain_clients.clear();
globalVerifyHandle.reset();
ECC_Stop();
LogPrintf("%s: done\n", __func__);
diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp
--- a/src/wallet/init.cpp
+++ b/src/wallet/init.cpp
@@ -366,7 +366,11 @@
}
void UnloadWallets() {
- for (const std::shared_ptr<CWallet> &pwallet : GetWallets()) {
- RemoveWallet(pwallet);
+ auto wallets = GetWallets();
+ while (!wallets.empty()) {
+ auto wallet = wallets.back();
+ wallets.pop_back();
+ RemoveWallet(wallet);
+ UnloadWallet(std::move(wallet));
}
}
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -3957,16 +3957,8 @@
if (!RemoveWallet(wallet)) {
throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded");
}
- UnregisterValidationInterface(wallet.get());
- // The wallet can be in use so it's not possible to explicitly unload here.
- // Just notify the unload intent so that all shared pointers are released.
- // The wallet will be destroyed once the last shared pointer is released.
- wallet->NotifyUnload();
-
- // There's no point in waiting for the wallet to unload.
- // At this point this method should never fail. The unloading could only
- // fail due to an unexpected error which would cause a process termination.
+ UnloadWallet(std::move(wallet));
return NullUniValue;
}
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -60,6 +60,13 @@
//! Close all wallets.
void UnloadWallets();
+//! Explicitly unload and delete the wallet.
+// Blocks the current thread after signaling the unload intent so that all
+// wallet clients release the wallet.
+// Note that, when blocking is not required, the wallet is implicitly unloaded
+// by the shared pointer deleter.
+void UnloadWallet(std::shared_ptr<CWallet> &&wallet);
+
bool AddWallet(const std::shared_ptr<CWallet> &wallet);
bool RemoveWallet(const std::shared_ptr<CWallet> &wallet);
bool HasWallets();
@@ -942,6 +949,8 @@
chainParams(chainParamsIn) {}
~CWallet() {
+ // Should not have slots connected at this point.
+ assert(NotifyUnload.empty());
delete encrypted_batch;
encrypted_batch = nullptr;
}
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -88,12 +88,52 @@
return nullptr;
}
+static Mutex g_wallet_release_mutex;
+static std::condition_variable g_wallet_release_cv;
+static std::set<std::string> g_unloading_wallet_set;
+
// Custom deleter for shared_ptr<CWallet>.
static void ReleaseWallet(CWallet *wallet) {
+ // Unregister and delete the wallet right after
+ // BlockUntilSyncedToCurrentChain so that it's in sync with the current
+ // chainstate.
+ const std::string name = wallet->GetName();
wallet->WalletLogPrintf("Releasing wallet\n");
wallet->BlockUntilSyncedToCurrentChain();
wallet->Flush();
+ UnregisterValidationInterface(wallet);
delete wallet;
+ // Wallet is now released, notify UnloadWallet, if any.
+ {
+ LOCK(g_wallet_release_mutex);
+ if (g_unloading_wallet_set.erase(name) == 0) {
+ // UnloadWallet was not called for this wallet, all done.
+ return;
+ }
+ }
+ g_wallet_release_cv.notify_all();
+}
+
+void UnloadWallet(std::shared_ptr<CWallet> &&wallet) {
+ // Mark wallet for unloading.
+ const std::string name = wallet->GetName();
+ {
+ LOCK(g_wallet_release_mutex);
+ auto it = g_unloading_wallet_set.insert(name);
+ assert(it.second);
+ }
+ // The wallet can be in use so it's not possible to explicitly unload here.
+ // Notify the unload intent so that all remaining shared pointers are
+ // released.
+ wallet->NotifyUnload();
+ // Time to ditch our shared_ptr and wait for ReleaseWallet call.
+ wallet.reset();
+ {
+ WAIT_LOCK(g_wallet_release_mutex, lock);
+ while (g_unloading_wallet_set.count(name) == 1) {
+ g_wallet_release_cv.wait(lock);
+ }
+ }
}
static const size_t OUTPUT_GROUP_MAX_ENTRIES = 10;

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 1, 10:08 (3 h, 57 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5185673
Default Alt Text
D5080.id15831.diff (4 KB)

Event Timeline