From e14cc8fc69cb3e3a98076fbb23a94eba7873368a Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 8 Sep 2023 10:58:37 -0300 Subject: gui: macOS, do not process dock icon actions during shutdown 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. --- src/qt/bitcoingui.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/qt/bitcoingui.cpp') diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index b84cd02bda..73e09630de 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -864,6 +864,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(); }); @@ -875,6 +876,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") : -- cgit v1.2.3 From bae209e3879fa099302d3b211362c49bbbfbdd14 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 12 Sep 2023 11:15:41 -0300 Subject: gui: macOS, make appMenuBar part of the main app window By moving the appMenuBar destruction responsibility to the QT framework, we ensure the disconnection of the submenus signals prior to the destruction of the main app window. The standalone menu bar may have served a purpose in earlier versions when it didn't contain actions that directly open specific screens within the main application window. However, at present, all the actions within the appMenuBar lead to the opening of screens within the main app window. So, the absence of a main app window makes these actions essentially pointless. --- src/qt/bitcoingui.cpp | 7 ------- 1 file changed, 7 deletions(-) (limited to 'src/qt/bitcoingui.cpp') diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 73e09630de..3375573677 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 @@ -470,13 +469,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")); -- cgit v1.2.3