[Digikam-devel] [digikam] [Bug 323888] Face recognition makes digikam fill all the available memory (Qt SQlite plugin relevant)
Gilles Caulier
caulier.gilles at gmail.com
Mon Dec 15 12:55:03 GMT 2014
https://bugs.kde.org/show_bug.cgi?id=323888
--- Comment #101 from Gilles Caulier <caulier.gilles at gmail.com> ---
Git commit b9f8dbfe470609ef31c5442cb2a4c97e02344233 by Marcel Wiesweg.
Committed on 15/11/2014 at 13:45.
Pushed by mwiesweg into branch 'master'.
Rewrite per-thread database connection cleanup
Use QThreadStorage of a per-thread DatabaseThreadData object which is destroyed
when the thread finishes.
M +103 -113 libs/database/core/databasecorebackend.cpp
M +0 -5 libs/database/core/databasecorebackend.h
M +22 -12 libs/database/core/databasecorebackend_p.h
http://commits.kde.org/digikam/b9f8dbfe470609ef31c5442cb2a4c97e02344233
diff --git a/libs/database/core/databasecorebackend.cpp
b/libs/database/core/databasecorebackend.cpp
index c553bb1..76a958a 100644
--- a/libs/database/core/databasecorebackend.cpp
+++ b/libs/database/core/databasecorebackend.cpp
@@ -79,22 +79,64 @@ public:
}
};
+DatabaseThreadData::DatabaseThreadData()
+ : valid(0),
+ transactionCount(0)
+{
+}
+
+DatabaseThreadData::~DatabaseThreadData()
+{
+ if (transactionCount)
+ {
+ kDebug() << "WARNING !!! Transaction count is" << transactionCount <<
"when destroying database!!!";
+ }
+ closeDatabase();
+}
+
+void DatabaseThreadData::closeDatabase()
+{
+ QString connectionToRemove;
+ if (database.isOpen())
+ {
+ connectionToRemove = database.connectionName();
+ }
+
+ // Destroy object
+ database = QSqlDatabase();
+
+ valid = 0;
+ transactionCount = 0;
+ lastError = QSqlError();
+
+ // Remove connection
+ if (!connectionToRemove.isNull())
+ {
+ QSqlDatabase::removeDatabase(connectionToRemove);
+ }
+}
+
DatabaseCoreBackendPrivate::DatabaseCoreBackendPrivate(DatabaseCoreBackend*
const backend)
- : q(backend)
+ : currentValidity(0),
+ isInTransaction(false),
+ status(DatabaseCoreBackend::Unavailable),
+ lock(0),
+ operationStatus(DatabaseCoreBackend::ExecuteNormal),
+ errorLockOperationStatus(DatabaseCoreBackend::ExecuteNormal),
+ errorHandler(0),
+ q(backend)
{
- status = DatabaseCoreBackend::Unavailable;
- isInTransaction = false;
- operationStatus = DatabaseCoreBackend::ExecuteNormal;
- errorHandler = 0;
- lock = 0;
- errorLockOperationStatus = DatabaseCoreBackend::ExecuteNormal;
}
-void DatabaseCoreBackendPrivate::init(const QString& name, DatabaseLocking*
const l)
+DatabaseCoreBackendPrivate::~DatabaseCoreBackendPrivate()
{
- QObject::connect(QCoreApplication::instance(), SIGNAL(aboutToQuit()),
- q, SLOT(slotMainThreadFinished()));
+ // Must be shut down from the main thread.
+ // Clean up the QThreadStorage. It deletes any stored data.
+ threadDataStorage.setLocalData(0);
+}
+void DatabaseCoreBackendPrivate::init(const QString& name, DatabaseLocking*
const l)
+{
backendName = name;
lock = l;
@@ -110,82 +152,43 @@ void DatabaseCoreBackendPrivate::init(const QString&
name, DatabaseLocking* cons
// finishing of the thread.
QSqlDatabase DatabaseCoreBackendPrivate::databaseForThread()
{
- QThread* const thread = QThread::currentThread();
- QSqlDatabase db = threadDatabases[thread];
- int isValid = databasesValid[thread];
-
- if (!isValid || !db.isOpen())
+ DatabaseThreadData* threadData = 0;
+ if (!threadDataStorage.hasLocalData())
{
- // need to open a db for thread
- bool success = open(db);
-
- if (!success)
- {
- kDebug() << "Error while opening the database. Details: [" <<
db.lastError() << "]";
- }
-
- QObject::connect(thread, SIGNAL(finished()),
- q, SLOT(slotThreadFinished()));
+ threadData = new DatabaseThreadData;
+ threadDataStorage.setLocalData(threadData);
}
-
-#ifdef DATABASCOREBACKEND_DEBUG
else
{
- kDebug() << "Database ["<< connectionName(thread) <<"] already open
for thread ["<< thread <<"].";
+ threadData = threadDataStorage.localData();
}
-#endif
-
- return db;
-}
-
-void DatabaseCoreBackendPrivate::closeDatabaseForThread()
-{
- QThread* const thread = QThread::currentThread();
+ // do we need to reopen the database because parameter changed and
validity was increased?
+ if (threadData->valid && threadData->valid < currentValidity)
+ {
+ threadData->closeDatabase();
+ }
- // scope, so that db is destructed when calling removeDatabase
+ if (!threadData->valid || !threadData->database.isOpen())
{
- QSqlDatabase db = threadDatabases[thread];
+ threadData->database = createDatabaseConnection();
- if (db.isValid())
+ if (threadData->database.open())
+ {
+ threadData->valid = currentValidity;
+ }
+ else
{
- db.close();
+ kDebug() << "Error while opening the database. Error was" <<
threadData->database.lastError();
}
}
- threadDatabases.remove(thread);
- databaseErrors.remove(thread);
- databasesValid[thread] = 0;
- transactionCount.remove(thread);
- QSqlDatabase::removeDatabase(connectionName(thread));
-}
-
-QSqlError DatabaseCoreBackendPrivate::databaseErrorForThread()
-{
- QThread* const thread = QThread::currentThread();
- return databaseErrors[thread];
-}
-
-void DatabaseCoreBackendPrivate::setDatabaseErrorForThread(const QSqlError&
lastError)
-{
- QThread* const thread = QThread::currentThread();
- databaseErrors.insert(thread, lastError);
-}
-
-QString DatabaseCoreBackendPrivate::connectionName(QThread* const thread)
-{
- return backendName + QString::number((quintptr)thread);
+ return threadData->database;
}
-bool DatabaseCoreBackendPrivate::open(QSqlDatabase& db)
+QSqlDatabase DatabaseCoreBackendPrivate::createDatabaseConnection()
{
- if (db.isValid())
- {
- db.close();
- }
-
- QThread* const thread = QThread::currentThread();
- db =
QSqlDatabase::addDatabase(parameters.databaseType, connectionName(thread));
+ QSqlDatabase db =
QSqlDatabase::addDatabase(parameters.databaseType, connectionName());
QString connectOptions = parameters.connectOptions;
if (parameters.isSQLite())
@@ -211,46 +214,47 @@ bool DatabaseCoreBackendPrivate::open(QSqlDatabase& db)
db.setUserName(parameters.userName);
db.setPassword(parameters.password);
- bool success = db.open();
+ return db;
+}
- if (success==false)
+void DatabaseCoreBackendPrivate::closeDatabaseForThread()
+{
+ if (threadDataStorage.hasLocalData())
{
- kDebug() << "Error while opening the database. Error was <" <<
db.lastError() << ">";
+ threadDataStorage.localData()->closeDatabase();
}
-
- threadDatabases[thread] = db;
- databasesValid[thread] = 1;
- transactionCount[thread] = 0;
-
- return success;
}
-bool DatabaseCoreBackendPrivate::incrementTransactionCount()
+QSqlError DatabaseCoreBackendPrivate::databaseErrorForThread()
{
- QThread* const thread = QThread::currentThread();
- return (!transactionCount[thread]++);
+ if (threadDataStorage.hasLocalData())
+ {
+ return threadDataStorage.localData()->lastError;
+ }
+ return QSqlError();
}
-bool DatabaseCoreBackendPrivate::decrementTransactionCount()
+void DatabaseCoreBackendPrivate::setDatabaseErrorForThread(const QSqlError&
lastError)
{
- QThread* const thread = QThread::currentThread();
- return (!--transactionCount[thread]);
+ if (threadDataStorage.hasLocalData())
+ {
+ threadDataStorage.localData()->lastError = lastError;
+ }
}
-bool DatabaseCoreBackendPrivate::isInTransactionInOtherThread() const
+QString DatabaseCoreBackendPrivate::connectionName()
{
- QThread* const thread = QThread::currentThread();
- QHash<QThread*, int>::const_iterator it;
+ return backendName + QString::number((quintptr)QThread::currentThread());
+}
- for (it = transactionCount.constBegin(); it !=
transactionCount.constEnd(); ++it)
- {
- if (it.key() != thread && it.value())
- {
- return true;
- }
- }
+bool DatabaseCoreBackendPrivate::incrementTransactionCount()
+{
+ return (!threadDataStorage.localData()->transactionCount++);
+}
- return false;
+bool DatabaseCoreBackendPrivate::decrementTransactionCount()
+{
+ return (!--threadDataStorage.localData()->transactionCount);
}
bool DatabaseCoreBackendPrivate::isInMainThread() const
@@ -740,18 +744,6 @@ void
DatabaseCoreBackend::setDatabaseErrorHandler(DatabaseErrorHandler* const ha
d->errorHandler = handler;
}
-void DatabaseCoreBackend::slotThreadFinished()
-{
- Q_D(DatabaseCoreBackend);
- d->closeDatabaseForThread();
-}
-
-void DatabaseCoreBackend::slotMainThreadFinished()
-{
- Q_D(DatabaseCoreBackend);
- d->closeDatabaseForThread();
-}
-
bool DatabaseCoreBackend::isCompatible(const DatabaseParameters& parameters)
{
return QSqlDatabase::drivers().contains(parameters.databaseType);
@@ -761,10 +753,8 @@ bool DatabaseCoreBackend::open(const DatabaseParameters&
parameters)
{
Q_D(DatabaseCoreBackend);
d->parameters = parameters;
-
- // Force possibly opened thread dbs to re-open with new parameters.
- // They are not accessible from this thread!
- d->databasesValid.clear();
+ // This will make possibly opened thread dbs reload at next access
+ d->currentValidity++;
int retries = 0;
@@ -1634,7 +1624,7 @@ DatabaseCoreBackend::QueryState
DatabaseCoreBackend::commitTransaction()
bool DatabaseCoreBackend::isInTransaction() const
{
Q_D(const DatabaseCoreBackend);
- return d->isInTransactionInOtherThread();
+ return d->isInTransaction;
}
void DatabaseCoreBackend::rollbackTransaction()
diff --git a/libs/database/core/databasecorebackend.h
b/libs/database/core/databasecorebackend.h
index 41e47da..fa4dba4 100644
--- a/libs/database/core/databasecorebackend.h
+++ b/libs/database/core/databasecorebackend.h
@@ -472,11 +472,6 @@ public:
LastInsertId
*/
-private Q_SLOTS:
-
- void slotThreadFinished();
- void slotMainThreadFinished();
-
protected:
DatabaseCoreBackendPrivate* const d_ptr;
diff --git a/libs/database/core/databasecorebackend_p.h
b/libs/database/core/databasecorebackend_p.h
index 2078509..ff3a3fa 100644
--- a/libs/database/core/databasecorebackend_p.h
+++ b/libs/database/core/databasecorebackend_p.h
@@ -29,6 +29,7 @@
#include <QHash>
#include <QSqlDatabase>
#include <QThread>
+#include <QThreadStorage>
#include <QWaitCondition>
// Local includes
@@ -40,25 +41,38 @@
namespace Digikam
{
+class DatabaseThreadData
+{
+public:
+
+ DatabaseThreadData();
+ ~DatabaseThreadData();
+
+ void closeDatabase();
+
+ QSqlDatabase database;
+ int valid;
+ int transactionCount;
+ QSqlError lastError;
+};
+
class DIGIKAM_EXPORT DatabaseCoreBackendPrivate : public DatabaseErrorAnswer
{
public:
explicit DatabaseCoreBackendPrivate(DatabaseCoreBackend* const backend);
- virtual ~DatabaseCoreBackendPrivate()
- {
- }
+ virtual ~DatabaseCoreBackendPrivate();
void init(const QString& connectionName, DatabaseLocking* const locking);
- QString connectionName(QThread* const thread);
+ QString connectionName();
QSqlDatabase databaseForThread();
QSqlError databaseErrorForThread();
void setDatabaseErrorForThread(const QSqlError& lastError);
+ QSqlDatabase createDatabaseConnection();
void closeDatabaseForThread();
- bool open(QSqlDatabase& db);
bool incrementTransactionCount();
bool decrementTransactionCount();
bool isInTransactionInOtherThread() const;
@@ -88,14 +102,10 @@ public:
public:
- // this is always accessed in mutex context, no need for QThreadStorage
- QHash<QThread*, QSqlDatabase> threadDatabases;
- // this is not only db.isValid(), but also "parameters changed, need to
reopen"
- QHash<QThread*, int> databasesValid;
- // for recursive transactions
- QHash<QThread*, int> transactionCount;
+ QThreadStorage<DatabaseThreadData*> threadDataStorage;
- QHash<QThread*, QSqlError> databaseErrors;
+ // This compares to DatabaseThreadData's valid. If currentValidity is
increased and > valid, the db is marked as invalid
+ int currentValidity;
bool isInTransaction;
--
You are receiving this mail because:
You are the assignee for the bug.
More information about the Digikam-devel
mailing list