diff --git a/src/GUI/QtLauncher.cxx b/src/GUI/QtLauncher.cxx index a06e7e020..ea7e44217 100644 --- a/src/GUI/QtLauncher.cxx +++ b/src/GUI/QtLauncher.cxx @@ -232,6 +232,14 @@ static void simgearMessageOutput(QtMsgType type, const QMessageLogContext &conte namespace flightgear { +// making this a unique ptr ensures the QApplication will be deleted +// event if we forget to call shutdownQtApp. Cleanly destroying this is +// important so QPA resources, in particular the XCB thread, are exited +// cleanly on quit. However, at present, the official policy is that static +// destruction is too late to call this, hence why we have shutdownQtApp() + +std::unique_ptr static_qApp; + // Only requires FGGlobals to be initialized if 'doInitQSettings' is true. // Safe to call several times. void initApp(int& argc, char** argv, bool doInitQSettings) @@ -256,13 +264,13 @@ void initApp(int& argc, char** argv, bool doInitQSettings) #if QT_VERSION >= QT_VERSION_CHECK(5, 6, 0) QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling); #endif - QApplication* app = new QApplication(s_argc, argv); - app->setOrganizationName("FlightGear"); - app->setApplicationName("FlightGear"); - app->setOrganizationDomain("flightgear.org"); + static_qApp.reset(new QApplication(s_argc, argv)); + static_qApp->setOrganizationName("FlightGear"); + static_qApp->setApplicationName("FlightGear"); + static_qApp->setOrganizationDomain("flightgear.org"); #if QT_VERSION >= QT_VERSION_CHECK(5, 7, 0) - app->setDesktopFileName( + static_qApp->setDesktopFileName( QStringLiteral("org.flightgear.FlightGear.desktop")); #endif @@ -277,6 +285,11 @@ void initApp(int& argc, char** argv, bool doInitQSettings) } } +void shutdownQtApp() +{ + static_qApp.reset(); +} + // Requires FGGlobals to be initialized. Safe to call several times. void initQSettings() { diff --git a/src/GUI/QtLauncher.hxx b/src/GUI/QtLauncher.hxx index 1b414043f..22742be3c 100644 --- a/src/GUI/QtLauncher.hxx +++ b/src/GUI/QtLauncher.hxx @@ -26,6 +26,11 @@ namespace flightgear // Only requires FGGlobals to be initialized if 'doInitQSettings' is true. // Safe to call several times. void initApp(int& argc, char** argv, bool doInitQSettings = true); + + // ensures Qt-related resources are cleaned up. Avoids crashes on shutdown + // if QPA assets are hanging around. (With the XCB QPA plugin especially) + void shutdownQtApp(); + // Requires FGGlobals to be initialized. Safe to call several times. void initQSettings(); diff --git a/src/Main/bootstrap.cxx b/src/Main/bootstrap.cxx index 61c4089df..e1648b2ce 100644 --- a/src/Main/bootstrap.cxx +++ b/src/Main/bootstrap.cxx @@ -69,6 +69,10 @@ #include "fg_os.hxx" +#if defined(HAVE_QT) +#include +#endif + #if defined(HAVE_CRASHRPT) #include @@ -351,6 +355,10 @@ int main ( int argc, char **argv ) perror("Possible cause"); } +#if defined(HAVE_QT) + flightgear::shutdownQtApp(); +#endif + #if defined(HAVE_CRASHRPT) crUninstall(); #endif @@ -364,10 +372,13 @@ void fgExitCleanup() { if (_bootstrap_OSInit != 0) { fgSetMouseCursor(MOUSE_CURSOR_POINTER); - fgOSCloseWindow(); } + // you might imagine we'd call shutdownQtApp here, but it's not safe to do + // so in an atexit handler, and crashes on Mac. Thiago states this explicitly: + // https://bugreports.qt.io/browse/QTBUG-48709 + // on the common exit path globals is already deleted, and NULL, // so this only happens on error paths. delete globals;