Merge bitcoin-core/gui#381: refactor: Make BitcoinCore class reusable

8169fc4 qt, refactor: Fix code styling of moved InitExecutor class (Hennadii Stepanov)
c82165a qt, refactor: Move InitExecutor class into its own module (Hennadii Stepanov)
dbcf56b scripted-diff: Rename BitcoinCore class to InitExecutor (Hennadii Stepanov)
19a1d00 qt: Add BitcoinCore::m_thread member (Hennadii Stepanov)

Pull request description:

  This PR makes the `BitcoinCore` class reusable, i.e., it can be used by the widget-based GUI or by the [QML-based](https://github.com/bitcoin-core/gui-qml/tree/main/src/qml) one, and it makes the divergence between these two repos minimal.

  The small benefit to the current branch is more structured code.

  Actually, this PR is ported from bitcoin-core/gui-qml#10.
  The example of the re-using of the `BitcoinCore` class is bitcoin-core/gui-qml#11.

ACKs for top commit:
  laanwj:
    ACK 8169fc4
  ryanofsky:
    Code review ACK 8169fc4. Only change is switching from `m_executor` from pointer to optional type (thanks for update!)

Tree-SHA512: a0552c32d26d9acf42921eb12bcdf68f02d52f7183c688c43257b1a58679f64e45f193ee2d316850c7f0f516561e17abe989fe545bfa05e158ad3f4c66d19bca

src/Makefile.qt.include

@@ -52,6 +52,7 @@ QT_MOC_CPP = \
   qt/moc_editaddressdialog.cpp \
   qt/moc_governancelist.cpp \
   qt/moc_guiutil.cpp \
+  qt/moc_initexecutor.cpp \
   qt/moc_intro.cpp \
   qt/moc_macdockiconhandler.cpp \
   qt/moc_macnotificationhandler.cpp \
@@ -124,6 +125,7 @@ BITCOIN_QT_H = \
   qt/governancelist.h \
   qt/guiconstants.h \
   qt/guiutil.h \
+  qt/initexecutor.h \
   qt/intro.h \
   qt/macdockiconhandler.h \
   qt/macnotificationhandler.h \
@@ -210,6 +212,7 @@ BITCOIN_QT_BASE_CPP = \
   qt/clientmodel.cpp \
   qt/csvmodelwriter.cpp \
   qt/guiutil.cpp \
+  qt/initexecutor.cpp \
   qt/intro.cpp \
   qt/modaloverlay.cpp \
   qt/networkstyle.cpp \

src/qt/bitcoin.cpp

@@ -8,37 +8,40 @@
 #endif
 
 #include <qt/bitcoin.h>
-#include <qt/bitcoingui.h>
 
 #include <chainparams.h>
 #include <fs.h>
+#include <init.h>
+#include <interfaces/handler.h>
+#include <interfaces/node.h>
+#include <node/context.h>
+#include <node/ui_interface.h>
+#include <noui.h>
+#include <qt/bitcoingui.h>
 #include <qt/clientmodel.h>
 #include <qt/guiconstants.h>
 #include <qt/guiutil.h>
+#include <qt/initexecutor.h>
 #include <qt/intro.h>
 #include <net.h>
 #include <qt/networkstyle.h>
 #include <qt/optionsmodel.h>
 #include <qt/splashscreen.h>
 #include <qt/utilitydialog.h>
 #include <qt/winshutdownmonitor.h>
+#include <uint256.h>
+#include <util/system.h>
+#include <util/threadnames.h>
+#include <util/translation.h>
+#include <validation.h>
 
 #ifdef ENABLE_WALLET
 #include <qt/paymentserver.h>
 #include <qt/walletcontroller.h>
 #include <qt/walletmodel.h>
 #endif // ENABLE_WALLET
 
-#include <interfaces/handler.h>
-#include <interfaces/node.h>
-#include <node/context.h>
-#include <noui.h>
 #include <stacktraces.h>
-#include <ui_interface.h>
-#include <uint256.h>
-#include <util/system.h>
-#include <util/threadnames.h>
-#include <util/translation.h>
 
 #include <memory>
 
@@ -199,7 +202,6 @@ static const char* qt_argv = "dash-qt";
 
 BitcoinApplication::BitcoinApplication(interfaces::Node& node):
     QApplication(qt_argc, const_cast<char **>(&qt_argv)),
-    coreThread(nullptr),
     m_node(node),
     optionsModel(nullptr),
     clientModel(nullptr),
@@ -213,13 +215,7 @@ BitcoinApplication::BitcoinApplication(interfaces::Node& node):
 
 BitcoinApplication::~BitcoinApplication()
 {
-    if(coreThread)
-    {
-        qDebug() << __func__ << ": Stopping thread";
-        coreThread->quit();
-        coreThread->wait();
-        qDebug() << __func__ << ": Stopped thread";
-    }
+    m_executor.reset();
 
     delete window;
     window = nullptr;
@@ -270,23 +266,15 @@ bool BitcoinApplication::baseInitialize()
 
 void BitcoinApplication::startThread()
 {
-    if(coreThread)
-        return;
-    coreThread = new QThread(this);
-    BitcoinCore *executor = new BitcoinCore(m_node);
-    executor->moveToThread(coreThread);
+    assert(!m_executor);
+    m_executor.emplace(node());
 
     /*  communication to and from thread */
-    connect(executor, &BitcoinCore::initializeResult, this, &BitcoinApplication::initializeResult);
-    connect(executor, &BitcoinCore::shutdownResult, this, &BitcoinApplication::shutdownResult);
-    connect(executor, &BitcoinCore::runawayException, this, &BitcoinApplication::handleRunawayException);
-    connect(this, &BitcoinApplication::requestedInitialize, executor, &BitcoinCore::initialize);
-    connect(this, &BitcoinApplication::requestedShutdown, executor, &BitcoinCore::shutdown);
-    connect(window, &BitcoinGUI::requestedRestart, executor, &BitcoinCore::restart);
-    /*  make sure executor object is deleted in its own thread */
-    connect(coreThread, &QThread::finished, executor, &QObject::deleteLater);
-
-    coreThread->start();
+    connect(&m_executor.value(), &InitExecutor::initializeResult, this, &BitcoinApplication::initializeResult);
+    connect(&m_executor.value(), &InitExecutor::shutdownResult, this, &BitcoinApplication::shutdownResult);
+    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);
 }
 
 void BitcoinApplication::parameterSetup()
@@ -321,7 +309,6 @@ void BitcoinApplication::requestShutdown()
     shutdownWindow.reset(ShutdownWindow::showShutdownWindow(m_node, window));
 
     qDebug() << __func__ << ": Requesting shutdown";
-    startThread();
     window->hide();
     // Must disconnect node signals otherwise current thread can deadlock since
     // no event loop is running.

src/qt/bitcoin.h

@@ -9,10 +9,13 @@
 #include <config/bitcoin-config.h>
 #endif
 
-#include <QApplication>
+#include <interfaces/node.h>
+#include <qt/initexecutor.h>
+
 #include <memory>
+#include <optional>
 
-#include <interfaces/node.h>
+#include <QApplication>
 
 class BitcoinGUI;
 class ClientModel;
@@ -23,32 +26,6 @@ class WalletController;
 class WalletModel;
 
 
-/** Class encapsulating Bitcoin Core startup and shutdown.
- * Allows running startup and shutdown in a different thread from the UI thread.
- */
-class BitcoinCore: public QObject
-{
-    Q_OBJECT
-public:
-    explicit BitcoinCore(interfaces::Node& node);
-
-public Q_SLOTS:
-    void initialize();
-    void shutdown();
-    void restart(QStringList args);
-
-Q_SIGNALS:
-    void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info);
-    void shutdownResult();
-    void runawayException(const QString &message);
-
-private:
-    /// Pass fatal exception message to UI thread
-    void handleRunawayException(const std::exception_ptr e);
-
-    interfaces::Node& m_node;
-};
-
 /** Main Bitcoin application object */
 class BitcoinApplication: public QApplication
 {
@@ -99,7 +76,7 @@ public Q_SLOTS:
     void windowShown(BitcoinGUI* window);
 
 private:
-    QThread *coreThread;
+    std::optional<InitExecutor> m_executor;
     interfaces::Node& m_node;
     OptionsModel *optionsModel;
     ClientModel *clientModel;

src/qt/initexecutor.cpp

@@ -0,0 +1,67 @@
+// Copyright (c) 2014-2021 The Bitcoin Core developers
+// Copyright (c) 2014-2021 The Dash Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <qt/initexecutor.h>
+
+#include <interfaces/node.h>
+#include <util/system.h>
+#include <util/threadnames.h>
+
+#include <exception>
+
+#include <QDebug>
+#include <QObject>
+#include <QString>
+#include <QThread>
+
+InitExecutor::InitExecutor(interfaces::Node& node)
+    : QObject(), m_node(node)
+{
+    this->moveToThread(&m_thread);
+    m_thread.start();
+}
+
+InitExecutor::~InitExecutor()
+{
+    qDebug() << __func__ << ": Stopping thread";
+    m_thread.quit();
+    m_thread.wait();
+    qDebug() << __func__ << ": Stopped thread";
+}
+
+void InitExecutor::handleRunawayException(const std::exception* e)
+{
+    PrintExceptionContinue(e, "Runaway exception");
+    Q_EMIT runawayException(QString::fromStdString(m_node.getWarnings().translated));
+}
+
+void InitExecutor::initialize()
+{
+    try {
+        util::ThreadRename("qt-init");
+        qDebug() << __func__ << ": Running initialization in thread";
+        interfaces::BlockAndHeaderTipInfo tip_info;
+        bool rv = m_node.appInitMain(&tip_info);
+        Q_EMIT initializeResult(rv, tip_info);
+    } catch (const std::exception& e) {
+        handleRunawayException(&e);
+    } catch (...) {
+        handleRunawayException(nullptr);
+    }
+}
+
+void InitExecutor::shutdown()
+{
+    try {
+        qDebug() << __func__ << ": Running Shutdown in thread";
+        m_node.appShutdown();
+        qDebug() << __func__ << ": Shutdown finished";
+        Q_EMIT shutdownResult();
+    } catch (const std::exception& e) {
+        handleRunawayException(&e);
+    } catch (...) {
+        handleRunawayException(nullptr);
+    }
+}

src/qt/initexecutor.h

@@ -0,0 +1,47 @@
+// Copyright (c) 2014-2021 The Bitcoin Core developers
+// Copyright (c) 2014-2021 The Dash Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef DASH_QT_INITEXECUTOR_H
+#define DASH_QT_INITEXECUTOR_H
+
+#include <interfaces/node.h>
+
+#include <exception>
+
+#include <QObject>
+#include <QThread>
+
+QT_BEGIN_NAMESPACE
+class QString;
+QT_END_NAMESPACE
+
+/** Class encapsulating Dash Core startup and shutdown.
+ * Allows running startup and shutdown in a different thread from the UI thread.
+ */
+class InitExecutor : public QObject
+{
+    Q_OBJECT
+public:
+    explicit InitExecutor(interfaces::Node& node);
+    ~InitExecutor();
+
+public Q_SLOTS:
+    void initialize();
+    void shutdown();
+
+Q_SIGNALS:
+    void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info);
+    void shutdownResult();
+    void runawayException(const QString& message);
+
+private:
+    /// Pass fatal exception message to UI thread
+    void handleRunawayException(const std::exception* e);
+
+    interfaces::Node& m_node;
+    QThread m_thread;
+};
+
+#endif // DASH_QT_INITEXECUTOR_H

src/qt/test/test_main.cpp

@@ -9,6 +9,7 @@
 
 #include <interfaces/node.h>
 #include <qt/bitcoin.h>
+#include <qt/initexecutor.h>
 #include <qt/test/apptests.h>
 #include <qt/test/rpcnestedtests.h>
 #include <qt/test/uritests.h>