gui: fix crash when closing wallet

The crash occurs because 'WalletController::removeAndDeleteWallet' is called
twice for the same wallet model: first in the GUI's button connected function
'WalletController::closeWallet', and then again when the backend emits the
'WalletModel::unload' signal.

This causes the issue because 'removeAndDeleteWallet' inlines an
erase(std::remove()). So, if 'std::remove' returns an iterator to the end
(indicating the element wasn't found because it was already erased), the
subsequent call to 'erase' leads to an undefined behavior.

Github-Pull: bitcoin-core/gui#835
Rebased-From: a965f2bc07
This commit is contained in:
furszy 2024-09-12 18:50:43 -03:00 committed by Ava Chow
parent d39262e5d4
commit 674dded875
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
2 changed files with 13 additions and 8 deletions

View file

@ -79,6 +79,14 @@ std::map<std::string, std::pair<bool, std::string>> WalletController::listWallet
return wallets; return wallets;
} }
void WalletController::removeWallet(WalletModel* wallet_model)
{
// Once the wallet is successfully removed from the node, the model will emit the 'WalletModel::unload' signal.
// This signal is already connected and will complete the removal of the view from the GUI.
// Look at 'WalletController::getOrCreateWallet' for the signal connection.
wallet_model->wallet().remove();
}
void WalletController::closeWallet(WalletModel* wallet_model, QWidget* parent) void WalletController::closeWallet(WalletModel* wallet_model, QWidget* parent)
{ {
QMessageBox box(parent); QMessageBox box(parent);
@ -89,10 +97,7 @@ void WalletController::closeWallet(WalletModel* wallet_model, QWidget* parent)
box.setDefaultButton(QMessageBox::Yes); box.setDefaultButton(QMessageBox::Yes);
if (box.exec() != QMessageBox::Yes) return; if (box.exec() != QMessageBox::Yes) return;
// First remove wallet from node. removeWallet(wallet_model);
wallet_model->wallet().remove();
// Now release the model.
removeAndDeleteWallet(wallet_model);
} }
void WalletController::closeAllWallets(QWidget* parent) void WalletController::closeAllWallets(QWidget* parent)
@ -105,11 +110,8 @@ void WalletController::closeAllWallets(QWidget* parent)
QMutexLocker locker(&m_mutex); QMutexLocker locker(&m_mutex);
for (WalletModel* wallet_model : m_wallets) { for (WalletModel* wallet_model : m_wallets) {
wallet_model->wallet().remove(); removeWallet(wallet_model);
Q_EMIT walletRemoved(wallet_model);
delete wallet_model;
} }
m_wallets.clear();
} }
WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wallet> wallet) WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wallet> wallet)

View file

@ -85,6 +85,9 @@ private:
friend class WalletControllerActivity; friend class WalletControllerActivity;
friend class MigrateWalletActivity; friend class MigrateWalletActivity;
//! Starts the wallet closure procedure
void removeWallet(WalletModel* wallet_model);
}; };
class WalletControllerActivity : public QObject class WalletControllerActivity : public QObject