From 44c7bb618eba208c094ca087804c0376cbb9a4c5 Mon Sep 17 00:00:00 2001 From: fanquake Date: Sat, 31 Aug 2019 08:50:17 +0800 Subject: [PATCH] Merge #16716: wallet: Use wallet name instead of pointer on unload/release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit d9d8984270dbb004ec94f8dbb289be2bc9e4dbc3 wallet: Use wallet name instead of pointer on unload/release (João Barbosa) Pull request description: Fixes #16668. Wallet name is unique so it can be used instead of pointer. ACKs for top commit: meshcollider: utACK d9d8984270dbb004ec94f8dbb289be2bc9e4dbc3 instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/16716/commits/d9d8984270dbb004ec94f8dbb289be2bc9e4dbc3 ryanofsky: utACK d9d8984270dbb004ec94f8dbb289be2bc9e4dbc3. Alternately I think it might be possible to use an intptr_t set instead of a string set to get around the undefined behavior described in the issue. Tree-SHA512: eccd4d260cd4c02b52c30deeb32dbfd190a1151a5340eb3aa4ece0dc6ae3b3ed746ce5617336461f6f27c437c435629cd07d20beb1c5450f23b75edde6728598 --- src/wallet/wallet.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 19ad2f2370..ff36270470 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -100,13 +100,14 @@ std::shared_ptr GetWallet(const std::string& name) 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_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(); @@ -115,7 +116,7 @@ static void ReleaseWallet(CWallet* wallet) // Wallet is now released, notify UnloadWallet, if any. { LOCK(g_wallet_release_mutex); - if (g_unloading_wallet_set.erase(wallet) == 0) { + if (g_unloading_wallet_set.erase(name) == 0) { // UnloadWallet was not called for this wallet, all done. return; } @@ -126,21 +127,21 @@ static void ReleaseWallet(CWallet* wallet) void UnloadWallet(std::shared_ptr&& wallet) { // Mark wallet for unloading. - CWallet* pwallet = wallet.get(); + const std::string name = wallet->GetName(); { LOCK(g_wallet_release_mutex); - auto it = g_unloading_wallet_set.insert(pwallet); + 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. - pwallet->NotifyUnload(); + 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(pwallet) == 1) { + while (g_unloading_wallet_set.count(name) == 1) { g_wallet_release_cv.wait(lock); } }