Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not exit and re-enter main event loop during shutdown #336

Merged
merged 10 commits into from
Sep 30, 2021
8 changes: 4 additions & 4 deletions src/qt/addressbookpage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
19 changes: 9 additions & 10 deletions src/qt/bitcoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
#include <QThread>
#include <QTimer>
#include <QTranslator>
#include <QWindow>

#if defined(QT_STATICPLUGIN)
#include <QtPlugin>
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -295,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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -408,15 +414,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(
Expand Down Expand Up @@ -637,8 +638,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 {
Expand Down
5 changes: 2 additions & 3 deletions src/qt/bitcoin.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -73,7 +71,8 @@ class BitcoinApplication: public QApplication

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);

Expand Down
19 changes: 10 additions & 9 deletions src/qt/bitcoingui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -990,10 +990,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)
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -1410,7 +1411,7 @@ void BitcoinGUI::detectShutdown()
{
if(rpcConsole)
rpcConsole->hide();
qApp->quit();
Q_EMIT quitRequested();
}
}

Expand Down
1 change: 1 addition & 0 deletions src/qt/bitcoingui.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
8 changes: 8 additions & 0 deletions src/qt/guiutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <QClipboard>
#include <QDateTime>
#include <QDesktopServices>
#include <QDialog>
#include <QDoubleValidator>
#include <QFileDialog>
#include <QFont>
Expand Down Expand Up @@ -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
6 changes: 6 additions & 0 deletions src/qt/guiutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class QAbstractButton;
class QAbstractItemView;
class QAction;
class QDateTime;
class QDialog;
class QFont;
class QKeySequence;
class QLineEdit;
Expand Down Expand Up @@ -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
3 changes: 2 additions & 1 deletion src/qt/optionsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,8 @@ void OptionsDialog::on_resetButton_clicked()

/* reset all options and close GUI */
model->Reset();
QApplication::quit();
close();
Q_EMIT quitOnReset();
}
}

Expand Down
1 change: 1 addition & 0 deletions src/qt/optionsdialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ private Q_SLOTS:

Q_SIGNALS:
void proxyIpChecks(QValidatedLineEdit *pUiProxyIp, uint16_t nProxyPort);
void quitOnReset();

private:
Ui::OptionsDialog *ui;
Expand Down
13 changes: 7 additions & 6 deletions src/qt/sendcoinsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions src/qt/transactionview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/qt/walletframe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
7 changes: 4 additions & 3 deletions src/qt/walletmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
24 changes: 11 additions & 13 deletions src/qt/walletview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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);
}
}

Expand Down