gui: Fix shutdown order

Summary:
 * gui: Fix WalletController deletion

The wallet controller instanced must be deleted after the window instance
since it is used there.

 * gui: Expose BitcoinGUI::unsubscribeFromCoreSignals

Move only change that makes unsubscribeFromCoreSignals public. It must be
called if the event loop is not running otherwise core signals handlers
can deadlock.

 * gui: Fix m_node.startShutdown() order

This change forwards the shutdown request on the GUI (close the
application for instace) to the node as soon as possible. This way the
GUI doesn't have to wait for long operations to complete (rescan the
wallet for instance), instead those operations detect the shutdown
request and abort/interrupt.

 * Check m_internals in UnregisterValidationInterface

When a wallet is created it is registered in the validation interface (in
CWallet::CreateWalletFromFile) but it is not immediately added to the
wallets list. If a shutdown is requested before AddWallet (case more
evident when -rescan is set) then m_internals can be released (in
Shutdown -> UnregisterBackgroundSignalScheduler) before the wallet and
then ReleaseWallet would call UnregisterValidationInterface with
m_internals already released.

This is a backport of Core [[bitcoin/bitcoin#15280 | PR15280]]

Test Plan:
  ninja all check-all

Run bitcoin-qt on win64 and verify it shuts down properly.

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D6344

src/qt/bitcoin.cpp

@@ -217,6 +217,8 @@ BitcoinApplication::~BitcoinApplication() {
 #ifdef ENABLE_WALLET
     delete paymentServer;
     paymentServer = nullptr;
+    delete m_wallet_controller;
+    m_wallet_controller = nullptr;
 #endif
     delete optionsModel;
     optionsModel = nullptr;
@@ -331,18 +333,20 @@ void BitcoinApplication::requestShutdown(Config &config) {
     qDebug() << __func__ << ": Requesting shutdown";
     startThread();
     window->hide();
+    // Must disconnect node signals otherwise current thread can deadlock since
+    // no event loop is running.
+    window->unsubscribeFromCoreSignals();
+    // Request node shutdown, which can interrupt long operations, like
+    // rescanning a wallet.
+    m_node.startShutdown();
+    // Unsetting the client model can cause the current thread to wait for node
+    // to complete an operation, like wait for a RPC execution to complate.
     window->setClientModel(nullptr);
     pollShutdownTimer->stop();
 
-#ifdef ENABLE_WALLET
-    delete m_wallet_controller;
-    m_wallet_controller = nullptr;
-#endif
     delete clientModel;
     clientModel = nullptr;
 
-    m_node.startShutdown();
-
     // Request shutdown from core thread
     Q_EMIT requestedShutdown();
 }

src/qt/bitcoingui.h

@@ -94,6 +94,9 @@ class BitcoinGUI : public QMainWindow {
 #endif // ENABLE_WALLET
     bool enableWallet = false;
 
+    /** Disconnect core signals from GUI client */
+    void unsubscribeFromCoreSignals();
+
 protected:
     void changeEvent(QEvent *e) override;
     void closeEvent(QCloseEvent *event) override;
@@ -183,8 +186,6 @@ class BitcoinGUI : public QMainWindow {
 
     /** Connect core signals to GUI client */
     void subscribeToCoreSignals();
-    /** Disconnect core signals from GUI client */
-    void unsubscribeFromCoreSignals();
 
     /** Update UI with latest network info from model. */
     void updateNetworkState();

src/validationinterface.cpp

@@ -11,8 +11,8 @@
 #include <atomic>
 #include <future>
 #include <list>
-#include <utility>
 #include <tuple>
+#include <utility>
 
 #include <boost/signals2/signal.hpp>
 
@@ -140,7 +140,9 @@ void RegisterValidationInterface(CValidationInterface *pwalletIn) {
 }
 
 void UnregisterValidationInterface(CValidationInterface *pwalletIn) {
-    g_signals.m_internals->m_connMainSignals.erase(pwalletIn);
+    if (g_signals.m_internals) {
+        g_signals.m_internals->m_connMainSignals.erase(pwalletIn);
+    }
 }
 
 void UnregisterAllValidationInterfaces() {