Commit 1593c17d authored by Daniel Vrátil's avatar Daniel Vrátil 🤖

Fix crash when Connection is closed while ItemRetriever is running

ItemRetriever::exec() runs a nested QEventLoop. If the parent
Connection is terminated while the event loop is running, the
delayed AkThread::quit() invocation is executed and the thread
is destroyed, while technically there is still execution going
on. To workaround that issue we don't destroy the thread in
Connection::quit() if we detect a nested QEventLoop but instead
we just notify the event loop to quit and destroy the thread
later when execution returns back to ItemRetriever::slotNewData().

This fixes a crash in QPSQLQuery, which was dereferencing a
QPSQLDriver after the driver has been deleted as the thread was
destroyed and avoids more of random crashes due to the Connection
being virtually deleted from its own nested event loop.

BUG: 374734
FIXED-IN: 5.4.3
parent 5219f770
......@@ -31,7 +31,8 @@
#include "tracer.h"
#include "collectionreferencemanager.h"
#include <assert.h>
#include <cassert>
#include <cxxabi.h>
#include <private/protocol_p.h>
#include <private/datastream_p_p.h>
......@@ -53,6 +54,7 @@ Connection::Connection(QObject *parent)
, m_verifyCacheOnRetrieval(false)
, m_idleTimer(Q_NULLPTR)
, m_totalTime( 0 )
, m_connectionClosing(false)
, m_reportTime( false )
{
}
......@@ -107,6 +109,12 @@ void Connection::init()
void Connection::quit()
{
if (QThread::currentThread()->loopLevel() > 1) {
m_connectionClosing = true;
Q_EMIT connectionClosing();
return;
}
Tracer::self()->endConnection(m_identifier, QString());
collectionReferenceManager()->removeSession(m_sessionId);
......@@ -255,6 +263,15 @@ void Connection::slotNewData()
if (m_currentHandler) {
m_currentHandler->failureResponse(QString::fromUtf8(e.type()) + QLatin1String(": ") + QString::fromUtf8(e.what()));
}
} catch (abi::__forced_unwind&) {
// HACK: NPTL throws __forced_unwind during thread cancellation and
// we *must* rethrow it otherwise the program aborts. Due to the issue
// described in #376385 we might end up destroying (cancelling) the
// thread from a nested loop executed inside parseStream() above,
// so the exception raised in there gets caught by this try..catch
// statement and it must be rethrown at all cost. Remove this hack
// once the root problem is fixed.
throw;
} catch (...) {
qCCritical(AKONADISERVER_LOG) << "Unknown exception caught in Connection for session" << m_sessionId;
if (m_currentHandler) {
......@@ -271,6 +288,12 @@ void Connection::slotNewData()
Q_EMIT disconnected();
return;
}
if (m_connectionClosing) {
m_socket->disconnect(this);
QTimer::singleShot(0, this, &Connection::quit);
return;
}
}
// reset, arm the timer
......
......@@ -77,6 +77,7 @@ public Q_SLOTS:
Q_SIGNALS:
void disconnected();
void connectionClosing();
protected Q_SLOTS:
/**
......@@ -114,6 +115,8 @@ protected:
QHash<QString, qint64> m_totalTimeByHandler;
QHash<QString, qint64> m_executionsByHandler;
bool m_connectionClosing;
private:
void sendResponse(qint64 tag, const Protocol::Command &response);
......
......@@ -160,6 +160,7 @@ void DataStore::close()
QueryCache::clear();
m_database.close();
m_database = QSqlDatabase();
m_transactionQueries.clear();
QSqlDatabase::removeDatabase(m_connectionName);
m_dbOpened = false;
......
......@@ -329,6 +329,8 @@ bool ItemRetriever::exec()
}
}
}, Qt::UniqueConnection);
connect(mConnection, &Connection::connectionClosing,
&eventLoop, [&eventLoop]() { eventLoop.exit(1); });
auto it = requests.begin();
while (it != requests.end()) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment