From 13f618818dc57673ac0287ad8b28ceb450efb374 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 7 Jun 2021 18:00:36 +0300 Subject: [PATCH 01/10] qt: Add GUIUtil::ShowModalDialogAndDeleteOnClose --- src/qt/guiutil.cpp | 8 ++++++++ src/qt/guiutil.h | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index e98e50ba148..bbd33f1de0f 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -958,4 +959,11 @@ void PrintSlotException( PrintExceptionContinue(exception, description.c_str()); } +void ShowModalDialogAndDeleteOnClose(QDialog* dialog) +{ + dialog->setAttribute(Qt::WA_DeleteOnClose); + dialog->setWindowModality(Qt::ApplicationModal); + dialog->show(); +} + } // namespace GUIUtil diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index 06a3b636686..274f0bdcbfd 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -41,6 +41,7 @@ class QAbstractButton; class QAbstractItemView; class QAction; class QDateTime; +class QDialog; class QFont; class QKeySequence; class QLineEdit; @@ -417,6 +418,11 @@ namespace GUIUtil type); } + /** + * Shows a QDialog instance asynchronously, and deletes it on close. + */ + void ShowModalDialogAndDeleteOnClose(QDialog* dialog); + } // namespace GUIUtil #endif // BITCOIN_QT_GUIUTIL_H From 7830cd0b35f315570d744f4d2719104c08b33ff1 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 7 Jun 2021 18:17:59 +0300 Subject: [PATCH 02/10] qt, refactor: Keep OptionsDialog in the main event loop --- src/qt/bitcoingui.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 862bdd3bfe8..7954a7dcc96 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -990,10 +990,10 @@ void BitcoinGUI::openOptionsDialogWithTab(OptionsDialog::Tab tab) if (!clientModel || !clientModel->getOptionsModel()) return; - OptionsDialog dlg(this, enableWallet); - dlg.setCurrentTab(tab); - dlg.setModel(clientModel->getOptionsModel()); - dlg.exec(); + auto dlg = new OptionsDialog(this, enableWallet); + dlg->setCurrentTab(tab); + dlg->setModel(clientModel->getOptionsModel()); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, double nVerificationProgress, bool header, SynchronizationState sync_state) From 59f7ba4fd7a9e4bc73d784ee74d5b777da9cc436 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 7 Jun 2021 18:28:14 +0300 Subject: [PATCH 03/10] qt, refactor: Keep CoinControlDialog in the main event loop --- src/qt/sendcoinsdialog.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index c9bf757dfc9..22302ab72eb 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -914,9 +914,9 @@ void SendCoinsDialog::coinControlFeatureChanged(bool checked) // Coin Control: button inputs -> show actual coin control dialog void SendCoinsDialog::coinControlButtonClicked() { - CoinControlDialog dlg(*m_coin_control, model, platformStyle); - dlg.exec(); - coinControlUpdateLabels(); + auto dlg = new CoinControlDialog(*m_coin_control, model, platformStyle); + connect(dlg, &QDialog::finished, this, &SendCoinsDialog::coinControlUpdateLabels); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } // Coin Control: checkbox custom change address From 6f6fde30e7601185a8f6052b3bf1770407fcc14b Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 7 Jun 2021 18:31:51 +0300 Subject: [PATCH 04/10] qt, refactor: Keep EditAddressDialog in the main event loop --- src/qt/addressbookpage.cpp | 8 ++++---- src/qt/transactionview.cpp | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp index c31f0aceeaa..40f9f972c0a 100644 --- a/src/qt/addressbookpage.cpp +++ b/src/qt/addressbookpage.cpp @@ -182,14 +182,14 @@ void AddressBookPage::onEditAction() if(indexes.isEmpty()) return; - EditAddressDialog dlg( + auto dlg = new EditAddressDialog( tab == SendingTab ? EditAddressDialog::EditSendingAddress : EditAddressDialog::EditReceivingAddress, this); - dlg.setModel(model); + dlg->setModel(model); QModelIndex origIndex = proxyModel->mapToSource(indexes.at(0)); - dlg.loadRow(origIndex.row()); - dlg.exec(); + dlg->loadRow(origIndex.row()); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void AddressBookPage::on_newAddress_clicked() diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 908cb917f18..d04dd11ad13 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -500,22 +500,22 @@ void TransactionView::editLabel() // Determine type of address, launch appropriate editor dialog type QString type = modelIdx.data(AddressTableModel::TypeRole).toString(); - EditAddressDialog dlg( + auto dlg = new EditAddressDialog( type == AddressTableModel::Receive ? EditAddressDialog::EditReceivingAddress : EditAddressDialog::EditSendingAddress, this); - dlg.setModel(addressBook); - dlg.loadRow(idx); - dlg.exec(); + dlg->setModel(addressBook); + dlg->loadRow(idx); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } else { // Add sending address - EditAddressDialog dlg(EditAddressDialog::NewSendingAddress, + auto dlg = new EditAddressDialog(EditAddressDialog::NewSendingAddress, this); - dlg.setModel(addressBook); - dlg.setAddress(address); - dlg.exec(); + dlg->setModel(addressBook); + dlg->setAddress(address); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } } } From 7fa91e831227e556bd8a7ae3da64bd59d4f30d5f Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 7 Jun 2021 18:34:14 +0300 Subject: [PATCH 05/10] qt, refactor: Keep AskPassphraseDialog in the main event loop --- src/qt/walletview.cpp | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index 309806a1c40..7813b89e41a 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -205,11 +205,10 @@ void WalletView::showOutOfSyncWarning(bool fShow) void WalletView::encryptWallet() { - AskPassphraseDialog dlg(AskPassphraseDialog::Encrypt, this); - dlg.setModel(walletModel); - dlg.exec(); - - Q_EMIT encryptionStatusChanged(); + auto dlg = new AskPassphraseDialog(AskPassphraseDialog::Encrypt, this); + dlg->setModel(walletModel); + connect(dlg, &QDialog::finished, this, &WalletView::encryptionStatusChanged); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void WalletView::backupWallet() @@ -234,19 +233,18 @@ void WalletView::backupWallet() void WalletView::changePassphrase() { - AskPassphraseDialog dlg(AskPassphraseDialog::ChangePass, this); - dlg.setModel(walletModel); - dlg.exec(); + auto dlg = new AskPassphraseDialog(AskPassphraseDialog::ChangePass, this); + dlg->setModel(walletModel); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void WalletView::unlockWallet() { // Unlock wallet when requested by wallet model - if (walletModel->getEncryptionStatus() == WalletModel::Locked) - { - AskPassphraseDialog dlg(AskPassphraseDialog::Unlock, this); - dlg.setModel(walletModel); - dlg.exec(); + if (walletModel->getEncryptionStatus() == WalletModel::Locked) { + auto dlg = new AskPassphraseDialog(AskPassphraseDialog::Unlock, this); + dlg->setModel(walletModel); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } } From c8bae37a7a646badf8e79669bf06ac174e13cd6f Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 7 Jun 2021 18:37:13 +0300 Subject: [PATCH 06/10] qt, refactor: Keep PSBTOperationsDialog in the main event loop --- src/qt/walletframe.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index f694fbecb51..3c2e669c9a0 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -220,10 +220,9 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard) return; } - PSBTOperationsDialog* dlg = new PSBTOperationsDialog(this, currentWalletModel(), clientModel); + auto dlg = new PSBTOperationsDialog(this, currentWalletModel(), clientModel); dlg->openWithPSBT(psbtx); - dlg->setAttribute(Qt::WA_DeleteOnClose); - dlg->exec(); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void WalletFrame::encryptWallet() From 332dea2852d9c68f900ed1f0be99b6cea79c7457 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 7 Jun 2021 18:41:58 +0300 Subject: [PATCH 07/10] qt, refactor: Keep HelpMessageDialog in the main event loop --- src/qt/bitcoingui.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 7954a7dcc96..6ecd3c0f5ca 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -848,8 +848,8 @@ void BitcoinGUI::aboutClicked() if(!clientModel) return; - HelpMessageDialog dlg(this, true); - dlg.exec(); + auto dlg = new HelpMessageDialog(this, /* about */ true); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void BitcoinGUI::showDebugWindow() From b4e0d2c43181ad97c15b252e95181e2c3f6c1d2a Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 7 Jun 2021 18:43:13 +0300 Subject: [PATCH 08/10] qt, refactor: Allocate SendConfirmationDialog instances on heap This change is require for the next commit. --- src/qt/sendcoinsdialog.cpp | 7 ++++--- src/qt/walletmodel.cpp | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 22302ab72eb..29cd6d7c36d 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -399,9 +399,10 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked) const QString confirmation = model->wallet().privateKeysDisabled() && !model->wallet().hasExternalSigner() ? tr("Confirm transaction proposal") : tr("Confirm send coins"); const QString confirmButtonText = model->wallet().privateKeysDisabled() && !model->wallet().hasExternalSigner() ? tr("Create Unsigned") : tr("Sign and send"); - SendConfirmationDialog confirmationDialog(confirmation, question_string, informative_text, detailed_text, SEND_CONFIRM_DELAY, confirmButtonText, this); - confirmationDialog.exec(); - QMessageBox::StandardButton retval = static_cast(confirmationDialog.result()); + auto confirmationDialog = new SendConfirmationDialog(confirmation, question_string, informative_text, detailed_text, SEND_CONFIRM_DELAY, confirmButtonText, this); + confirmationDialog->setAttribute(Qt::WA_DeleteOnClose); + // TODO: Replace QDialog::exec() with safer QDialog::show(). + const auto retval = static_cast(confirmationDialog->exec()); if(retval != QMessageBox::Yes) { diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 967dd588b49..052453cf65d 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -506,9 +506,10 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) questionString.append(tr("Warning: This may pay the additional fee by reducing change outputs or adding inputs, when necessary. It may add a new change output if one does not already exist. These changes may potentially leak privacy.")); } - SendConfirmationDialog confirmationDialog(tr("Confirm fee bump"), questionString); - confirmationDialog.exec(); - QMessageBox::StandardButton retval = static_cast(confirmationDialog.result()); + auto confirmationDialog = new SendConfirmationDialog(tr("Confirm fee bump"), questionString); + confirmationDialog->setAttribute(Qt::WA_DeleteOnClose); + // TODO: Replace QDialog::exec() with safer QDialog::show(). + const auto retval = static_cast(confirmationDialog->exec()); // cancel sign&broadcast if user doesn't want to bump the fee if (retval != QMessageBox::Yes) { From f3a17bbe5f7d23b6ecc20e363920492b50859dad Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 7 Jun 2021 18:45:04 +0300 Subject: [PATCH 09/10] qt: Do not exit and re-enter main event loop during shutdown --- src/qt/bitcoin.cpp | 14 +++++++++----- src/qt/bitcoin.h | 4 ++-- src/qt/bitcoingui.cpp | 7 ++++--- src/qt/bitcoingui.h | 1 + src/qt/optionsdialog.cpp | 3 ++- src/qt/optionsdialog.h | 1 + 6 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index f6ea147ddba..8ae6db831fc 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -54,6 +54,7 @@ #include #include #include +#include #if defined(QT_STATICPLUGIN) #include @@ -258,6 +259,7 @@ void BitcoinApplication::createOptionsModel(bool resetSettings) void BitcoinApplication::createWindow(const NetworkStyle *networkStyle) { window = new BitcoinGUI(node(), platformStyle, networkStyle, nullptr); + connect(window, &BitcoinGUI::quitRequested, this, &BitcoinApplication::requestShutdown); pollShutdownTimer = new QTimer(window); connect(pollShutdownTimer, &QTimer::timeout, window, &BitcoinGUI::detectShutdown); @@ -325,13 +327,17 @@ void BitcoinApplication::requestInitialize() void BitcoinApplication::requestShutdown() { + for (const auto w : QGuiApplication::topLevelWindows()) { + w->hide(); + } + // Show a simple window indicating shutdown status // Do this first as some of the steps may take some time below, // for example the RPC console may still be executing a command. shutdownWindow.reset(ShutdownWindow::showShutdownWindow(window)); qDebug() << __func__ << ": Requesting shutdown"; - window->hide(); + // Must disconnect node signals otherwise current thread can deadlock since // no event loop is running. window->unsubscribeFromCoreSignals(); @@ -408,13 +414,13 @@ void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHead pollShutdownTimer->start(200); } else { Q_EMIT splashFinished(); // Make sure splash screen doesn't stick around during shutdown - quit(); // Exit first main loop invocation + requestShutdown(); } } void BitcoinApplication::shutdownResult() { - quit(); // Exit second main loop invocation after shutdown finished + quit(); } void BitcoinApplication::handleRunawayException(const QString &message) @@ -637,8 +643,6 @@ int GuiMain(int argc, char* argv[]) #if defined(Q_OS_WIN) WinShutdownMonitor::registerShutdownBlockReason(QObject::tr("%1 didn't yet exit safely…").arg(PACKAGE_NAME), (HWND)app.getMainWinId()); #endif - app.exec(); - app.requestShutdown(); app.exec(); rv = app.getReturnValue(); } else { diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index ed2f26b7f38..6c2c1c9ebcb 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -56,8 +56,6 @@ class BitcoinApplication: public QApplication /// Request core initialization void requestInitialize(); - /// Request core shutdown - void requestShutdown(); /// Get process return value int getReturnValue() const { return returnValue; } @@ -73,6 +71,8 @@ class BitcoinApplication: public QApplication public Q_SLOTS: void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info); + /// Request core shutdown + void requestShutdown(); void shutdownResult(); /// Handle runaway exceptions. Shows a message box with the problem and quits the program. void handleRunawayException(const QString &message); diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 6ecd3c0f5ca..abbc6f706bb 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -371,7 +371,7 @@ void BitcoinGUI::createActions() m_mask_values_action->setStatusTip(tr("Mask the values in the Overview tab")); m_mask_values_action->setCheckable(true); - connect(quitAction, &QAction::triggered, qApp, QApplication::quit); + connect(quitAction, &QAction::triggered, this, &BitcoinGUI::quitRequested); connect(aboutAction, &QAction::triggered, this, &BitcoinGUI::aboutClicked); connect(aboutQtAction, &QAction::triggered, qApp, QApplication::aboutQt); connect(optionsAction, &QAction::triggered, this, &BitcoinGUI::optionsClicked); @@ -991,6 +991,7 @@ void BitcoinGUI::openOptionsDialogWithTab(OptionsDialog::Tab tab) return; auto dlg = new OptionsDialog(this, enableWallet); + connect(dlg, &OptionsDialog::quitOnReset, this, &BitcoinGUI::quitRequested); dlg->setCurrentTab(tab); dlg->setModel(clientModel->getOptionsModel()); GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); @@ -1216,7 +1217,7 @@ void BitcoinGUI::closeEvent(QCloseEvent *event) // close rpcConsole in case it was open to make some space for the shutdown window rpcConsole->close(); - QApplication::quit(); + Q_EMIT quitRequested(); } else { @@ -1410,7 +1411,7 @@ void BitcoinGUI::detectShutdown() { if(rpcConsole) rpcConsole->hide(); - qApp->quit(); + Q_EMIT quitRequested(); } } diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index c83cd446a07..27045f5cc37 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -214,6 +214,7 @@ class BitcoinGUI : public QMainWindow void openOptionsDialogWithTab(OptionsDialog::Tab tab); Q_SIGNALS: + void quitRequested(); /** Signal raised when a URI was entered or dragged to the GUI */ void receivedURI(const QString &uri); /** Signal raised when RPC console shown */ diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 5ad4fc9b330..0ffa4acc36b 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -290,7 +290,8 @@ void OptionsDialog::on_resetButton_clicked() /* reset all options and close GUI */ model->Reset(); - QApplication::quit(); + close(); + Q_EMIT quitOnReset(); } } diff --git a/src/qt/optionsdialog.h b/src/qt/optionsdialog.h index ba35ff3b67e..f14aec3449b 100644 --- a/src/qt/optionsdialog.h +++ b/src/qt/optionsdialog.h @@ -68,6 +68,7 @@ private Q_SLOTS: Q_SIGNALS: void proxyIpChecks(QValidatedLineEdit *pUiProxyIp, uint16_t nProxyPort); + void quitOnReset(); private: Ui::OptionsDialog *ui; From 451ca244db8bc71ffc3cc9982d025f144cc8f3bc Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 7 Jun 2021 18:50:52 +0300 Subject: [PATCH 10/10] qt, refactor: Drop intermediate BitcoinApplication::shutdownResult slot --- src/qt/bitcoin.cpp | 7 +------ src/qt/bitcoin.h | 1 - 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 8ae6db831fc..e51c7d46f62 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -297,7 +297,7 @@ void BitcoinApplication::startThread() /* communication to and from thread */ connect(&m_executor.value(), &InitExecutor::initializeResult, this, &BitcoinApplication::initializeResult); - connect(&m_executor.value(), &InitExecutor::shutdownResult, this, &BitcoinApplication::shutdownResult); + connect(&m_executor.value(), &InitExecutor::shutdownResult, this, &QCoreApplication::quit); connect(&m_executor.value(), &InitExecutor::runawayException, this, &BitcoinApplication::handleRunawayException); connect(this, &BitcoinApplication::requestedInitialize, &m_executor.value(), &InitExecutor::initialize); connect(this, &BitcoinApplication::requestedShutdown, &m_executor.value(), &InitExecutor::shutdown); @@ -418,11 +418,6 @@ void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHead } } -void BitcoinApplication::shutdownResult() -{ - quit(); -} - void BitcoinApplication::handleRunawayException(const QString &message) { QMessageBox::critical( diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index 6c2c1c9ebcb..116e9415019 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -73,7 +73,6 @@ public Q_SLOTS: void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info); /// Request core shutdown void requestShutdown(); - void shutdownResult(); /// Handle runaway exceptions. Shows a message box with the problem and quits the program. void handleRunawayException(const QString &message);