From 358e5d557d527675c7bc62e58a4c7ad3b408897c Mon Sep 17 00:00:00 2001 From: Manuel Nickschas Date: Thu, 31 May 2018 00:34:13 +0200 Subject: [PATCH 1/1] mono: Run the internal core in a separate thread Prevent the internal core from blocking the UI of the monolithic client by running it in a separate thread. This is particularly important during startup, where e.g. a database upgrade may block the core for a significant time, preventing the main window from being rendered, or even showing up. --- src/client/coreconnection.cpp | 2 -- src/client/coreconnection.h | 3 +-- src/common/internalpeer.cpp | 1 + src/common/internalpeer.h | 3 +++ src/core/core.cpp | 26 ++++++++++++++++++++- src/core/core.h | 26 +++++++++++++-------- src/qtui/monoapplication.cpp | 43 ++++++++++++++++++++++++++++++----- src/qtui/monoapplication.h | 14 ++++++++++-- 8 files changed, 96 insertions(+), 22 deletions(-) diff --git a/src/client/coreconnection.cpp b/src/client/coreconnection.cpp index d889a9c6..dd196d11 100644 --- a/src/client/coreconnection.cpp +++ b/src/client/coreconnection.cpp @@ -381,14 +381,12 @@ void CoreConnection::connectToCurrentAccount() qWarning() << "Cannot connect to internal core in client-only mode!"; return; } - emit startInternalCore(); InternalPeer *peer = new InternalPeer(); _peer = peer; Client::instance()->signalProxy()->addPeer(peer); // sigproxy will take ownership emit connectToInternalCore(peer); setState(Connected); - return; } diff --git a/src/client/coreconnection.h b/src/client/coreconnection.h index ba8a17ca..69b4c9e5 100644 --- a/src/client/coreconnection.h +++ b/src/client/coreconnection.h @@ -101,8 +101,7 @@ signals: void coreSetupSuccess(); void coreSetupFailed(const QString &error); - void startInternalCore(); - void connectToInternalCore(InternalPeer *connection); + void connectToInternalCore(QPointer connection); // These signals MUST be handled synchronously! void userAuthenticationRequired(CoreAccount *, bool *valid, const QString &errorMessage = QString()); diff --git a/src/common/internalpeer.cpp b/src/common/internalpeer.cpp index 4180b76b..30b46698 100644 --- a/src/common/internalpeer.cpp +++ b/src/common/internalpeer.cpp @@ -26,6 +26,7 @@ InternalPeer::InternalPeer(QObject *parent) : Peer(nullptr, parent) { static bool registered = []() { + qRegisterMetaType>(); qRegisterMetaType(); qRegisterMetaType(); qRegisterMetaType(); diff --git a/src/common/internalpeer.h b/src/common/internalpeer.h index c00319b7..f40f8eb9 100644 --- a/src/common/internalpeer.h +++ b/src/common/internalpeer.h @@ -20,6 +20,7 @@ #pragma once +#include #include #include "peer.h" @@ -94,3 +95,5 @@ private: SignalProxy *_proxy{nullptr}; bool _isOpen{false}; }; + +Q_DECLARE_METATYPE(QPointer) diff --git a/src/core/core.cpp b/src/core/core.cpp index 3089a739..3f49a227 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -248,6 +248,13 @@ bool Core::init() Core::restoreState(); } + _initialized = true; + + if (_pendingInternalConnection) { + connectInternalPeer(_pendingInternalConnection); + _pendingInternalConnection = {}; + } + return true; } @@ -760,7 +767,18 @@ void Core::addClientHelper(RemotePeer *peer, UserId uid) } -void Core::setupInternalClientSession(InternalPeer *clientPeer) +void Core::connectInternalPeer(QPointer peer) +{ + if (_initialized && peer) { + setupInternalClientSession(peer); + } + else { + _pendingInternalConnection = peer; + } +} + + +void Core::setupInternalClientSession(QPointer clientPeer) { if (!_configured) { stopListening(); @@ -773,6 +791,12 @@ void Core::setupInternalClientSession(InternalPeer *clientPeer) } else { quWarning() << "Core::setupInternalClientSession(): You're trying to run monolithic Quassel with an unusable Backend! Go fix it!"; + QCoreApplication::exit(EXIT_FAILURE); + return; + } + + if (!clientPeer) { + quWarning() << "Client peer went away, not starting a session"; return; } diff --git a/src/core/core.h b/src/core/core.h index 53869c2b..e0ec7481 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -47,10 +48,12 @@ class CoreAuthHandler; class CoreSession; -struct NetworkInfo; +class InternalPeer; class SessionThread; class SignalProxy; +struct NetworkInfo; + class AbstractSqlMigrationReader; class AbstractSqlMigrationWriter; @@ -692,6 +695,13 @@ public: static const int AddClientEventId; +signals: + //! Sent when a BufferInfo is updated in storage. + void bufferInfoUpdated(UserId user, const BufferInfo &info); + + //! Relay from CoreSession::sessionState(). Used for internal connection only + void sessionState(const Protocol::SessionState &sessionState); + public slots: bool init(); @@ -710,15 +720,9 @@ public slots: void cacheSysIdent(); - void setupInternalClientSession(InternalPeer *clientConnection); QString setupCore(const QString &adminUser, const QString &adminPassword, const QString &backend, const QVariantMap &setupData, const QString &authenticator, const QVariantMap &authSetupMap); -signals: - //! Sent when a BufferInfo is updated in storage. - void bufferInfoUpdated(UserId user, const BufferInfo &info); - - //! Relay from CoreSession::sessionState(). Used for internal connection only - void sessionState(const Protocol::SessionState &sessionState); + void connectInternalPeer(QPointer peer); protected: void customEvent(QEvent *event) override; @@ -746,6 +750,7 @@ private: void addClientHelper(RemotePeer *peer, UserId uid); //void processCoreSetup(QTcpSocket *socket, QVariantMap &msg); QString setupCoreForInternalUsage(); + void setupInternalClientSession(QPointer peer); bool createUser(); @@ -796,7 +801,10 @@ private: QDateTime _startTime; - bool _configured; + bool _initialized{false}; + bool _configured{false}; + + QPointer _pendingInternalConnection; /// Whether or not strict ident mode is enabled, locking users' idents to Quassel username bool _strictIdentEnabled; diff --git a/src/qtui/monoapplication.cpp b/src/qtui/monoapplication.cpp index c2ae9e5a..1a8822d6 100644 --- a/src/qtui/monoapplication.cpp +++ b/src/qtui/monoapplication.cpp @@ -22,6 +22,7 @@ #include "coreapplication.h" #include "client.h" #include "core.h" +#include "internalpeer.h" #include "qtui.h" class InternalPeer; @@ -42,7 +43,14 @@ bool MonolithicApplication::init() if (!QtUiApplication::init()) return false; - connect(Client::coreConnection(), SIGNAL(startInternalCore()), SLOT(startInternalCore())); + connect(Client::coreConnection(), SIGNAL(connectToInternalCore(QPointer)), this, SLOT(onConnectionRequest(QPointer))); + + // If port is set, start internal core directly so external clients can connect + // This is useful in case the mono client re-gains remote connection capability, + // in which case the internal core would not have to be started by default. + if (Quassel::isOptionSet("port")) { + startInternalCore(); + } return true; } @@ -52,17 +60,40 @@ MonolithicApplication::~MonolithicApplication() { // Client needs to be destroyed first Client::destroy(); - _core.reset(); + _coreThread.quit(); + _coreThread.wait(); Quassel::destroy(); } void MonolithicApplication::startInternalCore() +{ + if (_core) { + // Already started + return; + } + + // 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(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))); + + _coreThread.start(); +} + + +void MonolithicApplication::onConnectionRequest(QPointer peer) { if (!_core) { - _core.reset(new Core{}); // FIXME C++14: std::make_unique - Core::instance()->init(); + startInternalCore(); } - connect(Client::coreConnection(), SIGNAL(connectToInternalCore(InternalPeer*)), Core::instance(), SLOT(setupInternalClientSession(InternalPeer*))); - connect(Core::instance(), SIGNAL(sessionState(Protocol::SessionState)), Client::coreConnection(), SLOT(internalSessionStateReceived(Protocol::SessionState))); + + // While starting the core may take a while, the object itself is instantiated synchronously and the connections + // established, so it's safe to emit this immediately. The core will take care of queueing the request until + // initialization is complete. + emit connectInternalPeer(peer); } diff --git a/src/qtui/monoapplication.h b/src/qtui/monoapplication.h index 34363bba..59e9549a 100644 --- a/src/qtui/monoapplication.h +++ b/src/qtui/monoapplication.h @@ -20,24 +20,34 @@ #pragma once -#include +#include +#include #include "qtuiapplication.h" class Core; +class InternalPeer; class MonolithicApplication : public QtUiApplication { Q_OBJECT + public: MonolithicApplication(int &, char **); ~MonolithicApplication() override; bool init() override; +signals: + void connectInternalPeer(QPointer peer); + private slots: + void onConnectionRequest(QPointer peer); + +private: void startInternalCore(); private: - std::unique_ptr _core; + QPointer _core; + QThread _coreThread; }; -- 2.20.1