Fix CoreServices shutdown responsibility and fix lp1912129

src/coreservices.cpp

@@ -105,7 +105,8 @@ namespace mixxx {
 
 CoreServices::CoreServices(const CmdlineArgs& args, QApplication* pApp)
         : m_runtime_timer(QLatin1String("CoreServices::runtime")),
-          m_cmdlineArgs(args) {
+          m_cmdlineArgs(args),
+          m_isInitialized(false) {
     m_runtime_timer.start();
     mixxx::Time::start();
     ScopedTimer t("CoreServices::CoreServices");
@@ -123,7 +124,57 @@ CoreServices::CoreServices(const CmdlineArgs& args, QApplication* pApp)
     initializeKeyboard();
 }
 
-CoreServices::~CoreServices() = default;
+CoreServices::~CoreServices() {
+    if (m_isInitialized) {
+        finalize();
+    }
+
+    // Tear down remaining stuff that was initialized in the constructor.
+    CLEAR_AND_CHECK_DELETED(m_pKeyboardEventFilter);
+    CLEAR_AND_CHECK_DELETED(m_pKbdConfig);
+    CLEAR_AND_CHECK_DELETED(m_pKbdConfigEmpty);
+
+    if (m_cmdlineArgs.getDeveloper()) {
+        StatsManager::destroy();
+    }
+
+    // HACK: Save config again. We saved it once before doing some dangerous
+    // stuff. We only really want to save it here, but the first one was just
+    // a precaution. The earlier one can be removed when stuff is more stable
+    // at exit.
+    m_pSettingsManager->save();
+    m_pSettingsManager.reset();
+
+    Sandbox::shutdown();
+
+    // Check for leaked ControlObjects and give warnings.
+    {
+        const QList<QSharedPointer<ControlDoublePrivate>> leakedControls =
+                ControlDoublePrivate::takeAllInstances();
+        if (!leakedControls.isEmpty()) {
+            qWarning()
+                    << "The following"
+                    << leakedControls.size()
+                    << "controls were leaked:";
+            for (auto pCDP : leakedControls) {
+                ConfigKey key = pCDP->getKey();
+                qWarning() << key.group << key.item << pCDP->getCreatorCO();
+                // Deleting leaked objects helps to satisfy valgrind.
+                // These delete calls could cause crashes if a destructor for a control
+                // we thought was leaked is triggered after this one exits.
+                // So, only delete so if developer mode is on.
+                if (CmdlineArgs::Instance().getDeveloper()) {
+                    pCDP->deleteCreatorCO();
+                }
+            }
+            DEBUG_ASSERT(!"Controls were leaked!");
+        }
+        // Finally drop all shared pointers by exiting this scope
+    }
+
+    // Report the total time we have been running.
+    m_runtime_timer.elapsed(true);
+}
 
 void CoreServices::initializeSettings() {
 #ifdef __APPLE__
@@ -154,6 +205,10 @@ void CoreServices::initializeLogging() {
 }
 
 void CoreServices::initialize(QApplication* pApp) {
+    VERIFY_OR_DEBUG_ASSERT(!m_isInitialized) {
+        return;
+    }
+
     ScopedTimer t("CoreServices::initialize");
 
     VERIFY_OR_DEBUG_ASSERT(SoundSourceProxy::registerProviders()) {
@@ -392,6 +447,8 @@ void CoreServices::initialize(QApplication* pApp) {
             m_pPlayerManager->slotLoadToDeck(musicFiles.at(i), i + 1);
         }
     }
+
+    m_isInitialized = true;
 }
 
 void CoreServices::initializeKeyboard() {
@@ -475,8 +532,13 @@ bool CoreServices::initializeDatabase() {
     return MixxxDb::initDatabaseSchema(dbConnection);
 }
 
-void CoreServices::shutdown() {
-    Timer t("CoreServices::shutdown");
+void CoreServices::finalize() {
+    VERIFY_OR_DEBUG_ASSERT(m_isInitialized) {
+        qDebug() << "Skipping CoreServices finalization because it was never initialized.";
+        return;
+    }
+
+    Timer t("CoreServices::~CoreServices");
     t.start();
 
     // Stop all pending library operations
@@ -549,57 +611,11 @@ void CoreServices::shutdown() {
     m_pDbConnectionPool->destroyThreadLocalConnection();
     m_pDbConnectionPool.reset(); // should drop the last reference
 
-    // HACK: Save config again. We saved it once before doing some dangerous
-    // stuff. We only really want to save it here, but the first one was just
-    // a precaution. The earlier one can be removed when stuff is more stable
-    // at exit.
-    m_pSettingsManager->save();
-
     m_pTouchShift.reset();
 
     m_pControlIndicatorTimer.reset();
 
-    // Check for leaked ControlObjects and give warnings.
-    {
-        const QList<QSharedPointer<ControlDoublePrivate>> leakedControls =
-                ControlDoublePrivate::takeAllInstances();
-        if (!leakedControls.isEmpty()) {
-            qWarning()
-                    << "The following"
-                    << leakedControls.size()
-                    << "controls were leaked:";
-            for (auto pCDP : leakedControls) {
-                ConfigKey key = pCDP->getKey();
-                qWarning() << key.group << key.item << pCDP->getCreatorCO();
-                // Deleting leaked objects helps to satisfy valgrind.
-                // These delete calls could cause crashes if a destructor for a control
-                // we thought was leaked is triggered after this one exits.
-                // So, only delete so if developer mode is on.
-                if (CmdlineArgs::Instance().getDeveloper()) {
-                    pCDP->deleteCreatorCO();
-                }
-            }
-            DEBUG_ASSERT(!"Controls were leaked!");
-        }
-        // Finally drop all shared pointers by exiting this scope
-    }
-
-    Sandbox::shutdown();
-
-    qDebug() << t.elapsed(false).debugMillisWithUnit() << "deleting SettingsManager";
-    m_pSettingsManager.reset();
-
-    CLEAR_AND_CHECK_DELETED(m_pKeyboardEventFilter);
-    CLEAR_AND_CHECK_DELETED(m_pKbdConfig);
-    CLEAR_AND_CHECK_DELETED(m_pKbdConfigEmpty);
-
     t.elapsed(true);
-    // Report the total time we have been running.
-    m_runtime_timer.elapsed(true);
-
-    if (m_cmdlineArgs.getDeveloper()) {
-        StatsManager::destroy();
-    }
 }
 
 } // namespace mixxx

src/coreservices.h

@@ -42,7 +42,6 @@ class CoreServices : public QObject {
 
     /// The secondary long run which should be called after displaying the start up screen
     void initialize(QApplication* pApp);
-    void shutdown();
 
     std::shared_ptr<KeyboardEventFilter> getKeyboardEventFilter() const {
         return m_pKeyboardEventFilter;
@@ -123,6 +122,9 @@ class CoreServices : public QObject {
     void initializeScreensaverManager();
     void initializeLogging();
 
+    /// Tear down CoreServices that were previously initialized by `initialize()`.
+    void finalize();
+
     std::shared_ptr<SettingsManager> m_pSettingsManager;
     std::shared_ptr<mixxx::ControlIndicatorTimer> m_pControlIndicatorTimer;
     std::shared_ptr<EffectsManager> m_pEffectsManager;
@@ -153,6 +155,7 @@ class CoreServices : public QObject {
 
     Timer m_runtime_timer;
     const CmdlineArgs& m_cmdlineArgs;
+    bool m_isInitialized;
 };
 
 } // namespace mixxx

src/main.cpp

@@ -27,28 +27,36 @@ int runMixxx(MixxxApplication* pApp, const CmdlineArgs& args) {
 
     CmdlineArgs::Instance().parseForUserFeedback();
 
-    MixxxMainWindow mainWindow(pCoreServices);
-    pApp->processEvents();
-    pApp->installEventFilter(&mainWindow);
-
-    QObject::connect(pCoreServices.get(),
-            &mixxx::CoreServices::initializationProgressUpdate,
-            &mainWindow,
-            &MixxxMainWindow::initializationProgressUpdate);
-    pCoreServices->initialize(pApp);
-    mainWindow.initialize();
-
-    // If startup produced a fatal error, then don't even start the
-    // Qt event loop.
-    if (ErrorDialogHandler::instance()->checkError()) {
-        return kFatalErrorOnStartupExitCode;
-    } else {
-        qDebug() << "Displaying main window";
-        mainWindow.show();
-
-        qDebug() << "Running Mixxx";
-        return pApp->exec();
+    int exitCode;
+
+    // This scope ensures that `MixxxMainWindow` is destroyed *before*
+    // CoreServices is shut down. Otherwise a debug assertion complaining about
+    // leaked COs may be triggered.
+    {
+        MixxxMainWindow mainWindow(pCoreServices);
+        pApp->processEvents();
+        pApp->installEventFilter(&mainWindow);
+
+        QObject::connect(pCoreServices.get(),
+                &mixxx::CoreServices::initializationProgressUpdate,
+                &mainWindow,
+                &MixxxMainWindow::initializationProgressUpdate);
+        pCoreServices->initialize(pApp);
+        mainWindow.initialize();
+
+        // If startup produced a fatal error, then don't even start the
+        // Qt event loop.
+        if (ErrorDialogHandler::instance()->checkError()) {
+            exitCode = kFatalErrorOnStartupExitCode;
+        } else {
+            qDebug() << "Displaying main window";
+            mainWindow.show();
+
+            qDebug() << "Running Mixxx";
+            exitCode = pApp->exec();
+        }
     }
+    return exitCode;
 }
 
 } // anonymous namespace

src/mixxxmainwindow.cpp

@@ -400,8 +400,6 @@ MixxxMainWindow::~MixxxMainWindow() {
 
     delete m_pGuiTick;
     delete m_pVisualsManager;
-
-    m_pCoreServices->shutdown();
 }
 
 void MixxxMainWindow::initializeWindow() {