aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorW. J. van der Laan <laanwj@protonmail.com>2021-09-30 10:26:37 +0200
committerW. J. van der Laan <laanwj@protonmail.com>2021-09-30 11:35:15 +0200
commit81e7748bc18aef167a6aed07c96d85bf00c5929e (patch)
tree3ee7782471f7f7cb8730b6dcbd71c1ced6085ae4
parentf52929063f4811d460b40c80da309aa6c40a9410 (diff)
parent451ca244db8bc71ffc3cc9982d025f144cc8f3bc (diff)
Merge bitcoin-core/gui#336: Do not exit and re-enter main event loop during shutdown
451ca244db8bc71ffc3cc9982d025f144cc8f3bc qt, refactor: Drop intermediate BitcoinApplication::shutdownResult slot (Hennadii Stepanov) f3a17bbe5f7d23b6ecc20e363920492b50859dad qt: Do not exit and re-enter main event loop during shutdown (Hennadii Stepanov) b4e0d2c43181ad97c15b252e95181e2c3f6c1d2a qt, refactor: Allocate SendConfirmationDialog instances on heap (Hennadii Stepanov) 332dea2852d9c68f900ed1f0be99b6cea79c7457 qt, refactor: Keep HelpMessageDialog in the main event loop (Hennadii Stepanov) c8bae37a7a646badf8e79669bf06ac174e13cd6f qt, refactor: Keep PSBTOperationsDialog in the main event loop (Hennadii Stepanov) 7fa91e831227e556bd8a7ae3da64bd59d4f30d5f qt, refactor: Keep AskPassphraseDialog in the main event loop (Hennadii Stepanov) 6f6fde30e7601185a8f6052b3bf1770407fcc14b qt, refactor: Keep EditAddressDialog in the main event loop (Hennadii Stepanov) 59f7ba4fd7a9e4bc73d784ee74d5b777da9cc436 qt, refactor: Keep CoinControlDialog in the main event loop (Hennadii Stepanov) 7830cd0b35f315570d744f4d2719104c08b33ff1 qt, refactor: Keep OptionsDialog in the main event loop (Hennadii Stepanov) 13f618818dc57673ac0287ad8b28ceb450efb374 qt: Add GUIUtil::ShowModalDialogAndDeleteOnClose (Hennadii Stepanov) Pull request description: On master (1ef34ee25ed34b2b092f15bf3dca5c0508092829) during shutdown `QApplication` exits the main event loop, then re-enter again. This PR streamlines shutdown process by removing the need to interrupt the main event loop, that is required for #59. Also, blocking [`QDialog::exec()`](https://doc.qt.io/qt-5/qdialog.html#exec) calls are replaced with safer [`QDialog::show()`](https://doc.qt.io/qt-5/qwidget.html#show), except for `SendConfirmationDialog` as that change is not trivial (marked as TODO). The [`QDialog::open()`](https://doc.qt.io/qt-5/qdialog.html#open) was not used because the actual modality mode (application modal or window modal) of a dialog depends on whether it has a parent. This PR does not change behavior, and all touched dialogs are still application modal. As a follow up, a design research could suggest to make some dialogs window modal. NOTE for reviewers: quitting app while a dialog is open (e.g., via systray icon menu) must work fine. ACKs for top commit: laanwj: Code review and lighly tested ACK 451ca244db8bc71ffc3cc9982d025f144cc8f3bc promag: ACK 451ca244db8bc71ffc3cc9982d025f144cc8f3bc, just changed signal to `quitRequested`. Tree-SHA512: ef01ab6ed803b202e776019a4e1f592e816f7bc786e00574b25a0bf16be2374ddf9db21f0a26da08700df7ef0ab9e879550df46dcfe3b6d940f5ed02ca5f8447
-rw-r--r--src/qt/addressbookpage.cpp8
-rw-r--r--src/qt/bitcoin.cpp19
-rw-r--r--src/qt/bitcoin.h5
-rw-r--r--src/qt/bitcoingui.cpp19
-rw-r--r--src/qt/bitcoingui.h1
-rw-r--r--src/qt/guiutil.cpp8
-rw-r--r--src/qt/guiutil.h6
-rw-r--r--src/qt/optionsdialog.cpp3
-rw-r--r--src/qt/optionsdialog.h1
-rw-r--r--src/qt/sendcoinsdialog.cpp13
-rw-r--r--src/qt/transactionview.cpp16
-rw-r--r--src/qt/walletframe.cpp5
-rw-r--r--src/qt/walletmodel.cpp7
-rw-r--r--src/qt/walletview.cpp24
14 files changed, 75 insertions, 60 deletions
diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp
index e78594390b..a617bb4451 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/bitcoin.cpp b/src/qt/bitcoin.cpp
index d4895ea6ff..00c9fd3059 100644
--- a/src/qt/bitcoin.cpp
+++ b/src/qt/bitcoin.cpp
@@ -55,6 +55,7 @@
#include <QThread>
#include <QTimer>
#include <QTranslator>
+#include <QWindow>
#if defined(QT_STATICPLUGIN)
#include <QtPlugin>
@@ -259,6 +260,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);
@@ -296,7 +298,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);
@@ -326,13 +328,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();
@@ -409,15 +415,10 @@ 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
-}
-
void BitcoinApplication::handleRunawayException(const QString &message)
{
QMessageBox::critical(
@@ -640,8 +641,6 @@ int GuiMain(int argc, char* argv[])
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 {
// A dialog with detailed error will have been shown by InitError()
diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h
index 602b76052c..5678ca90d2 100644
--- a/src/qt/bitcoin.h
+++ b/src/qt/bitcoin.h
@@ -61,8 +61,6 @@ public:
/// Request core initialization
void requestInitialize();
- /// Request core shutdown
- void requestShutdown();
/// Get process return value
int getReturnValue() const { return returnValue; }
@@ -77,7 +75,8 @@ public:
public Q_SLOTS:
void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info);
- void shutdownResult();
+ /// Request core shutdown
+ void requestShutdown();
/// 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 2ffb6e650b..af05f35759 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -373,7 +373,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);
@@ -850,8 +850,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()
@@ -992,10 +992,11 @@ 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);
+ connect(dlg, &OptionsDialog::quitOnReset, this, &BitcoinGUI::quitRequested);
+ 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)
@@ -1218,7 +1219,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
{
@@ -1412,7 +1413,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 c83cd446a0..27045f5cc3 100644
--- a/src/qt/bitcoingui.h
+++ b/src/qt/bitcoingui.h
@@ -214,6 +214,7 @@ private:
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/guiutil.cpp b/src/qt/guiutil.cpp
index 91f2591a31..7b1384b485 100644
--- a/src/qt/guiutil.cpp
+++ b/src/qt/guiutil.cpp
@@ -36,6 +36,7 @@
#include <QClipboard>
#include <QDateTime>
#include <QDesktopServices>
+#include <QDialog>
#include <QDoubleValidator>
#include <QFileDialog>
#include <QFont>
@@ -970,4 +971,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 06a3b63668..274f0bdcbf 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
diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp
index ac50e6518f..0cc2d61df6 100644
--- a/src/qt/optionsdialog.cpp
+++ b/src/qt/optionsdialog.cpp
@@ -292,7 +292,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 ba35ff3b67..f14aec3449 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;
diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
index ff53d8160f..2718392940 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<QMessageBox::StandardButton>(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<QMessageBox::StandardButton>(confirmationDialog->exec());
if(retval != QMessageBox::Yes)
{
@@ -914,9 +915,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
diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp
index 1973c9de9a..653f3dda6d 100644
--- a/src/qt/transactionview.cpp
+++ b/src/qt/transactionview.cpp
@@ -504,22 +504,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);
}
}
}
diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp
index 5eeb2d5308..4ff92bf82c 100644
--- a/src/qt/walletframe.cpp
+++ b/src/qt/walletframe.cpp
@@ -221,10 +221,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()
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 967dd588b4..052453cf65 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<QMessageBox::StandardButton>(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<QMessageBox::StandardButton>(confirmationDialog->exec());
// cancel sign&broadcast if user doesn't want to bump the fee
if (retval != QMessageBox::Yes) {
diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp
index 309806a1c4..7813b89e41 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);
}
}