File 0001-Bookmarks-Runner-Open-and-close-the-databa... of Package plasma5-workspace

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-Bookmarks-Runner-Open-and-close-the-database-connect.patch of Package plasma5-workspace (Revision ee89f52e6dce7ab42f0e347d09c1ded5)

Currently displaying revision ee89f52e6dce7ab42f0e347d09c1ded5 , Show latest

From 194a04dced7f45f92f202c63146149a93da17c92 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Br=C3=BCns?= <stefan.bruens@rwth-aachen.de>
Date: Fri, 14 Sep 2018 00:10:08 +0200
Subject: [PATCH 1/2] [Bookmarks Runner] Open and close the database connection
 in the query thread

According to the documentation, "It is highly recommended that you do not
keep a copy of the QSqlDatabase around as a member of a class, as this
will prevent the instance from being correctly cleaned up on shutdown."

Also, the current database connection should be removed before it is
reused - "QSqlDatabasePrivate::addDatabase: duplicate connection name
'...', old connection removed."

Connections should only be removed if there are no more references to it,
e.g. from a QSqlDatabase instance - "QSqlDatabasePrivate::removeDatabase:
connection '..." is still in use, all queries will cease to work."
Move the db instance in a new scope and remove the connection after the
scope has been left.

As connections can not be moved between threads, the connection handling
has to happen from the query thread. Opening the DB connection on each
query implies some overhead, which is neglegible as it only contributes
about 10% of the bookmark runner runtime and about 0.2% of all
in-process runners combined.
---
 runners/bookmarks/fetchsqlite.cpp | 53 +++++++++++++++++++++----------
 runners/bookmarks/fetchsqlite.h   |  1 -
 2 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/runners/bookmarks/fetchsqlite.cpp b/runners/bookmarks/fetchsqlite.cpp
index acf2f19f..7e495f48 100644
--- a/runners/bookmarks/fetchsqlite.cpp
+++ b/runners/bookmarks/fetchsqlite.cpp
@@ -26,13 +26,12 @@
 #include <QSqlError>
 #include <QSqlRecord>
 #include <QMutexLocker>
+#include <thread>
+#include <sstream>
 
 FetchSqlite::FetchSqlite(const QString &originalFilePath, const QString &copyTo, QObject *parent) :
     QObject(parent), m_databaseFile(copyTo)
 {
-    m_db = QSqlDatabase::addDatabase(QStringLiteral("QSQLITE"), originalFilePath);
-    m_db.setHostName(QStringLiteral("localhost"));
-
     QFile originalFile(originalFilePath);
     QFile(copyTo).remove();
     bool couldCopy = originalFile.copy(copyTo);
@@ -44,33 +43,48 @@ FetchSqlite::FetchSqlite(const QString &originalFilePath, const QString &copyTo,
 
 FetchSqlite::~FetchSqlite()
 {
-    if(m_db.isOpen()) m_db.close();
     QFile(m_databaseFile).remove();
 }
 
 void FetchSqlite::prepare()
 {
-    QMutexLocker lock(&m_mutex);
-    m_db.setDatabaseName(m_databaseFile);
-    bool ok = m_db.open();
-    //qDebug() << "Sqlite Database " << m_databaseFile << " was opened: " << ok;
-    if(!ok) {
-        //qDebug() << "Error: " << m_db.lastError().text();
-    }
 }
 
 void FetchSqlite::teardown()
 {
-    QMutexLocker lock(&m_mutex);
-    m_db.close();
+    const auto connections = QSqlDatabase::connectionNames();
+    for (auto c : connections) {
+        QSqlDatabase::removeDatabase(c);
+    }
+}
+
+static QSqlDatabase openDb(const QString& databaseFile) {
+    auto connection = databaseFile;
+    std::stringstream s{"-"};
+    s << std::this_thread::get_id();
+    connection += QString::fromStdString(s.str());
+
+    auto db = QSqlDatabase::database(connection);
+    if (db.isValid()) {
+        return db;
+    }
+
+    db = QSqlDatabase::addDatabase(QStringLiteral("QSQLITE"), connection);
+    db.setHostName(QStringLiteral("localhost"));
+    db.setDatabaseName(databaseFile);
+    db.open();
+
+    return db;
 }
 
 QList<QVariantMap> FetchSqlite::query(const QString &sql, QMap<QString, QVariant> bindObjects)
 {
     QMutexLocker lock(&m_mutex);
 
+    auto db = openDb(m_databaseFile);
+
     //qDebug() << "query: " << sql;
-    QSqlQuery query(m_db);
+    QSqlQuery query(db);
     query.prepare(sql);
     foreach(const QString &variableName, bindObjects.keys()) {
         query.bindValue(variableName, bindObjects.value(variableName));
@@ -78,7 +92,7 @@ QList<QVariantMap> FetchSqlite::query(const QString &sql, QMap<QString, QVariant
     }
 
     if(!query.exec()) {
-        QSqlError error = m_db.lastError();
+        QSqlError error = db.lastError();
         //qDebug() << "query failed: " << error.text() << " (" << error.type() << ", " << error.number() << ")";
         //qDebug() << query.lastQuery();
     }
@@ -93,12 +107,19 @@ QList<QVariantMap> FetchSqlite::query(const QString &sql, QMap<QString, QVariant
         }
         result << recordValues;
     }
+    db.close();
+
     return result;
 }
 
 QStringList FetchSqlite::tables(QSql::TableType type)
 {
     QMutexLocker lock(&m_mutex);
+    QStringList tables;
+
+    auto db = openDb(m_databaseFile);
+    tables = db.tables(type);
+    db.close();
 
-    return m_db.tables(type);
+    return tables;
 }
diff --git a/runners/bookmarks/fetchsqlite.h b/runners/bookmarks/fetchsqlite.h
index 058d9915..ffe0ce96 100644
--- a/runners/bookmarks/fetchsqlite.h
+++ b/runners/bookmarks/fetchsqlite.h
@@ -46,7 +46,6 @@ public:
 private:
     QString const m_databaseFile;
     QMutex m_mutex;
-    QSqlDatabase m_db;
 };
 
 #endif // FETCHSQLITE_H
-- 
2.19.0

Places

File 0001-Bookmarks-Runner-Open-and-close-the-database-connect.patch of Package plasma5-workspace (Revision ee89f52e6dce7ab42f0e347d09c1ded5)

Places