Skip to content

Commit

Permalink
Merge pull request #4213 from Holzhaus/coreservices-shutdown
Browse files Browse the repository at this point in the history 
Fix CoreServices shutdown responsibility and fix lp1912129
  • Loading branch information
@daschuer
daschuer authored Aug 17, 2021
2 parents 0b428d3 + 54bd6f5 commit fd79c05
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 74 deletions.
116 changes: 66 additions & 50 deletions src/coreservices.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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__
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -392,6 +447,8 @@ void CoreServices::initialize(QApplication* pApp) {
m_pPlayerManager->slotLoadToDeck(musicFiles.at(i), i + 1);
}
}

m_isInitialized = true;
}

void CoreServices::initializeKeyboard() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
5 changes: 4 additions & 1 deletion src/coreservices.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -153,6 +155,7 @@ class CoreServices : public QObject {

Timer m_runtime_timer;
const CmdlineArgs& m_cmdlineArgs;
bool m_isInitialized;
};

} // namespace mixxx
50 changes: 29 additions & 21 deletions src/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions src/mixxxmainwindow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,6 @@ MixxxMainWindow::~MixxxMainWindow() {

delete m_pGuiTick;
delete m_pVisualsManager;

m_pCoreServices->shutdown();
}

void MixxxMainWindow::initializeWindow() {
Expand Down

0 comments on commit fd79c05

Please sign in to comment.