Reproducible: Sometimes Steps to Reproduce: 1.Go to face management UI. 2.Go to option and check "reset and rebuild recognition data" 3.run Actual Results: recognition.db isn't erase and data are added after existing datas Expected Results: Data are erased, and database is rebuild I've done this a lot of time with expected result. This time, it's don't work as expected. What I have done before : erase some empty folders in collection, with a sql request (not in digikam). erase ImageHaarMatrix data in mysql db Thank you, Eric PS : I have done a lot of facerecognition data rebuild for testing, with always the same tags and face pictures, it's the first time this happen. The recognition database has always the same size as I don't change anything in the picture that are face tagged. Workaround : go in /home/youruser/.kde/share/apps/likface/database, save recognition.db if you want then delete the file. Start or restart digikam (a new empty file with about 13 ko is created) Digikam Version : 4.0.0 beta from git
It still reproducible using last digiKam 4.2.0 ? Gilles Caulier
Same in 4.4.0. Have to delete recognition.db manually.
Git commit f6bcec9d07f0f6530835498370a6042f9b1e1675 by Marcel Wiesweg. Committed on 14/11/2014 at 09:56. Pushed by mwiesweg into branch 'master'. Some fixes in face progress calculation M +1 -0 utilities/facemanagement/facepipeline.cpp M +2 -0 utilities/facemanagement/facepipeline.h M +31 -14 utilities/maintenance/facedetector.cpp http://commits.kde.org/digikam/f6bcec9d07f0f6530835498370a6042f9b1e1675 diff --git a/utilities/facemanagement/facepipeline.cpp b/utilities/facemanagement/facepipeline.cpp index a655385..d512a92 100644 --- a/utilities/facemanagement/facepipeline.cpp +++ b/utilities/facemanagement/facepipeline.cpp @@ -1171,6 +1171,7 @@ void FacePipeline::Private::send(FacePipelineExtendedPackage::Ptr package) { start(); ++totalPackagesAdded; + emit(q->processing(*package)); if (senderFlowControl(package)) { diff --git a/utilities/facemanagement/facepipeline.h b/utilities/facemanagement/facepipeline.h index 4b381ca..45e733a 100644 --- a/utilities/facemanagement/facepipeline.h +++ b/utilities/facemanagement/facepipeline.h @@ -292,6 +292,8 @@ Q_SIGNALS: /// Emitted when processing has started void started(const QString& message); + /// Emitted when one package begins processing + void processing(const FacePipelinePackage& package); /// Emitted when one package has finished processing void processed(const FacePipelinePackage& package); void progressValueChanged(float progress); diff --git a/utilities/maintenance/facedetector.cpp b/utilities/maintenance/facedetector.cpp index 7bd8c0e..222652b 100644 --- a/utilities/maintenance/facedetector.cpp +++ b/utilities/maintenance/facedetector.cpp @@ -96,18 +96,27 @@ class FaceDetector::Private public: Private() + : benchmark(false), + total(0), + progressValue(0), + currentProgressChunk(0), + currentScheduled(0), + currentFinished(0) { - benchmark = false; - total = 0; } - bool benchmark; + bool benchmark; - int total; + int total; - AlbumPointerList<> albumTodoList; - ImageInfoJob albumListing; - FacePipeline pipeline; + AlbumPointerList<> albumTodoList; + ImageInfoJob albumListing; + FacePipeline pipeline; + QMap<Album*,double> relativeProgressValue; + double progressValue; + double currentProgressChunk; + int currentScheduled; + int currentFinished; }; FaceDetector::FaceDetector(const FaceScanSettings& settings, ProgressItem* const parent) @@ -275,25 +284,33 @@ void FaceDetector::slotStart() QApplication::restoreOverrideCursor(); } - d->total = 0; - + // first, we use the relativeProgressValue map to store absolute counts foreach(Album* const album, d->albumTodoList) { if (album->type() == Album::PHYSICAL) { - d->total += palbumCounts.value(album->id()); + d->relativeProgressValue[album] = palbumCounts.value(album->id()); } else // this is possibly broken of course because we do not know if images have multiple tags, // but there's no better solution without expensive operation { - d->total += talbumCounts.value(album->id()); + d->relativeProgressValue[album] = talbumCounts.value(album->id()); } } - - kDebug() << "Total is" << d->total; - + // second, calculate (approximate) overall sum + d->total = 0; + foreach (double count, d->relativeProgressValue) + { + d->total += (int)count; + } d->total = qMax(1, d->total); + kDebug() << "Total is" << d->total; + // third, break absolute to relative values + for (QMap<Album*,double>::iterator it = d->relativeProgressValue.begin(); it != d->relativeProgressValue.end(); ++it) + { + it.value() /= double(d->total); + } setUsesBusyIndicator(false); setTotalItems(d->total);
Git commit 59f0bdb6b2709171354418ed297d4aa1d667ebad by Marcel Wiesweg. Committed on 15/11/2014 at 13:47. Pushed by mwiesweg into branch 'master'. Add database cleanup calls to have a clean shutdown of SQLite data at application termination M +3 -0 app/main/main.cpp M +17 -3 tests/testdatabase.cpp http://commits.kde.org/digikam/59f0bdb6b2709171354418ed297d4aa1d667ebad diff --git a/app/main/main.cpp b/app/main/main.cpp index 3564d23..85fcbc7 100644 --- a/app/main/main.cpp +++ b/app/main/main.cpp @@ -62,6 +62,7 @@ #include "databaseparameters.h" #include "digikamapp.h" #include "scancontroller.h" +#include "thumbnaildatabaseaccess.h" #include "version.h" using namespace Digikam; @@ -235,6 +236,8 @@ int main(int argc, char* argv[]) int ret = app.exec(); + DatabaseAccess::cleanUpDatabase(); + ThumbnailDatabaseAccess::cleanUpDatabase(); KExiv2Iface::KExiv2::cleanupExiv2(); return ret; diff --git a/tests/testdatabase.cpp b/tests/testdatabase.cpp index 10c14ad..0930029 100644 --- a/tests/testdatabase.cpp +++ b/tests/testdatabase.cpp @@ -30,6 +30,7 @@ #include <QSqlDatabase> #include <QDBusConnection> #include <QString> +#include <QTimer> // KDE includes @@ -47,6 +48,7 @@ #include "databaseparameters.h" #include "scancontroller.h" #include "setup.h" +#include "thumbnaildatabaseaccess.h" #include "version.h" namespace Digikam @@ -76,16 +78,28 @@ int main(int argc, char** argv) KCmdLineArgs::init(argc, argv, &aboutData); KApplication app; - DatabaseParameters params = DatabaseParameters::parametersFromConfig(KGlobal::config()); + DatabaseParameters params; + params.databaseType = DatabaseParameters::SQLiteDatabaseType(); + params.setDatabasePath(QDir::currentPath() + "/digikam-test.db"); + params.setThumbsDatabasePath(QDir::currentPath() + "/digikam-thumbs-test.db"); + params.legacyAndDefaultChecks(); QDBusConnection::sessionBus().registerService("org.kde.digikam.startup-" + QString::number(QCoreApplication::instance()->applicationPid())); // initialize database - bool b = AlbumManager::instance()->setDatabase(params, false); + bool b = AlbumManager::instance()->setDatabase(params, false, "/media/fotos/Digikam Sample/"); kDebug() << "Database initialization done: " << b; - + + QTimer::singleShot(500, &app, SLOT(quit())); + app.exec(); + + ScanController::instance()->shutDown(); + + DatabaseAccess::cleanUpDatabase(); + ThumbnailDatabaseAccess::cleanUpDatabase(); + return 0; }
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;
Git commit bab8c3f159930a3b0b31d09b25b9eb42ab1c3c62 by Marcel Wiesweg. Committed on 15/11/2014 at 14:45. Pushed by mwiesweg into branch 'master'. Backport all core db changes regarding thread clean up from main digikam Back and forward port code polish and style changes M +153 -150 libkface/database/databasecorebackend.cpp M +15 -12 libkface/database/databasecorebackend.h M +35 -25 libkface/database/databasecorebackend_p.h M +1 -0 libkface/database/databaseparameters.h http://commits.kde.org/libkface/bab8c3f159930a3b0b31d09b25b9eb42ab1c3c62 diff --git a/libkface/database/databasecorebackend.cpp b/libkface/database/databasecorebackend.cpp index 784e9b7..2193ce2 100644 --- a/libkface/database/databasecorebackend.cpp +++ b/libkface/database/databasecorebackend.cpp @@ -21,12 +21,6 @@ * * ============================================================ */ -/* -#ifndef DATABASCOREBACKEND_DEBUG -#define DATABASCOREBACKEND_DEBUG -#endif -*/ - #include "databasecorebackend.moc" #include "databasecorebackend_p.h" @@ -51,8 +45,8 @@ // Local includes -#include "schemaupdater.h" #include "dbactiontype.h" +#include "schemaupdater.h" namespace KFaceIface { @@ -102,22 +96,65 @@ DatabaseCoreBackendPrivate::ErrorLocker::ErrorLocker(DatabaseCoreBackendPrivate* // ----------------------------------------------------------------------------------------- + +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; - errorLockOperationStatus = DatabaseCoreBackend::ExecuteNormal; - errorHandler = 0; - lock = 0; } -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; @@ -133,84 +170,44 @@ 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()) { - db.close(); + threadData->valid = currentValidity; + } + else + { + 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)); - - QString connectOptions;// = parameters.connectOptions; + QSqlDatabase db = QSqlDatabase::addDatabase(parameters.databaseType, connectionName()); + QString connectOptions = parameters.connectOptions; if (parameters.isSQLite()) { @@ -230,55 +227,48 @@ bool DatabaseCoreBackendPrivate::open(QSqlDatabase& db) db.setDatabaseName(parameters.databaseName); db.setConnectOptions(connectOptions); - /*db.setHostName(parameters.hostName); - db.setPort(parameters.port); - db.setUserName(parameters.userName); - db.setPassword(parameters.password);*/ - bool success = db.open(); + return db; +} - if (success) +void DatabaseCoreBackendPrivate::closeDatabaseForThread() +{ + if (threadDataStorage.hasLocalData()) { - db.exec("PRAGMA synchronous=1;"); + threadDataStorage.localData()->closeDatabase(); } - else +} + +QSqlError DatabaseCoreBackendPrivate::databaseErrorForThread() +{ + if (threadDataStorage.hasLocalData()) { - kDebug() << "Error while opening the database. Error was <" << db.lastError() << ">"; + return threadDataStorage.localData()->lastError; } - - threadDatabases[thread] = db; - databasesValid[thread] = 1; - transactionCount[thread] = 0; - - return success; + return QSqlError(); } -bool DatabaseCoreBackendPrivate::incrementTransactionCount() +void DatabaseCoreBackendPrivate::setDatabaseErrorForThread(const QSqlError& lastError) { - QThread* const thread = QThread::currentThread(); - return !transactionCount[thread]++; + if (threadDataStorage.hasLocalData()) + { + threadDataStorage.localData()->lastError = lastError; + } } -bool DatabaseCoreBackendPrivate::decrementTransactionCount() +QString DatabaseCoreBackendPrivate::connectionName() { - QThread* const thread = QThread::currentThread(); - return !--transactionCount[thread]; + return backendName + QString::number((quintptr)QThread::currentThread()); } -bool DatabaseCoreBackendPrivate::isInTransactionInOtherThread() const +bool DatabaseCoreBackendPrivate::incrementTransactionCount() { - QThread* const thread = QThread::currentThread(); - QHash<QThread*, int>::const_iterator it; - - for (it = transactionCount.constBegin(); it != transactionCount.constEnd(); ++it) - { - if (it.key() != thread && it.value()) - { - return true; - } - } + return (!threadDataStorage.localData()->transactionCount++); +} - return false; +bool DatabaseCoreBackendPrivate::decrementTransactionCount() +{ + return (!--threadDataStorage.localData()->transactionCount); } bool DatabaseCoreBackendPrivate::isInMainThread() const @@ -295,7 +285,7 @@ bool DatabaseCoreBackendPrivate::isInUIThread() const return false; } - return QThread::currentThread() == app->thread(); + return (QThread::currentThread() == app->thread()); } bool DatabaseCoreBackendPrivate::reconnectOnError() const @@ -325,7 +315,8 @@ bool DatabaseCoreBackendPrivate::isConnectionError(const SqlQuery& query) const return false; } - return (query.lastError().type() == QSqlError::ConnectionError || query.lastError().number()==2006); + return query.lastError().type() == QSqlError::ConnectionError || + query.lastError().number() == 2006; } bool DatabaseCoreBackendPrivate::needToConsultUserForError(const SqlQuery&) const @@ -336,12 +327,15 @@ bool DatabaseCoreBackendPrivate::needToConsultUserForError(const SqlQuery&) cons bool DatabaseCoreBackendPrivate::needToHandleWithErrorHandler(const SqlQuery& query) const { - return isConnectionError(query) || needToConsultUserForError(query); + return (isConnectionError(query) || needToConsultUserForError(query)); } bool DatabaseCoreBackendPrivate::checkRetrySQLiteLockError(int retries) { - kDebug() << "Database is locked. Waited" << retries*10; + if (!(retries % 25)) + { + kDebug() << "Database is locked. Waited" << retries*10; + } const int uiMaxRetries = 50; const int maxRetries = 1000; @@ -476,7 +470,7 @@ bool DatabaseCoreBackendPrivate::handleWithErrorHandler(const SqlQuery* const qu } else { - //TODO check if it's better to use an own error handler for kio slaves. + // TODO check if it's better to use an own error handler for kio slaves. // But for now, close only the database in the hope, that the next // access will be successful. closeDatabaseForThread(); @@ -755,18 +749,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); @@ -776,10 +758,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; @@ -913,7 +893,7 @@ DatabaseCoreBackend::QueryState DatabaseCoreBackend::handleQueryResult(SqlQuery& return DatabaseCoreBackend::NoErrors; } -// ---------------------------------------------------------------------------------------------------- +// ------------------------------------------------------------------------------------- DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, QList<QVariant>* const values, QVariant* const lastInsertId) { @@ -922,52 +902,54 @@ DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, const QVariant& boundValue1, - QList<QVariant>* const values, QVariant* const lastInsertId) + QList<QVariant>* const values, QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, boundValue1); return handleQueryResult(query, values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, - const QVariant& boundValue1, const QVariant& boundValue2, - QList<QVariant>* const values, QVariant* const lastInsertId) + const QVariant& boundValue1, const QVariant& boundValue2, + QList<QVariant>* const values, QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, boundValue1, boundValue2); return handleQueryResult(query, values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, - const QVariant& boundValue1, const QVariant& boundValue2, - const QVariant& boundValue3, QList<QVariant>* const values, - QVariant* const lastInsertId) + const QVariant& boundValue1, const QVariant& boundValue2, + const QVariant& boundValue3, QList<QVariant>* const values, + QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, boundValue1, boundValue2, boundValue3); return handleQueryResult(query, values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, - const QVariant& boundValue1, const QVariant& boundValue2, - const QVariant& boundValue3, const QVariant& boundValue4, - QList<QVariant>* const values, QVariant* const lastInsertId) + const QVariant& boundValue1, const QVariant& boundValue2, + const QVariant& boundValue3, const QVariant& boundValue4, + QList<QVariant>* const values, QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, boundValue1, boundValue2, boundValue3, boundValue4); return handleQueryResult(query, values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, const QList<QVariant>& boundValues, - QList<QVariant>* const values, QVariant* const lastInsertId) + QList<QVariant>* const values, QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, boundValues); return handleQueryResult(query, values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, const QMap<QString, QVariant>& bindingMap, - QList<QVariant>* const values, QVariant* const lastInsertId) + QList<QVariant>* const values, QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, bindingMap); return handleQueryResult(query, values, lastInsertId); } +// ------------------------------------------------------------------------------------- + DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(SqlQuery& preparedQuery, QList<QVariant>* const values, QVariant* const lastInsertId) { exec(preparedQuery); @@ -1014,6 +996,8 @@ DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(SqlQuery& preparedQ return handleQueryResult(preparedQuery, values, lastInsertId); } +// ------------------------------------------------------------------------------------- + SqlQuery DatabaseCoreBackend::execQuery(const QString& sql, const QVariant& boundValue1) { SqlQuery query = prepareQuery(sql); @@ -1066,6 +1050,8 @@ SqlQuery DatabaseCoreBackend::execQuery(const QString& sql) return query; } +// ------------------------------------------------------------------------------------- + void DatabaseCoreBackend::execQuery(SqlQuery& query, const QVariant& boundValue1) { query.bindValue(0, boundValue1); @@ -1111,6 +1097,8 @@ void DatabaseCoreBackend::execQuery(SqlQuery& query, const QList<QVariant>& boun exec(query); } +// ------------------------------------------------------------------------------------- + SqlQuery DatabaseCoreBackend::execQuery(const QString& sql, const QMap<QString, QVariant>& bindingMap) { QString preparedString = sql; @@ -1252,7 +1240,7 @@ SqlQuery DatabaseCoreBackend::execQuery(const QString& sql, const QMap<QString, SqlQuery query = prepareQuery(preparedString); - for (int i=0; i<valuesToBind.size(); ++i) + for (int i=0; i < valuesToBind.size(); ++i) { query.bindValue(i, valuesToBind.at(i)); } @@ -1262,12 +1250,12 @@ SqlQuery DatabaseCoreBackend::execQuery(const QString& sql, const QMap<QString, } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execUpsertDBAction(const DatabaseAction& action, const QVariant& id, - const QStringList fieldNames, const QList<QVariant>& values) + const QStringList fieldNames, const QList<QVariant>& values) { QMap<QString, QVariant> parameters; QMap<QString, QVariant> fieldValueMap; - for (int i=0; i<fieldNames.size(); ++i) + for (int i = 0; i < fieldNames.size(); ++i) { fieldValueMap.insert(fieldNames.at(i), values.at(i)); } @@ -1285,7 +1273,7 @@ DatabaseCoreBackend::QueryState DatabaseCoreBackend::execUpsertDBAction(const Da } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execUpsertDBAction(const QString& action, const QVariant& id, - const QStringList fieldNames, const QList<QVariant>& values) + const QStringList fieldNames, const QList<QVariant>& values) { return execUpsertDBAction(getDBAction(action), id, fieldNames, values); } @@ -1516,6 +1504,7 @@ SqlQuery DatabaseCoreBackend::copyQuery(const SqlQuery& old) #endif query.prepare(old.lastQuery()); query.setForwardOnly(old.isForwardOnly()); + // only for positional binding QList<QVariant> boundValues = old.boundValues().values(); @@ -1635,7 +1624,7 @@ DatabaseCoreBackend::QueryState DatabaseCoreBackend::commitTransaction() bool DatabaseCoreBackend::isInTransaction() const { Q_D(const DatabaseCoreBackend); - return d->isInTransactionInOtherThread(); + return d->isInTransaction; } void DatabaseCoreBackend::rollbackTransaction() @@ -1663,4 +1652,18 @@ QString DatabaseCoreBackend::lastError() return d->databaseForThread().lastError().text(); } +int DatabaseCoreBackend::maximumBoundValues() const +{ + Q_D(const DatabaseCoreBackend); + + if (d->parameters.isSQLite()) + { + return 999; // SQLITE_MAX_VARIABLE_NUMBER + } + else + { + return 65535; // MySQL + } +} + } // namespace KFaceIface diff --git a/libkface/database/databasecorebackend.h b/libkface/database/databasecorebackend.h index 9ce859a..a1beac3 100644 --- a/libkface/database/databasecorebackend.h +++ b/libkface/database/databasecorebackend.h @@ -44,6 +44,7 @@ namespace KFaceIface { class DatabaseCoreBackendPrivate; +class DatabaseErrorHandler; class SchemaUpdater; class DatabaseLocking @@ -147,17 +148,18 @@ public: */ void close(); - // ----------------------------------------------------------- +public: class QueryState { public: - QueryState() : value(DatabaseCoreBackend::NoErrors) + QueryState() + : value(DatabaseCoreBackend::NoErrors) { } - QueryState(QueryStateEnum value) + QueryState(const QueryStateEnum value) : value(value) { } @@ -177,6 +179,8 @@ public: QueryStateEnum value; }; +public: + /** * Returns the current status of the database backend */ @@ -275,10 +279,8 @@ public: const QVariant& boundValue1, const QVariant& boundValue2, const QVariant& boundValue3, const QVariant& boundValue4, QList<QVariant>* const values = 0, QVariant* const lastInsertId = 0); - QueryState execSql(const QString& sql, - const QList<QVariant>& boundValues, - QList<QVariant>* const values = 0, - QVariant* const lastInsertId = 0); + QueryState execSql(const QString& sql, const QList<QVariant>& boundValues, + QList<QVariant>* const values = 0, QVariant* const lastInsertId = 0); QueryState execSql(SqlQuery& preparedQuery, QList<QVariant>* const values = 0, QVariant* const lastInsertId = 0); QueryState execSql(SqlQuery& preparedQuery, const QVariant& boundValue1, @@ -438,6 +440,12 @@ public: */ QSqlError lastSQLError(); + /** + * Returns the maximum number of bound parameters allowed per query. + * This value depends on the database engine. + */ + int maximumBoundValues() const; + /* Qt SQL driver supported features SQLITE3: @@ -462,11 +470,6 @@ public: LastInsertId */ -private Q_SLOTS: - - void slotThreadFinished(); - void slotMainThreadFinished(); - protected: DatabaseCoreBackendPrivate* const d_ptr; diff --git a/libkface/database/databasecorebackend_p.h b/libkface/database/databasecorebackend_p.h index c39b33f..27faaa5 100644 --- a/libkface/database/databasecorebackend_p.h +++ b/libkface/database/databasecorebackend_p.h @@ -29,6 +29,7 @@ #include <QHash> #include <QSqlDatabase> #include <QThread> +#include <QThreadStorage> #include <QWaitCondition> // Local includes @@ -38,40 +39,54 @@ namespace KFaceIface { +class DatabaseThreadData +{ +public: + + DatabaseThreadData(); + ~DatabaseThreadData(); + + void closeDatabase(); + + QSqlDatabase database; + int valid; + int transactionCount; + QSqlError lastError; +}; + class DatabaseCoreBackendPrivate : public DatabaseErrorAnswer { public: - DatabaseCoreBackendPrivate(DatabaseCoreBackend* const backend); - virtual ~DatabaseCoreBackendPrivate() {} + explicit DatabaseCoreBackendPrivate(DatabaseCoreBackend* const backend); + 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; bool isInMainThread() const; - bool isInUIThread() const; + bool isInUIThread() const; - bool reconnectOnError() const; - bool isSQLiteLockError(const SqlQuery& query) const; + bool reconnectOnError() const; + bool isSQLiteLockError(const SqlQuery& query) const; bool isSQLiteLockTransactionError(const QSqlError& lastError) const; - bool checkRetrySQLiteLockError(int retries); - bool isConnectionError(const SqlQuery& query) const; - bool needToConsultUserForError(const SqlQuery& query) const; - bool needToHandleWithErrorHandler(const SqlQuery& query) const; - void debugOutputFailedQuery(const QSqlQuery& query) const; - void debugOutputFailedTransaction(const QSqlError& error) const; + bool isConnectionError(const SqlQuery& query) const; + bool needToConsultUserForError(const SqlQuery& query) const; + bool needToHandleWithErrorHandler(const SqlQuery& query) const; + void debugOutputFailedQuery(const QSqlQuery& query) const; + void debugOutputFailedTransaction(const QSqlError& error) const; + bool checkRetrySQLiteLockError(int retries); bool checkOperationStatus(); bool handleWithErrorHandler(const SqlQuery* const query); void setQueryOperationFlag(DatabaseCoreBackend::QueryOperationStatus status); @@ -80,19 +95,14 @@ public: // called by DatabaseErrorHandler, implementing DatabaseErrorAnswer virtual void connectionErrorContinueQueries(); virtual void connectionErrorAbortQueries(); - virtual void transactionFinished(); 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; @@ -121,7 +131,7 @@ public : { public: - AbstractUnlocker(DatabaseCoreBackendPrivate* const d); + explicit AbstractUnlocker(DatabaseCoreBackendPrivate* const d); ~AbstractUnlocker(); void finishAcquire(); @@ -157,7 +167,7 @@ public : { public: - ErrorLocker(DatabaseCoreBackendPrivate* const d); + explicit ErrorLocker(DatabaseCoreBackendPrivate* const d); void wait(); }; @@ -167,7 +177,7 @@ public : { public: - BusyWaiter(DatabaseCoreBackendPrivate* const d); + explicit BusyWaiter(DatabaseCoreBackendPrivate* const d); }; public : diff --git a/libkface/database/databaseparameters.h b/libkface/database/databaseparameters.h index 655b87a..7cf84ec 100644 --- a/libkface/database/databaseparameters.h +++ b/libkface/database/databaseparameters.h @@ -52,6 +52,7 @@ public: QString databaseType; QString databaseName; + QString connectOptions; bool operator==(const DatabaseParameters& other) const; bool operator!=(const DatabaseParameters& other) const;
Git commit 0fb2b35c650b20f7c208bb5b07bd701fd4d09d1f by Marcel Wiesweg. Committed on 15/11/2014 at 14:46. Pushed by mwiesweg into branch 'master'. Back and forward port code polish and style changes from libkface db backend. The files have now only the needed minor differences M +140 -136 libs/database/core/databasecorebackend.cpp M +6 -9 libs/database/core/databasecorebackend.h M +4 -5 libs/database/core/databasecorebackend_p.h http://commits.kde.org/digikam/0fb2b35c650b20f7c208bb5b07bd701fd4d09d1f diff --git a/libs/database/core/databasecorebackend.cpp b/libs/database/core/databasecorebackend.cpp index 76a958a..480f437 100644 --- a/libs/database/core/databasecorebackend.cpp +++ b/libs/database/core/databasecorebackend.cpp @@ -6,7 +6,7 @@ * Date : 2007-04-15 * Description : Abstract database backend * - * Copyright (C) 2007-2010 by Marcel Wiesweg <marcel dot wiesweg at gmx dot de> + * Copyright (C) 2007-2012 by Marcel Wiesweg <marcel dot wiesweg at gmx dot de> * * This program is free software; you can redistribute it * and/or modify it under the terms of the GNU General @@ -42,22 +42,23 @@ // KDE includes #include <kdebug.h> -#include <kglobal.h> // Local includes -#include "thumbnailschemaupdater.h" #include "dbactiontype.h" +#include "thumbnailschemaupdater.h" -//#define DATABASCOREBACKEND_DEBUG 1 namespace Digikam { DatabaseLocking::DatabaseLocking() - : mutex(QMutex::Recursive), lockCount(0) // create a recursive mutex + : mutex(QMutex::Recursive), + lockCount(0) // create a recursive mutex { } +// ----------------------------------------------------------------------------------------- + // For whatever reason, these methods are "static protected" class sotoSleep : public QThread { @@ -79,6 +80,23 @@ public: } }; +// ----------------------------------------------------------------------------------------- + +DatabaseCoreBackendPrivate::BusyWaiter::BusyWaiter(DatabaseCoreBackendPrivate* const d) + : AbstractWaitingUnlocker(d, &d->busyWaitMutex, &d->busyWaitCondVar) +{ +} + +// ----------------------------------------------------------------------------------------- + +DatabaseCoreBackendPrivate::ErrorLocker::ErrorLocker(DatabaseCoreBackendPrivate* const d) + : AbstractWaitingUnlocker(d, &d->errorLockMutex, &d->errorLockCondVar) +{ +} + +// ----------------------------------------------------------------------------------------- + + DatabaseThreadData::DatabaseThreadData() : valid(0), transactionCount(0) @@ -288,7 +306,7 @@ bool DatabaseCoreBackendPrivate::isSQLiteLockError(const SqlQuery& query) const bool DatabaseCoreBackendPrivate::isSQLiteLockTransactionError(const QSqlError& lastError) const { return parameters.isSQLite() && - lastError.type() == QSqlError::TransactionError && + lastError.type() == QSqlError::TransactionError && lastError.databaseText() == QLatin1String("database is locked"); // wouldnt it be great if they gave us the database error number... } @@ -356,74 +374,6 @@ void DatabaseCoreBackendPrivate::debugOutputFailedTransaction(const QSqlError& e << error.number() << error.type(); } - -DatabaseCoreBackendPrivate::AbstractUnlocker::AbstractUnlocker(DatabaseCoreBackendPrivate* const d) - : count(0), d(d) -{ - // Why two mutexes? The main mutex is recursive and won't work with a condvar. - - // acquire lock - d->lock->mutex.lock(); - // store lock count - count = d->lock->lockCount; - // set lock count to 0 - d->lock->lockCount = 0; - - // unlock - for (int i=0; i<count; ++i) - { - d->lock->mutex.unlock(); - } -} - -void DatabaseCoreBackendPrivate::AbstractUnlocker::finishAcquire() -{ - // drop lock acquired in first line. Main mutex is now free. - // We maintain lock order (first main mutex, second error lock mutex) - // but we drop main mutex lock for waiting on the cond var. - d->lock->mutex.unlock(); -} - -DatabaseCoreBackendPrivate::AbstractUnlocker::~AbstractUnlocker() -{ - // lock main mutex as often as it was locked before - for (int i=0; i<count; ++i) - { - d->lock->mutex.lock(); - } - - // update lock count - d->lock->lockCount += count; -} - -DatabaseCoreBackendPrivate::AbstractWaitingUnlocker::AbstractWaitingUnlocker(DatabaseCoreBackendPrivate* const d, - QMutex* const mutex, QWaitCondition* const condVar) - : AbstractUnlocker(d), mutex(mutex), condVar(condVar) -{ - // Why two mutexes? The main mutex is recursive and won't work with a condvar. - // lock condvar mutex (lock only if main mutex is locked) - mutex->lock(); - - finishAcquire(); -} - -DatabaseCoreBackendPrivate::AbstractWaitingUnlocker::~AbstractWaitingUnlocker() -{ - // unlock condvar mutex. Both mutexes are now free. - mutex->unlock(); - // now base class destructor is executed, reallocating main mutex -} - -bool DatabaseCoreBackendPrivate::AbstractWaitingUnlocker::wait(unsigned long time) -{ - return condVar->wait(mutex, time); -} - -DatabaseCoreBackendPrivate::BusyWaiter::BusyWaiter(DatabaseCoreBackendPrivate* const d) - : AbstractWaitingUnlocker(d, &d->busyWaitMutex, &d->busyWaitCondVar) -{ -} - void DatabaseCoreBackendPrivate::transactionFinished() { // wakes up any BusyWaiter waiting on the busyWaitCondVar. @@ -431,39 +381,18 @@ void DatabaseCoreBackendPrivate::transactionFinished() busyWaitCondVar.wakeOne(); } -DatabaseCoreBackendPrivate::ErrorLocker::ErrorLocker(DatabaseCoreBackendPrivate* const d) - : AbstractWaitingUnlocker(d, &d->errorLockMutex, &d->errorLockCondVar) -{ -} - -/** This suspends the current thread if the query status as - * set by setFlag() is Wait and until the thread is woken with wakeAll(). - * The DatabaseAccess mutex will be unlocked while waiting. - */ -void DatabaseCoreBackendPrivate::ErrorLocker::wait() -{ - // we use a copy of the flag under lock of the errorLockMutex to be able to check it here - while (d->errorLockOperationStatus == DatabaseCoreBackend::Wait) - { - wait(); - } -} - -/** Set the wait flag to queryStatus. Typically, call this with Wait. - */ +/** Set the wait flag to queryStatus. Typically, call this with Wait. */ void DatabaseCoreBackendPrivate::setQueryOperationFlag(DatabaseCoreBackend::QueryOperationStatus status) { // Enforce lock order (first main mutex, second error lock mutex) QMutexLocker l(&errorLockMutex); - // this change must be done under errorLockMutex lock errorLockOperationStatus = status; operationStatus = status; } /** Set the wait flag to queryStatus and wake all waiting threads. - * Typically, call wakeAll with status ExecuteNormal or AbortQueries. - */ + * Typically, call wakeAll with status ExecuteNormal or AbortQueries. */ void DatabaseCoreBackendPrivate::queryOperationWakeAll(DatabaseCoreBackend::QueryOperationStatus status) { QMutexLocker l(&errorLockMutex); @@ -530,7 +459,7 @@ bool DatabaseCoreBackendPrivate::handleWithErrorHandler(const SqlQuery* const qu } else { - kError() << "Failed to invoke DatabaseErrorHandler. Aborting all queries."; + kWarning() << "Failed to invoke DatabaseErrorHandler. Aborting all queries."; operationStatus = DatabaseCoreBackend::AbortQueries; } @@ -568,7 +497,88 @@ void DatabaseCoreBackendPrivate::connectionErrorAbortQueries() queryOperationWakeAll(DatabaseCoreBackend::AbortQueries); } -// ----------------------------------------------------------------------------------------------- +// ----------------------------------------------------------------------------------------- + +DatabaseCoreBackendPrivate::AbstractUnlocker::AbstractUnlocker(DatabaseCoreBackendPrivate* const d) + : count(0), d(d) +{ + // Why two mutexes? The main mutex is recursive and won't work with a condvar. + + // acquire lock + d->lock->mutex.lock(); + // store lock count + count = d->lock->lockCount; + // set lock count to 0 + d->lock->lockCount = 0; + + // unlock + for (int i=0; i<count; ++i) + { + d->lock->mutex.unlock(); + } +} + +void DatabaseCoreBackendPrivate::AbstractUnlocker::finishAcquire() +{ + // drop lock acquired in first line. Main mutex is now free. + // We maintain lock order (first main mutex, second error lock mutex) + // but we drop main mutex lock for waiting on the cond var. + d->lock->mutex.unlock(); +} + +DatabaseCoreBackendPrivate::AbstractUnlocker::~AbstractUnlocker() +{ + // lock main mutex as often as it was locked before + for (int i=0; i<count; ++i) + { + d->lock->mutex.lock(); + } + + // update lock count + d->lock->lockCount += count; +} + +// ----------------------------------------------------------------------------------------- + +DatabaseCoreBackendPrivate::AbstractWaitingUnlocker::AbstractWaitingUnlocker(DatabaseCoreBackendPrivate* const d, + QMutex* const mutex, QWaitCondition* const condVar) + : AbstractUnlocker(d), mutex(mutex), condVar(condVar) +{ + // Why two mutexes? The main mutex is recursive and won't work with a condvar. + // lock condvar mutex (lock only if main mutex is locked) + mutex->lock(); + + finishAcquire(); +} + +DatabaseCoreBackendPrivate::AbstractWaitingUnlocker::~AbstractWaitingUnlocker() +{ + // unlock condvar mutex. Both mutexes are now free. + mutex->unlock(); + // now base class destructor is executed, reallocating main mutex +} + +bool DatabaseCoreBackendPrivate::AbstractWaitingUnlocker::wait(unsigned long time) +{ + return condVar->wait(mutex, time); +} + +// ----------------------------------------------------------------------------------------- + +/** This suspends the current thread if the query status as + * set by setFlag() is Wait and until the thread is woken with wakeAll(). + * The DatabaseAccess mutex will be unlocked while waiting. + */ +void DatabaseCoreBackendPrivate::ErrorLocker::wait() +{ + // we use a copy of the flag under lock of the errorLockMutex to be able to check it here + while (d->errorLockOperationStatus == DatabaseCoreBackend::Wait) + { + wait(); + } +} + +// ----------------------------------------------------------------------------------------- DatabaseCoreBackend::DatabaseCoreBackend(const QString& backendName, DatabaseLocking* const locking) : d_ptr(new DatabaseCoreBackendPrivate(this)) @@ -602,33 +612,32 @@ DatabaseAction DatabaseCoreBackend::getDBAction(const QString& actionName) const if (action.name.isNull()) { - kError() << "No DB action defined for" << actionName << "! Implementation missing for this database type."; + kWarning() << "No DB action defined for" << actionName << "! Implementation missing for this database type."; } return action; } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execDBAction(const DatabaseAction& action, QList<QVariant>* const values, - QVariant* const lastInsertId) + QVariant* const lastInsertId) { return execDBAction(action, QMap<QString, QVariant>(), values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execDBAction(const QString& action, QList<QVariant>* const values, - QVariant* const lastInsertId) + QVariant* const lastInsertId) { return execDBAction(getDBAction(action), QMap<QString, QVariant>(), values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execDBAction(const QString& action, const QMap<QString, QVariant>& bindingMap, - QList<QVariant>* const values, QVariant* const lastInsertId) + QList<QVariant>* const values, QVariant* const lastInsertId) { return execDBAction(getDBAction(action), bindingMap, values, lastInsertId); } -DatabaseCoreBackend::QueryState DatabaseCoreBackend::execDBAction(const DatabaseAction& action, const QMap<QString, - QVariant>& bindingMap, QList<QVariant>* const values, - QVariant* const lastInsertId) +DatabaseCoreBackend::QueryState DatabaseCoreBackend::execDBAction(const DatabaseAction& action, const QMap<QString, QVariant>& bindingMap, + QList<QVariant>* const values, QVariant* const lastInsertId) { Q_D(DatabaseCoreBackend); @@ -637,7 +646,7 @@ DatabaseCoreBackend::QueryState DatabaseCoreBackend::execDBAction(const Database if (action.name.isNull()) { - kError() << "Attempt to execute null action"; + kWarning() << "Attempt to execute null action"; return DatabaseCoreBackend::SQLError; } @@ -726,8 +735,8 @@ QSqlQuery DatabaseCoreBackend::execDBActionQuery(const DatabaseAction& action, c if (result.lastError().isValid() && result.lastError().number()) { - kDebug() << "Error while executing DBAction ["<< action.name - <<"] Statement ["<<actionElement.statement<<"] Errornr. [" << result.lastError() << "]"; + kDebug() << "Error while executing DBAction [" << action.name + << "] Statement [" << actionElement.statement << "] Errornr. [" << result.lastError() << "]"; break; } } @@ -822,7 +831,7 @@ DatabaseCoreBackend::Status DatabaseCoreBackend::status() const } /* -bool DatabaseCoreBackend::execSql(const QString& sql, QStringList* values) +bool DatabaseCoreBackend::execSql(const QString& sql, QStringList* const values) { QSqlQuery query = execQuery(sql); @@ -865,8 +874,7 @@ QList<QVariant> DatabaseCoreBackend::readToList(SqlQuery& query) return list; } -DatabaseCoreBackend::QueryState DatabaseCoreBackend::handleQueryResult(SqlQuery& query, QList<QVariant>* const values, - QVariant* const lastInsertId) +DatabaseCoreBackend::QueryState DatabaseCoreBackend::handleQueryResult(SqlQuery& query, QList<QVariant>* const values, QVariant* const lastInsertId) { if (!query.isActive()) { @@ -891,57 +899,54 @@ DatabaseCoreBackend::QueryState DatabaseCoreBackend::handleQueryResult(SqlQuery& // ------------------------------------------------------------------------------------- -DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, QList<QVariant>* const values, - QVariant* const lastInsertId) +DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, QList<QVariant>* const values, QVariant* const lastInsertId) { SqlQuery query = execQuery(sql); return handleQueryResult(query, values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, const QVariant& boundValue1, - QList<QVariant>* values, QVariant* lastInsertId) + QList<QVariant>* const values, QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, boundValue1); return handleQueryResult(query, values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, - const QVariant& boundValue1, const QVariant& boundValue2, - QList<QVariant>* const values, QVariant* const lastInsertId) + const QVariant& boundValue1, const QVariant& boundValue2, + QList<QVariant>* const values, QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, boundValue1, boundValue2); return handleQueryResult(query, values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, - const QVariant& boundValue1, const QVariant& boundValue2, - const QVariant& boundValue3, QList<QVariant>* const values, - QVariant* const lastInsertId) + const QVariant& boundValue1, const QVariant& boundValue2, + const QVariant& boundValue3, QList<QVariant>* const values, + QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, boundValue1, boundValue2, boundValue3); return handleQueryResult(query, values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, - const QVariant& boundValue1, const QVariant& boundValue2, - const QVariant& boundValue3, const QVariant& boundValue4, - QList<QVariant>* const values, QVariant* const lastInsertId) + const QVariant& boundValue1, const QVariant& boundValue2, + const QVariant& boundValue3, const QVariant& boundValue4, + QList<QVariant>* const values, QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, boundValue1, boundValue2, boundValue3, boundValue4); return handleQueryResult(query, values, lastInsertId); } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, const QList<QVariant>& boundValues, - QList<QVariant>* const values, QVariant* const lastInsertId) + QList<QVariant>* const values, QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, boundValues); return handleQueryResult(query, values, lastInsertId); } -// ------------------------------------------------------------------------------------- - DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, const QMap<QString, QVariant>& bindingMap, - QList<QVariant>* const values, QVariant* const lastInsertId) + QList<QVariant>* const values, QVariant* const lastInsertId) { SqlQuery query = execQuery(sql, bindingMap); return handleQueryResult(query, values, lastInsertId); @@ -949,8 +954,7 @@ DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(const QString& sql, // ------------------------------------------------------------------------------------- -DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(SqlQuery& preparedQuery, - QList<QVariant>* const values, QVariant* const lastInsertId) +DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(SqlQuery& preparedQuery, QList<QVariant>* const values, QVariant* const lastInsertId) { exec(preparedQuery); return handleQueryResult(preparedQuery, values, lastInsertId); @@ -996,7 +1000,7 @@ DatabaseCoreBackend::QueryState DatabaseCoreBackend::execSql(SqlQuery& preparedQ return handleQueryResult(preparedQuery, values, lastInsertId); } -// ---------------------------------------------------------------------------------------- +// ------------------------------------------------------------------------------------- SqlQuery DatabaseCoreBackend::execQuery(const QString& sql, const QVariant& boundValue1) { @@ -1119,9 +1123,9 @@ SqlQuery DatabaseCoreBackend::execQuery(const QString& sql, const QMap<QString, if (!bindingMap.contains(namedPlaceholder)) { - kError() << "Missing place holder" << namedPlaceholder - << "in binding map. The following values are defined for this action:" - << bindingMap.keys() <<". This is a setup error!"; + kWarning() << "Missing place holder" << namedPlaceholder + << "in binding map. The following values are defined for this action:" + << bindingMap.keys() <<". This is a setup error!"; //TODO What should we do here? How can we cancel that action? } @@ -1240,7 +1244,7 @@ SqlQuery DatabaseCoreBackend::execQuery(const QString& sql, const QMap<QString, SqlQuery query = prepareQuery(preparedString); - for (int i=0; i<valuesToBind.size(); ++i) + for (int i=0; i < valuesToBind.size(); ++i) { query.bindValue(i, valuesToBind.at(i)); } @@ -1250,7 +1254,7 @@ SqlQuery DatabaseCoreBackend::execQuery(const QString& sql, const QMap<QString, } DatabaseCoreBackend::QueryState DatabaseCoreBackend::execUpsertDBAction(const DatabaseAction& action, const QVariant& id, - const QStringList fieldNames, const QList<QVariant>& values) + const QStringList fieldNames, const QList<QVariant>& values) { QMap<QString, QVariant> parameters; QMap<QString, QVariant> fieldValueMap; @@ -1266,7 +1270,7 @@ DatabaseCoreBackend::QueryState DatabaseCoreBackend::execUpsertDBAction(const Da parameters.insert(":id", id); parameters.insert(":fieldValueList", qVariantFromValue(fieldValueList)); - parameters.insert(":fieldList", qVariantFromValue (fieldList)); + parameters.insert(":fieldList", qVariantFromValue(fieldList)); parameters.insert(":valueList", qVariantFromValue(valueList)); return execDBAction(action, parameters); diff --git a/libs/database/core/databasecorebackend.h b/libs/database/core/databasecorebackend.h index fa4dba4..ea8d558 100644 --- a/libs/database/core/databasecorebackend.h +++ b/libs/database/core/databasecorebackend.h @@ -42,15 +42,12 @@ namespace Digikam { -class ThumbnailSchemaUpdater; -class DatabaseErrorHandler; class DatabaseCoreBackendPrivate; - -// ------------------------------------------------------------------------------------------------------------ +class DatabaseErrorHandler; +class ThumbnailSchemaUpdater; class DIGIKAM_EXPORT DatabaseLocking { - public: DatabaseLocking(); @@ -61,7 +58,7 @@ public: int lockCount; }; -// ------------------------------------------------------------------------------------------------------------ +// ----------------------------------------------------------------- class DIGIKAM_EXPORT DatabaseCoreBackend : public QObject { @@ -202,7 +199,7 @@ public: * Add a DatabaseErrorHandler. This object must be created in the main thread. * If a database error occurs, this object can handle problem solving and user interaction. */ - void setDatabaseErrorHandler(DatabaseErrorHandler* handler); + void setDatabaseErrorHandler(DatabaseErrorHandler* const handler); /** * Return config read from XML, @@ -265,10 +262,10 @@ public: * Executes the SQL statement, and write the returned data into the values list. * If you are not interested in the returned data, set values to 0. * Methods are provided for up to four bound values (positional binding), or for a list of bound values. - * If you want the last inserted id (and your query is suitable), sett lastInsertId to the address of a QVariant. + * If you want the last inserted id (and your query is suitable), set lastInsertId to the address of a QVariant. * Additionally, methods are provided for prepared statements. */ - QueryState execSql(const QString& sql, QList<QVariant>* values = 0, QVariant* const lastInsertId = 0); + QueryState execSql(const QString& sql, QList<QVariant>* const values = 0, QVariant* const lastInsertId = 0); QueryState execSql(const QString& sql, const QVariant& boundValue1, QList<QVariant>* const values = 0, QVariant* const lastInsertId = 0); QueryState execSql(const QString& sql, diff --git a/libs/database/core/databasecorebackend_p.h b/libs/database/core/databasecorebackend_p.h index ff3a3fa..1f60dd7 100644 --- a/libs/database/core/databasecorebackend_p.h +++ b/libs/database/core/databasecorebackend_p.h @@ -75,7 +75,6 @@ public: void closeDatabaseForThread(); bool incrementTransactionCount(); bool decrementTransactionCount(); - bool isInTransactionInOtherThread() const; bool isInMainThread() const; bool isInUIThread() const; @@ -145,10 +144,10 @@ public : DatabaseCoreBackendPrivate* const d; }; - // ---------------------------------------------------------------------- - friend class AbstractUnlocker; + // ------------------------------------------------------------------ + class AbstractWaitingUnlocker : public AbstractUnlocker { public: @@ -164,7 +163,7 @@ public : QWaitCondition* const condVar; }; - // ---------------------------------------------------------------------- + // ------------------------------------------------------------------ class ErrorLocker : public AbstractWaitingUnlocker { @@ -174,7 +173,7 @@ public : void wait(); }; - // ---------------------------------------------------------------------- + // ------------------------------------------------------------------ class BusyWaiter : public AbstractWaitingUnlocker {
Next digiKam release 4.6.0 will include several important commits from Marcel to try to fix this memory leak with SQlite database Gilles Caulier
Git commit 923314040b52f1923b084a04d45e215348829db2 by Maik Qualmann. Committed on 24/03/2015 at 22:07. Pushed by mqualmann into branch 'master'. clear all training data in the database before a rebuild begins FIXED-IN: 4.9.0 M +2 -1 NEWS M +2 -0 utilities/maintenance/facedetector.cpp http://commits.kde.org/digikam/923314040b52f1923b084a04d45e215348829db2
Git commit d04264c296b30c5b69a7df527934f6e925e53d74 by Gilles Caulier. Committed on 17/03/2015 at 05:03. Pushed by cgilles into branch 'frameworks'. backport commit #923314040b52f1923b084a04d45e215348829db2 from git/master to frameworks branch M +2 -0 utilities/maintenance/facesdetector.cpp http://commits.kde.org/digikam/d04264c296b30c5b69a7df527934f6e925e53d74