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 @@ 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 &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 &&wallet); + bool AddWallet(const std::shared_ptr &wallet); bool RemoveWallet(const std::shared_ptr &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 g_unloading_wallet_set; + // Custom deleter for shared_ptr. 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 &&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;