Merge #2564: [GUI] Clean restart process

a2de5cd Refactor: unify PrepareShutdown() and Shutdown() (random-zebra)
d3d8e1e GUI: force dir locks release when the restart process is triggered. (furszy)
18bb0a4 GUI: Unify shutdown and restart function. (furszy)
3fa4bea shutdown: print error if unable to remove the PID file. (furszy)
5cd43a5 pivx.cpp use nullptr instead of 0. (furszy)
6e900ad Remove unneeded restart variables (furszy)

Pull request description:

  Cleaning code redundancies from the "restart with args" process.

ACKs for top commit:
  random-zebra:
    ACK a2de5cd
  Fuzzbawls:
    ACK a2de5cd

Tree-SHA512: 29af2ba5709430d7dda7f97d826a03354e4d1a1ad3f3570fa6157c7369dd017598961baad10c7dca9383246df8584f2915396af58553af5448048e17784c33ac

src/init.cpp

@@ -87,7 +87,6 @@
 
 
 volatile bool fFeeEstimatesInitialized = false;
-volatile bool fRestartRequested = false; // true: restart false: shutdown
 static const bool DEFAULT_PROXYRANDOMIZE = true;
 static const bool DEFAULT_REST_ENABLE = false;
 static const bool DEFAULT_DISABLE_SAFEMODE = false;
@@ -160,7 +159,7 @@ void StartShutdown()
 }
 bool ShutdownRequested()
 {
-    return fRequestShutdown || fRestartRequested;
+    return fRequestShutdown;
 }
 
 class CCoinsViewErrorCatcher : public CCoinsViewBacked
@@ -201,11 +200,9 @@ void Interrupt()
         g_connman->Interrupt();
 }
 
-/** Preparing steps before shutting down or restarting the wallet */
-void PrepareShutdown()
+void Shutdown()
 {
     fRequestShutdown = true;  // Needed when we shutdown the wallet
-    fRestartRequested = true; // Needed when we restart the wallet
     LogPrintf("%s: In progress...\n", __func__);
     static RecursiveMutex cs_Shutdown;
     TRY_LOCK(cs_Shutdown, lockShutdown);
@@ -330,28 +327,14 @@ void PrepareShutdown()
 
 #ifndef WIN32
     try {
-        fs::remove(GetPidFile());
+        if (!fs::remove(GetPidFile())) {
+            LogPrintf("%s: Unable to remove PID file: File does not exist\n", __func__);
+        }
     } catch (const fs::filesystem_error& e) {
         LogPrintf("%s: Unable to remove pidfile: %s\n", __func__, e.what());
     }
 #endif
-}
 
