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 block GUI thread in RPCConsole #59

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions src/qt/bitcoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,8 @@ void BitcoinApplication::createOptionsModel(bool resetSettings)
void BitcoinApplication::createWindow(const NetworkStyle *networkStyle)
{
window = new BitcoinGUI(node(), platformStyle, networkStyle, nullptr);
connect(window, &BitcoinGUI::quitClicked, this, &BitcoinApplication::requestShutdown);
connect(window, &BitcoinGUI::rpcExecutorThreadFinished, this, &BitcoinApplication::requestNodeShutdown);

pollShutdownTimer = new QTimer(window);
connect(pollShutdownTimer, &QTimer::timeout, window, &BitcoinGUI::detectShutdown);
Expand Down Expand Up @@ -338,9 +340,7 @@ void BitcoinApplication::requestShutdown()
// for example the RPC console may still be executing a command.
shutdownWindow.reset(ShutdownWindow::showShutdownWindow(window));

qDebug() << __func__ << ": Requesting shutdown";
startThread();
window->hide();
window->hideAll();
// Must disconnect node signals otherwise current thread can deadlock since
// no event loop is running.
window->unsubscribeFromCoreSignals();
Expand All @@ -354,9 +354,6 @@ void BitcoinApplication::requestShutdown()

delete clientModel;
clientModel = nullptr;

// Request shutdown from core thread
Q_EMIT requestedShutdown();
}

void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info)
Expand Down Expand Up @@ -406,13 +403,21 @@ 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
requestNodeShutdown();
}
}

void BitcoinApplication::requestNodeShutdown()
{
qDebug() << __func__ << ": Requesting shutdown";
startThread();
// Request shutdown from core thread
Q_EMIT requestedShutdown();
}

void BitcoinApplication::shutdownResult()
{
quit(); // Exit second main loop invocation after shutdown finished
quit();
}

void BitcoinApplication::handleRunawayException(const QString &message)
Expand Down Expand Up @@ -616,8 +621,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: 3 additions & 2 deletions src/qt/bitcoin.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,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 @@ -95,6 +93,9 @@ class BitcoinApplication: public QApplication

public Q_SLOTS:
void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info);
/// Request core shutdown
void requestShutdown();
void requestNodeShutdown();
void shutdownResult();
/// Handle runaway exceptions. Shows a message box with the problem and quits the program.
void handleRunawayException(const QString &message);
Expand Down
31 changes: 14 additions & 17 deletions src/qt/bitcoingui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty
updateWindowTitle();

rpcConsole = new RPCConsole(node, _platformStyle, nullptr);
connect(rpcConsole, &RPCConsole::executorThreadFinished, this, &BitcoinGUI::rpcExecutorThreadFinished);

helpMessageDialog = new HelpMessageDialog(this, false);
#ifdef ENABLE_WALLET
if(enableWallet)
Expand Down Expand Up @@ -228,8 +230,6 @@ BitcoinGUI::~BitcoinGUI()

QSettings settings;
settings.setValue("MainWindowGeometry", saveGeometry());
if(trayIcon) // Hide tray icon, as deleting will let it linger until quit (on Ubuntu)
trayIcon->hide();
#ifdef Q_OS_MAC
delete m_app_nap_inhibitor;
delete appMenuBar;
Expand Down Expand Up @@ -369,15 +369,13 @@ 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::quitClicked);
connect(aboutAction, &QAction::triggered, this, &BitcoinGUI::aboutClicked);
connect(aboutQtAction, &QAction::triggered, qApp, QApplication::aboutQt);
connect(optionsAction, &QAction::triggered, this, &BitcoinGUI::optionsClicked);
connect(toggleHideAction, &QAction::triggered, this, &BitcoinGUI::toggleHidden);
connect(showHelpMessageAction, &QAction::triggered, this, &BitcoinGUI::showHelpMessageClicked);
connect(openRPCConsoleAction, &QAction::triggered, this, &BitcoinGUI::showDebugWindow);
// prevents an open debug window from becoming stuck/unusable on client shutdown
connect(quitAction, &QAction::triggered, rpcConsole, &QWidget::hide);

#ifdef ENABLE_WALLET
if(walletFrame)
Expand Down Expand Up @@ -625,11 +623,6 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel, interfaces::BlockAndH
} else {
// Disable possibility to show main window via action
toggleHideAction->setEnabled(false);
if(trayIconMenu)
{
// Disable context menu on tray icon
trayIconMenu->clear();
}
// Propagate cleared model to child objects
rpcConsole->setClientModel(nullptr);
#ifdef ENABLE_WALLET
Expand Down Expand Up @@ -955,6 +948,7 @@ void BitcoinGUI::openOptionsDialogWithTab(OptionsDialog::Tab tab)
OptionsDialog dlg(this, enableWallet);
dlg.setCurrentTab(tab);
dlg.setModel(clientModel->getOptionsModel());
connect(&dlg, &OptionsDialog::quitOnReset, this, &BitcoinGUI::quitClicked);
dlg.exec();
}

Expand Down Expand Up @@ -1167,10 +1161,7 @@ void BitcoinGUI::closeEvent(QCloseEvent *event)
{
if(!clientModel->getOptionsModel()->getMinimizeOnClose())
{
// close rpcConsole in case it was open to make some space for the shutdown window
rpcConsole->close();

QApplication::quit();
Q_EMIT quitClicked();
}
else
{
Expand Down Expand Up @@ -1363,9 +1354,7 @@ void BitcoinGUI::detectShutdown()
{
if (m_node.shutdownRequested())
{
if(rpcConsole)
rpcConsole->hide();
qApp->quit();
Q_EMIT quitClicked();
}
}

Expand Down Expand Up @@ -1395,6 +1384,14 @@ void BitcoinGUI::showModalOverlay()
modalOverlay->toggleVisibility();
}

void BitcoinGUI::hideAll()
{
trayIconMenu->clear();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why clear here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trayIconMenu contains some QAction instances. I think we do not need any chance to activate any of them during shutdown.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about trayIconMenu->setEnabled(false) instead?

trayIcon->hide();
rpcConsole->hide();
hide();
}

static bool ThreadSafeMessageBox(BitcoinGUI* gui, const bilingual_str& message, const std::string& caption, unsigned int style)
{
bool modal = (style & CClientUIInterface::MODAL);
Expand Down
5 changes: 5 additions & 0 deletions src/qt/bitcoingui.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ class BitcoinGUI : public QMainWindow
void openOptionsDialogWithTab(OptionsDialog::Tab tab);

Q_SIGNALS:
void quitClicked();
void rpcExecutorThreadFinished();
/** 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 Expand Up @@ -319,6 +321,9 @@ public Q_SLOTS:
void showProgress(const QString &title, int nProgress);

void showModalOverlay();

/** Hide all windows and tray icon. */
void hideAll();
};

class UnitDisplayStatusBarControl : public QLabel
Expand Down
3 changes: 2 additions & 1 deletion src/qt/optionsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,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
2 changes: 1 addition & 1 deletion src/qt/rpcconsole.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -707,8 +707,8 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_
}
if (!model) {
// Client model is being set to 0, this means shutdown() is about to be called.
connect(&thread, &QThread::finished, this, &RPCConsole::executorThreadFinished);
thread.quit();
thread.wait();
}
}

Expand Down
1 change: 1 addition & 0 deletions src/qt/rpcconsole.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ public Q_SLOTS:
Q_SIGNALS:
// For RPC command executor
void cmdRequest(const QString &command, const WalletModel* wallet_model);
void executorThreadFinished();

private:
struct TranslatedStrings {
Expand Down