diff --git a/doc/release-notes-14941.md b/doc/release-notes-14941.md new file mode 100644 index 0000000000..c3820d0368 --- /dev/null +++ b/doc/release-notes-14941.md @@ -0,0 +1,5 @@ +Miscellaneous RPC changes +------------ + +- 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 index a9d7e8e608..8ff72b1f04 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -375,6 +375,7 @@ void PrepareShutdown() LogPrintf("%s: Unable to remove PID file: %s\n", __func__, e.what()); } #endif + g_wallet_init_interface.Close(); UnregisterAllValidationInterfaces(); GetMainSignals().UnregisterBackgroundSignalScheduler(); GetMainSignals().UnregisterWithMempoolSignals(mempool); @@ -396,7 +397,6 @@ void Shutdown() PrepareShutdown(); } // Shutdown part 2: delete wallet instance - g_wallet_init_interface.Close(); globalVerifyHandle.reset(); ECC_Stop(); LogPrintf("%s: done\n", __func__); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index eec3396cfc..41ee1062c4 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -402,8 +402,12 @@ void WalletInit::Stop() const void WalletInit::Close() const { - 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 index 0d23aa9e36..bb0b1512b8 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3487,16 +3487,8 @@ static UniValue unloadwallet(const JSONRPCRequest& request) 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.cpp b/src/wallet/wallet.cpp index a4ac6784e9..3066a02b6b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -98,14 +98,52 @@ std::shared_ptr GetWallet(const std::string& name) 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. 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(wallet) == 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. + CWallet* pwallet = wallet.get(); + { + LOCK(g_wallet_release_mutex); + auto it = g_unloading_wallet_set.insert(pwallet); + 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. + pwallet->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(pwallet) == 1) { + g_wallet_release_cv.wait(lock); + } + } } const uint256 CMerkleTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index ba9bb6e21e..b51b5c03ef 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -37,6 +37,13 @@ #include #include +//! 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(); @@ -891,6 +898,8 @@ public: ~CWallet() { + // Should not have slots connected at this point. + assert(NotifyUnload.empty()); delete encrypted_batch; encrypted_batch = nullptr; }