-/**
-* Shutdown is split into 2 parts:
-* Part 1: shut down everything but the main wallet instance (done in PrepareShutdown() )
-* Part 2: delete wallet instance
-*
-* In case of a restart PrepareShutdown() was already called before, but this method here gets
-* called implicitly when the parent object is deleted. In this case we have to skip the
-* PrepareShutdown() part because it was already executed and just delete the wallet instance.
-*/
-void Shutdown()
-{
-    // Shutdown part 1: prepare shutdown
-    if (!fRestartRequested) {
-        PrepareShutdown();
-    }
 #ifdef ENABLE_WALLET
     for (CWalletRef pwallet : vpwallets) {
         delete pwallet;

src/init.h

@@ -22,7 +22,6 @@ bool ShutdownRequested();
 /** Interrupt threads */
 void Interrupt();
 void Shutdown();
-void PrepareShutdown();
 //!Initialize the logging infrastructure
 void InitLogging();
 //!Parameter interaction: change current parameters depending on various rules

src/qt/pivx.cpp

@@ -43,8 +43,9 @@
 #include "wallet/wallet.h"
 #endif
 
+#include <atomic>
+
 #include <QApplication>
-#include <QDebug>
 #include <QLibraryInfo>
 #include <QLocale>
 #include <QMessageBox>
@@ -163,17 +164,15 @@ class BitcoinCore : public QObject
 public Q_SLOTS:
     void initialize();
     void shutdown();
-    void restart(QStringList args);
+    bool shutdownFromThread(const QString& type = "Shutdown");
+    void restart(const QStringList& args);
 
 Q_SIGNALS:
     void initializeResult(int retval);
     void shutdownResult(int retval);
     void runawayException(const QString& message);
 
 private:
-    /// Flag indicating a restart
-    bool execute_restart{false};
-
     /// Pass fatal exception message to UI thread
     void handleRunawayException(const std::exception* e);
 };
@@ -259,8 +258,6 @@ void BitcoinCore::handleRunawayException(const std::exception* e)
 
 void BitcoinCore::initialize()
 {
-    execute_restart = true;
-
     try {
         qDebug() << __func__ << ": Running AppInit2 in thread";
         if (!AppInitBasicSetup()) {
@@ -280,56 +277,58 @@ void BitcoinCore::initialize()
     } catch (const std::exception& e) {
         handleRunawayException(&e);
     } catch (...) {
-        handleRunawayException(NULL);
+        handleRunawayException(nullptr);
     }
 }
 
-void BitcoinCore::restart(QStringList args)
+void BitcoinCore::restart(const QStringList& args)
 {
-    if (execute_restart) { // Only restart 1x, no matter how often a user clicks on a restart-button
-        execute_restart = false;
-        try {
-            qDebug() << __func__ << ": Running Restart in thread";
-            Interrupt();
-            PrepareShutdown();
-            qDebug() << __func__ << ": Shutdown finished";
-            Q_EMIT shutdownResult(1);
-            CExplicitNetCleanup::callCleanup();
-            QProcess::startDetached(QApplication::applicationFilePath(), args);
-            qDebug() << __func__ << ": Restart initiated...";
-            QApplication::quit();
-        } catch (const std::exception& e) {
-            handleRunawayException(&e);
-        } catch (...) {
-            handleRunawayException(NULL);
+    static std::atomic<bool> restartAvailable{true};
+    if (restartAvailable.exchange(false)) {
+        if (!shutdownFromThread("restart")) {
+            qDebug() << __func__ << ": Restart failed...";
+            return;
         }
+        // Forced cleanup.
+        CExplicitNetCleanup::callCleanup();
+        ReleaseDirectoryLocks();
+        QProcess::startDetached(QApplication::applicationFilePath(), args);
+        qDebug() << __func__ << ": Restart initiated...";
+        QApplication::quit();
     }
 }
 
 void BitcoinCore::shutdown()
+{
+    shutdownFromThread("Shutdown");
+}
+
+bool BitcoinCore::shutdownFromThread(const QString& type)
 {
     try {
-        qDebug() << __func__ << ": Running Shutdown in thread";
+        qDebug() << __func__ << ": Running "+type+" in thread";
         Interrupt();
         Shutdown();
-        qDebug() << __func__ << ": Shutdown finished";
+        qDebug() << __func__ << ": "+type+" finished";
         Q_EMIT shutdownResult(1);
+        return true;
     } catch (const std::exception& e) {
         handleRunawayException(&e);
     } catch (...) {
-        handleRunawayException(NULL);
+        handleRunawayException(nullptr);
     }
+    return false;
 }
 
 BitcoinApplication::BitcoinApplication(int& argc, char** argv) : QApplication(argc, argv),
-                                                                 coreThread(0),
-                                                                 optionsModel(0),
-                                                                 clientModel(0),
-                                                                 window(0),
-                                                                 pollShutdownTimer(0),
+                                                                 coreThread(nullptr),
+                                                                 optionsModel(nullptr),
+                                                                 clientModel(nullptr),
+                                                                 window(nullptr),
+                                                                 pollShutdownTimer(nullptr),
 #ifdef ENABLE_WALLET
-                                                                 paymentServer(0),
-                                                                 walletModel(0),
+                                                                 paymentServer(nullptr),
+                                                                 walletModel(nullptr),
 #endif
                                                                  returnValue(0)
 {
@@ -346,10 +345,10 @@ BitcoinApplication::~BitcoinApplication()
     }
 
     delete window;
-    window = 0;
+    window = nullptr;
 #ifdef ENABLE_WALLET
     delete paymentServer;
-    paymentServer = 0;
+    paymentServer = nullptr;
 #endif
     // Delete Qt-settings if user clicked on "Reset Options"
     QSettings settings;
@@ -358,7 +357,7 @@ BitcoinApplication::~BitcoinApplication()
         settings.sync();
     }
     delete optionsModel;
-    optionsModel = 0;
+    optionsModel = nullptr;
 }
 
 #ifdef ENABLE_WALLET
@@ -375,7 +374,7 @@ void BitcoinApplication::createOptionsModel()
 
 void BitcoinApplication::createWindow(const NetworkStyle* networkStyle)
 {
-    window = new PIVXGUI(networkStyle, 0);
+    window = new PIVXGUI(networkStyle, nullptr);
 
     pollShutdownTimer = new QTimer(window);
     connect(pollShutdownTimer, &QTimer::timeout, window, &PIVXGUI::detectShutdown);
@@ -541,7 +540,7 @@ void BitcoinApplication::shutdownResult(int retval)
 
 void BitcoinApplication::handleRunawayException(const QString& message)
 {
-    QMessageBox::critical(0, "Runaway exception", QObject::tr("A fatal error occurred. PIVX can no longer continue safely and will quit.") + QString("\n\n") + message);
+    QMessageBox::critical(nullptr, "Runaway exception", QObject::tr("A fatal error occurred. PIVX can no longer continue safely and will quit.") + QString("\n\n") + message);
     ::exit(1);
 }