diff options
| author | furszy <matiasfurszyfer@protonmail.com> | 2024-09-12 18:50:43 -0300 |
|---|---|---|
| committer | Ava Chow <github@achow101.com> | 2024-09-13 11:32:55 -0400 |
| commit | 674dded8756ddf9b731f3149c66dd044090e4c8e (patch) | |
| tree | 98df902e5081652641686b55ccbca8c31f027a93 | |
| parent | d39262e5d41e92d22e020d283ddb6e4e406647b2 (diff) | |
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: a965f2bc07a3588f8c2b8d6a542961562e3f5d0e
| -rw-r--r-- | src/qt/walletcontroller.cpp | 18 | ||||
| -rw-r--r-- | src/qt/walletcontroller.h | 3 |
2 files changed, 13 insertions, 8 deletions
diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 512ea8a1dc..dd093e984a 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -79,6 +79,14 @@ std::map<std::string, std::pair<bool, std::string>> WalletController::listWallet 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) { QMessageBox box(parent); @@ -89,10 +97,7 @@ void WalletController::closeWallet(WalletModel* wallet_model, QWidget* parent) box.setDefaultButton(QMessageBox::Yes); if (box.exec() != QMessageBox::Yes) return; - // First remove wallet from node. - wallet_model->wallet().remove(); - // Now release the model. - removeAndDeleteWallet(wallet_model); + removeWallet(wallet_model); } void WalletController::closeAllWallets(QWidget* parent) @@ -105,11 +110,8 @@ void WalletController::closeAllWallets(QWidget* parent) QMutexLocker locker(&m_mutex); for (WalletModel* wallet_model : m_wallets) { - wallet_model->wallet().remove(); - Q_EMIT walletRemoved(wallet_model); - delete wallet_model; + removeWallet(wallet_model); } - m_wallets.clear(); } WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wallet> wallet) diff --git a/src/qt/walletcontroller.h b/src/qt/walletcontroller.h index 7902c3b235..4d2ba43539 100644 --- a/src/qt/walletcontroller.h +++ b/src/qt/walletcontroller.h @@ -85,6 +85,9 @@ private: friend class WalletControllerActivity; friend class MigrateWalletActivity; + + //! Starts the wallet closure procedure + void removeWallet(WalletModel* wallet_model); }; class WalletControllerActivity : public QObject |