Merge bitcoin-core/gui#751: macOS, do not process actions during shutdown

bae209e3879fa099302d3b211362c49bbbfbdd14 gui: macOS, make appMenuBar part of the main app window (furszy)
e14cc8fc69cb3e3a98076fbb23a94eba7873368a gui: macOS, do not process dock icon actions during shutdown (furszy)

Pull request description:

  As the 'QMenuBar' is created without a parent window in MacOS, the app crashes when the user presses the shutdown button and, right after it, triggers any action in the menu bar.

  This happens because the QMenuBar is manually deleted in the BitcoinGUI destructor but the events attached to it children actions are not disconnected, so QActions events such us the 'QMenu::aboutToShow' could try to access null pointers.

  Instead of guarding every single QAction pointer inside the QMenu::aboutToShow slot, or manually disconnecting all registered events in the destructor, we can check if a shutdown was requested and discard the event.

  The 'node' field is a ref whose memory is held by the main application class, so it is safe to use here. Events are disconnected prior destructing the main application object.

  Furthermore, the 'MacDockIconHandler::dockIconClicked' signal can make the app crash during shutdown for the very same reason. The 'show()' call triggers the 'QApplication::focusWindowChanged' event, which is connected to the 'minimize_action' QAction, which is also part of the app menu bar, which could no longer exist.

  Another cause of crashes stems from the shortcuts provided by the `appMenuBar` submenus during shutdown. For instance, executing actions like opening the information dialog (command + I) or the console dialog (command + T) lead to access null pointers. The second commit addresses and resolves these issues.
  Basically, in the present setup, we create a parentless `appMenuBar` whose submenus `QActions` are connected to `qApp` events (the app's global instance). However, at the `BitcoinGUI` destructor, we manually destruct this object without properly disconnecting the events. This leaves `qApp` events, such as `focusWindowChanged`, tied to submenus' `QAction` pointers, which causes the application to crash when it attempts to access them.

  Important Note:
  This happened to me few times. The worst consequence was an inconsistent chain state during IBD. Which triggered a full "replay blocks" process on the next startup. Which was painfully slow.

ACKs for top commit:
  RandyMcMillan:
    utACK bae209e
  hebasto:
    ACK bae209e3879fa099302d3b211362c49bbbfbdd14.

Tree-SHA512: 432e19c5f7e02c3165b7e7bd7f96f2a902bae5b5e439c2594db1c69d74ab6e0d4509d90f02db8c076f616e567e6a07492ede416ef651b5f749637398291b92fd

1 files changed, 3 insertions, 7 deletions

diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
index 2862dddb56..8a46d46437 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -239,7 +239,6 @@ BitcoinGUI::~BitcoinGUI()
         trayIcon->hide();
 #ifdef Q_OS_MACOS
     delete m_app_nap_inhibitor;
-    delete appMenuBar;
     MacDockIconHandler::cleanup();
 #endif
 
@@ -479,13 +478,7 @@ void BitcoinGUI::createActions()
 
 void BitcoinGUI::createMenuBar()
 {
-#ifdef Q_OS_MACOS
-    // Create a decoupled menu bar on Mac which stays even if the window is closed
-    appMenuBar = new QMenuBar();
-#else
-    // Get the main window's menu bar on other platforms
     appMenuBar = menuBar();
-#endif
 
     // Configure the menus
     QMenu *file = appMenuBar->addMenu(tr("&File"));
@@ -876,6 +869,7 @@ void BitcoinGUI::createTrayIconMenu()
     // Note: On macOS, the Dock icon is used to provide the tray's functionality.
     MacDockIconHandler* dockIconHandler = MacDockIconHandler::instance();
     connect(dockIconHandler, &MacDockIconHandler::dockIconClicked, [this] {
+        if (m_node.shutdownRequested()) return; // nothing to show, node is shutting down.
         show();
         activateWindow();
     });
@@ -887,6 +881,8 @@ void BitcoinGUI::createTrayIconMenu()
         // See https://bugreports.qt.io/browse/QTBUG-91697
         trayIconMenu.get(), &QMenu::aboutToShow,
         [this, show_hide_action, send_action, receive_action, sign_action, verify_action, options_action, node_window_action, quit_action] {
+            if (m_node.shutdownRequested()) return; // nothing to do, node is shutting down.
+
             if (show_hide_action) show_hide_action->setText(
                 (!isHidden() && !isMinimized() && !GUIUtil::isObscured(this)) ?
                     tr("&Hide") :