From f88bfa81380ceb2c4afce5b15f753570a1ef063d Mon Sep 17 00:00:00 2001 From: Manuel Nickschas Date: Sun, 15 Jul 2018 23:48:32 +0200 Subject: [PATCH] core: Improve handling of core initialization errors Since the event loop may not be running while the core is being initialized, calling exit() does not have the desired effect. Instead, throw an exception with exit code and error message to be able to handle this correctly. Exceptions must not be thrown through the event loop, so ensure that they're caught before in cases where the event loop was started already. For the monolithic client, introduce an asynchronous init method that catches potential exceptions and calls exit() instead. Show an error popup in the UI if an error message is provided, so the user gets some feedback before the client is terminated. --- src/client/client.cpp | 10 +++ src/client/client.h | 4 ++ src/common/main.cpp | 12 +++- src/common/types.h | 14 ++++ src/core/core.cpp | 124 ++++++++++++++++++++--------------- src/core/core.h | 7 +- src/core/coreapplication.cpp | 11 ++-- src/core/coreapplication.h | 2 +- src/qtui/mainwin.cpp | 14 ++++ src/qtui/mainwin.h | 2 + src/qtui/monoapplication.cpp | 10 ++- src/qtui/monoapplication.h | 2 +- src/qtui/qtuiapplication.cpp | 33 +++++----- src/qtui/qtuiapplication.h | 2 +- 14 files changed, 161 insertions(+), 86 deletions(-) diff --git a/src/client/client.cpp b/src/client/client.cpp index c3c43851..23b838ff 100644 --- a/src/client/client.cpp +++ b/src/client/client.cpp @@ -214,6 +214,16 @@ void Client::onDbUpgradeInProgress(bool inProgress) } +void Client::onExitRequested(int exitCode, const QString &reason) +{ + if (!reason.isEmpty()) { + qCritical() << reason; + emit exitRequested(reason); + } + QCoreApplication::exit(exitCode); +} + + /*** Network handling ***/ QList Client::networkIds() diff --git a/src/client/client.h b/src/client/client.h index 45880cfd..00c9c974 100644 --- a/src/client/client.h +++ b/src/client/client.h @@ -256,6 +256,9 @@ signals: //! Emitted when database schema upgrade starts or ends (only mono client) void dbUpgradeInProgress(bool inProgress); + //! Emitted before an exit request is handled + void exitRequested(const QString &reason); + public slots: void disconnectFromCore(); @@ -266,6 +269,7 @@ public slots: void markBufferAsRead(BufferId id); void onDbUpgradeInProgress(bool inProgress); + void onExitRequested(int exitCode, const QString &reason); private slots: void setSyncedToCore(); diff --git a/src/common/main.cpp b/src/common/main.cpp index 3076eea5..6c4b5a42 100644 --- a/src/common/main.cpp +++ b/src/common/main.cpp @@ -71,6 +71,7 @@ Q_IMPORT_PLUGIN(qgif) #endif #include "quassel.h" +#include "types.h" int main(int argc, char **argv) { @@ -241,8 +242,15 @@ int main(int argc, char **argv) AboutData::setQuasselPersons(&aboutData); KAboutData::setApplicationData(aboutData.kAboutData()); #endif - if (!app.init()) - return EXIT_FAILURE; + try { + app.init(); + } + catch (ExitException e) { + if (!e.errorString.isEmpty()) { + qCritical() << e.errorString; + } + return e.exitCode; + } return app.exec(); } diff --git a/src/common/types.h b/src/common/types.h index d7b44044..72cee266 100644 --- a/src/common/types.h +++ b/src/common/types.h @@ -169,3 +169,17 @@ QDataStream &operator>>(QDataStream &in, T &value) { value = static_cast(v); return in; } + +// Exceptions + +/** + * Thrown during initialization to enforce exiting the application before the event loop is started + */ +struct ExitException +{ + int exitCode; + QString errorString; + + ExitException(int code = EXIT_FAILURE, QString error = {}) + : exitCode(code), errorString(std::move(error)) {} +}; diff --git a/src/core/core.cpp b/src/core/core.cpp index 5818e2fb..9b7af58d 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -33,6 +33,7 @@ #include "quassel.h" #include "sqlauthenticator.h" #include "sqlitestorage.h" +#include "types.h" #include "util.h" // Currently building with LDAP bindings is optional. @@ -100,7 +101,7 @@ Core::~Core() } -bool Core::init() +void Core::init() { _startTime = QDateTime::currentDateTime().toUTC(); // for uptime :) @@ -112,9 +113,7 @@ bool Core::init() // so far, we only have 1 CoreSettings s; if (s.version() != 1) { - qCritical() << "Invalid core settings version, terminating!"; - QCoreApplication::exit(EXIT_FAILURE); - return false; + throw ExitException{EXIT_FAILURE, tr("Invalid core settings version!")}; } // Set up storage and authentication backends @@ -135,7 +134,8 @@ bool Core::init() if (config_from_environment) { db_backend = environment.value("DB_BACKEND"); auth_authenticator = environment.value("AUTH_AUTHENTICATOR"); - } else { + } + else { CoreSettings cs; QVariantMap dbsettings = cs.storageSettings().toMap(); @@ -149,12 +149,15 @@ bool Core::init() writeError = !cs.isWritable(); } - // legacy - _configured = initStorage(db_backend, db_connectionProperties, environment, config_from_environment); - - // Not entirely sure what is 'legacy' about the above, but it seems to be the way things work! - if (_configured) { - initAuthenticator(auth_authenticator, auth_properties, environment, config_from_environment); + try { + _configured = initStorage(db_backend, db_connectionProperties, environment, config_from_environment); + if (_configured) { + _configured = initAuthenticator(auth_authenticator, auth_properties, environment, config_from_environment); + } + } + catch (ExitException) { + // Try again later + _configured = false; } if (Quassel::isOptionSet("select-backend") || Quassel::isOptionSet("select-authenticator")) { @@ -165,36 +168,36 @@ bool Core::init() if (Quassel::isOptionSet("select-authenticator")) { success &= selectAuthenticator(Quassel::optionValue("select-authenticator")); } - QCoreApplication::exit(success ? EXIT_SUCCESS : EXIT_FAILURE); - return success; + throw ExitException{success ? EXIT_SUCCESS : EXIT_FAILURE}; } if (!_configured) { if (config_from_environment) { - _configured = initStorage(db_backend, db_connectionProperties, environment, config_from_environment, true); - initAuthenticator(auth_authenticator, auth_properties, environment, config_from_environment, true); + try { + _configured = initStorage(db_backend, db_connectionProperties, environment, config_from_environment, true); + if (_configured) { + _configured = initAuthenticator(auth_authenticator, auth_properties, environment, config_from_environment, true); + } + } + catch (ExitException e) { + throw ExitException{EXIT_FAILURE, tr("Cannot configure from environment: %1").arg(e.errorString)}; + } if (!_configured) { - qWarning() << "Cannot configure from environment"; - QCoreApplication::exit(EXIT_FAILURE); - return false; + throw ExitException{EXIT_FAILURE, tr("Cannot configure from environment!")}; } } else { if (_registeredStorageBackends.empty()) { - quWarning() << qPrintable(tr("Could not initialize any storage backend! Exiting...")); - quWarning() - << qPrintable(tr("Currently, Quassel supports SQLite3 and PostgreSQL. You need to build your\n" - "Qt library with the sqlite or postgres plugin enabled in order for quasselcore\n" - "to work.")); - QCoreApplication::exit(EXIT_FAILURE); // TODO make this less brutal (especially for mono client -> popup) - return false; + throw ExitException{EXIT_FAILURE, + tr("Could not initialize any storage backend! Exiting...\n" + "Currently, Quassel supports SQLite3 and PostgreSQL. You need to build your\n" + "Qt library with the sqlite or postgres plugin enabled in order for quasselcore\n" + "to work.")}; } if (writeError) { - qWarning() << "Cannot write quasselcore configuration; probably a permission problem."; - QCoreApplication::exit(EXIT_FAILURE); - return false; + throw ExitException{EXIT_FAILURE, tr("Cannot write quasselcore configuration; probably a permission problem.")}; } quInfo() << "Core is currently not configured! Please connect with a Quassel Client for basic setup."; @@ -203,14 +206,12 @@ bool Core::init() else { if (Quassel::isOptionSet("add-user")) { bool success = createUser(); - QCoreApplication::exit(success ? EXIT_SUCCESS : EXIT_FAILURE); - return success; + throw ExitException{success ? EXIT_SUCCESS : EXIT_FAILURE}; } if (Quassel::isOptionSet("change-userpass")) { bool success = changeUserPass(Quassel::optionValue("change-userpass")); - QCoreApplication::exit(success ? EXIT_SUCCESS : EXIT_FAILURE); - return success; + throw ExitException{success ? EXIT_SUCCESS : EXIT_FAILURE}; } _strictIdentEnabled = Quassel::isOptionSet("strict-ident"); @@ -245,8 +246,7 @@ bool Core::init() connect(&_v6server, SIGNAL(newConnection()), this, SLOT(incomingConnection())); if (!startListening()) { - QCoreApplication::exit(EXIT_FAILURE); // TODO make this less brutal - return false; + throw ExitException{EXIT_FAILURE, tr("Cannot open port for listening!")}; } if (_configured && !Quassel::isOptionSet("norestore")) { @@ -259,10 +259,20 @@ bool Core::init() connectInternalPeer(_pendingInternalConnection); _pendingInternalConnection = {}; } +} - return true; + +void Core::initAsync() +{ + try { + init(); + } + catch (ExitException e) { + emit exitRequested(e.exitCode, e.errorString); + } } + /*** Session Restore ***/ void Core::saveState() @@ -325,14 +335,21 @@ QString Core::setupCore(const QString &adminUser, const QString &adminPassword, if (adminUser.isEmpty() || adminPassword.isEmpty()) { return tr("Admin user or password not set."); } - if (!(_configured = initStorage(backend, setupData, {}, false, true))) { - return tr("Could not setup storage!"); - } + try { + if (!(_configured = initStorage(backend, setupData, {}, false, true))) { + return tr("Could not setup storage!"); + } - quInfo() << "Selected authenticator:" << authenticator; - if (!(_configured = initAuthenticator(authenticator, authSetupData, {}, false, true))) - { - return tr("Could not setup authenticator!"); + quInfo() << "Selected authenticator:" << authenticator; + if (!(_configured = initAuthenticator(authenticator, authSetupData, {}, false, true))) + { + return tr("Could not setup authenticator!"); + } + } + catch (ExitException e) { + // Event loop is running, so trigger an exit rather than throwing an exception + QCoreApplication::exit(e.exitCode); + return e.errorString.isEmpty() ? tr("Fatal failure while trying to setup, terminating") : e.errorString; } if (!saveBackendSettings(backend, setupData)) { @@ -395,8 +412,7 @@ DeferredSharedPtr Core::storageBackend(const QString &backendId) const return it != _registeredStorageBackends.end() ? *it : nullptr; } -// old db settings: -// "Type" => "sqlite" + bool Core::initStorage(const QString &backend, const QVariantMap &settings, const QProcessEnvironment &environment, bool loadFromEnvironment, bool setup) { @@ -422,12 +438,12 @@ bool Core::initStorage(const QString &backend, const QVariantMap &settings, return initStorage(backend, settings, environment, loadFromEnvironment, false); return false; - // if initialization wasn't successful, we quit to keep from coming up unconfigured case Storage::NotAvailable: - qCritical() << "FATAL: Selected storage backend is not available:" << backend; if (!setup) { - QCoreApplication::exit(EXIT_FAILURE); + // If initialization wasn't successful, we quit to keep from coming up unconfigured + throw ExitException{EXIT_FAILURE, tr("Selected storage backend %1 is not available.").arg(backend)}; } + qCritical() << "Selected storage backend is not available:" << backend; return false; case Storage::IsReady: @@ -522,12 +538,12 @@ bool Core::initAuthenticator(const QString &backend, const QVariantMap &settings return initAuthenticator(backend, settings, environment, loadFromEnvironment, false); return false; - // if initialization wasn't successful, we quit to keep from coming up unconfigured case Authenticator::NotAvailable: - qCritical() << "FATAL: Selected auth backend is not available:" << backend; if (!setup) { - QCoreApplication::exit(EXIT_FAILURE); + // If initialization wasn't successful, we quit to keep from coming up unconfigured + throw ExitException{EXIT_FAILURE, tr("Selected auth backend %1 is not available.").arg(backend)}; } + qCritical() << "Selected auth backend is not available:" << backend; return false; case Authenticator::IsReady: @@ -797,7 +813,11 @@ void Core::setupInternalClientSession(QPointer clientPeer) { if (!_configured) { stopListening(); - setupCoreForInternalUsage(); + auto errorString = setupCoreForInternalUsage(); + if (!errorString.isEmpty()) { + emit exitRequested(EXIT_FAILURE, errorString); + return; + } } UserId uid; @@ -806,7 +826,7 @@ void Core::setupInternalClientSession(QPointer clientPeer) } else { quWarning() << "Core::setupInternalClientSession(): You're trying to run monolithic Quassel with an unusable Backend! Go fix it!"; - QCoreApplication::exit(EXIT_FAILURE); + emit exitRequested(EXIT_FAILURE, tr("Cannot setup storage backend.")); return; } diff --git a/src/core/core.h b/src/core/core.h index e7a8c635..102252b6 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -68,6 +68,8 @@ public: Core(); ~Core() override; + void init(); + /*** Storage access ***/ // These methods are threadsafe. @@ -707,8 +709,11 @@ signals: //! Emitted when database schema upgrade starts or ends void dbUpgradeInProgress(bool inProgress); + //! Emitted when a fatal error was encountered during async initialization + void exitRequested(int exitCode, const QString &reason); + public slots: - bool init(); + void initAsync(); /** Persist storage. * diff --git a/src/core/coreapplication.cpp b/src/core/coreapplication.cpp index cacd671c..e73a09fb 100644 --- a/src/core/coreapplication.cpp +++ b/src/core/coreapplication.cpp @@ -40,11 +40,12 @@ CoreApplication::~CoreApplication() } -bool CoreApplication::init() +void CoreApplication::init() { - if (Quassel::init()) { - _core.reset(new Core{}); // FIXME C++14: std::make_unique - return _core->init(); + if (!Quassel::init()) { + throw ExitException{EXIT_FAILURE, tr("Could not initialize Quassel!")}; } - return false; + + _core.reset(new Core{}); // FIXME C++14: std::make_unique + _core->init(); } diff --git a/src/core/coreapplication.h b/src/core/coreapplication.h index 3c1e5638..f157b548 100644 --- a/src/core/coreapplication.h +++ b/src/core/coreapplication.h @@ -35,7 +35,7 @@ public: CoreApplication(int &argc, char **argv); ~CoreApplication() override; - bool init(); + void init(); private: std::unique_ptr _core; diff --git a/src/qtui/mainwin.cpp b/src/qtui/mainwin.cpp index 80879a24..3ada47fd 100644 --- a/src/qtui/mainwin.cpp +++ b/src/qtui/mainwin.cpp @@ -216,6 +216,7 @@ void MainWin::init() connect(GraphicalUi::contextMenuActionProvider(), SIGNAL(showIgnoreList(QString)), SLOT(showIgnoreList(QString))); connect(Client::instance(), SIGNAL(showIgnoreList(QString)), SLOT(showIgnoreList(QString))); connect(Client::instance(), SIGNAL(dbUpgradeInProgress(bool)), SLOT(showMigrationWarning(bool))); + connect(Client::instance(), SIGNAL(exitRequested(QString)), SLOT(onExitRequested(QString))); connect(Client::coreConnection(), SIGNAL(startCoreSetup(QVariantList, QVariantList)), SLOT(showCoreConfigWizard(QVariantList, QVariantList))); connect(Client::coreConnection(), SIGNAL(connectionErrorPopup(QString)), SLOT(handleCoreConnectionError(QString))); @@ -858,6 +859,19 @@ void MainWin::showMigrationWarning(bool show) } +void MainWin::onExitRequested(const QString &reason) +{ + if (!reason.isEmpty()) { + QMessageBox box(QMessageBox::Critical, + tr("Fatal error"), + "" + tr("Quassel encountered a fatal error and is terminated.") + "", + QMessageBox::Ok, this); + box.setInformativeText("

" + tr("Reason:") + " " + reason + ""); + box.exec(); + } +} + + void MainWin::changeActiveBufferView(bool backwards) { if (_activeBufferViewIndex >= 0 && _activeBufferViewIndex < _bufferViews.count()) { diff --git a/src/qtui/mainwin.h b/src/qtui/mainwin.h index 476db511..9dc26949 100644 --- a/src/qtui/mainwin.h +++ b/src/qtui/mainwin.h @@ -100,6 +100,8 @@ public slots: void showMigrationWarning(bool show); + void onExitRequested(const QString &reason); + //! Quit application void quit(); diff --git a/src/qtui/monoapplication.cpp b/src/qtui/monoapplication.cpp index d880f33f..e92afcc6 100644 --- a/src/qtui/monoapplication.cpp +++ b/src/qtui/monoapplication.cpp @@ -38,10 +38,9 @@ MonolithicApplication::MonolithicApplication(int &argc, char **argv) } -bool MonolithicApplication::init() +void MonolithicApplication::init() { - if (!QtUiApplication::init()) - return false; + QtUiApplication::init(); connect(Client::coreConnection(), SIGNAL(connectToInternalCore(QPointer)), this, SLOT(onConnectionRequest(QPointer))); @@ -51,8 +50,6 @@ bool MonolithicApplication::init() if (Quassel::isOptionSet("port")) { startInternalCore(); } - - return true; } @@ -76,13 +73,14 @@ void MonolithicApplication::startInternalCore() // Start internal core in a separate thread, so it doesn't block the UI _core = new Core{}; _core->moveToThread(&_coreThread); - connect(&_coreThread, SIGNAL(started()), _core, SLOT(init())); + connect(&_coreThread, SIGNAL(started()), _core, SLOT(initAsync())); connect(&_coreThread, SIGNAL(finished()), _core, SLOT(deleteLater())); connect(this, SIGNAL(connectInternalPeer(QPointer)), _core, SLOT(connectInternalPeer(QPointer))); connect(_core, SIGNAL(sessionState(Protocol::SessionState)), Client::coreConnection(), SLOT(internalSessionStateReceived(Protocol::SessionState))); connect(_core, SIGNAL(dbUpgradeInProgress(bool)), Client::instance(), SLOT(onDbUpgradeInProgress(bool))); + connect(_core, SIGNAL(exitRequested(int,QString)), Client::instance(), SLOT(onExitRequested(int,QString))); _coreThread.start(); } diff --git a/src/qtui/monoapplication.h b/src/qtui/monoapplication.h index 59e9549a..56f4f93b 100644 --- a/src/qtui/monoapplication.h +++ b/src/qtui/monoapplication.h @@ -36,7 +36,7 @@ public: MonolithicApplication(int &, char **); ~MonolithicApplication() override; - bool init() override; + void init() override; signals: void connectInternalPeer(QPointer peer); diff --git a/src/qtui/qtuiapplication.cpp b/src/qtui/qtuiapplication.cpp index c9475fee..20c1662e 100644 --- a/src/qtui/qtuiapplication.cpp +++ b/src/qtui/qtuiapplication.cpp @@ -34,6 +34,7 @@ #include "mainwin.h" #include "qtui.h" #include "qtuisettings.h" +#include "types.h" QtUiApplication::QtUiApplication(int &argc, char **argv) #ifdef HAVE_KDE4 @@ -98,28 +99,26 @@ QtUiApplication::QtUiApplication(int &argc, char **argv) } -bool QtUiApplication::init() +void QtUiApplication::init() { - if (Quassel::init()) { - // Settings upgrade/downgrade handling - if (!migrateSettings()) { - qCritical() << "Could not load or upgrade client settings, terminating!"; - return false; - } + if (!Quassel::init()) { + throw ExitException{EXIT_FAILURE, tr("Could not initialize Quassel!")}; + } - Client::init(new QtUi()); + // Settings upgrade/downgrade handling + if (!migrateSettings()) { + throw ExitException{EXIT_FAILURE, tr("Could not load or upgrade client settings!")}; + } - // Init UI only after the event loop has started - // TODO Qt5: Make this a lambda - QTimer::singleShot(0, this, SLOT(initUi())); + Client::init(new QtUi()); - Quassel::registerQuitHandler([]() { - QtUi::mainWindow()->quit(); - }); + // Init UI only after the event loop has started + // TODO Qt5: Make this a lambda + QTimer::singleShot(0, this, SLOT(initUi())); - return true; - } - return false; + Quassel::registerQuitHandler([]() { + QtUi::mainWindow()->quit(); + }); } diff --git a/src/qtui/qtuiapplication.h b/src/qtui/qtuiapplication.h index e5e20b54..508bea52 100644 --- a/src/qtui/qtuiapplication.h +++ b/src/qtui/qtuiapplication.h @@ -47,7 +47,7 @@ class QtUiApplication : public QApplication public: QtUiApplication(int &, char **); ~QtUiApplication(); - virtual bool init(); + virtual void init(); void resumeSessionIfPossible(); inline bool isAboutToQuit() const { return _aboutToQuit; } -- 2.20.1