Remove threading code from dicom database and indexer

See discussion at #292.

Pending a re-implementation, remove thread support from the
ctkDICOMDatabase code.  Since the ctkDICOMDatabase internally
manages the database connection, the threading code should make
multiple instances of the database (one per thread) rather than
calling the same instance from multiple threads.  Because of this
there is no reason to have a mutex internal to the class.
Related call, methods, and member variables have been removed.

Libs/DICOM/Core/Testing/Cpp/ctkDICOMIndexerTest1.cpp

@@ -54,8 +54,5 @@ int ctkDICOMIndexerTest1( int argc, char * argv [] )
   // make sure it doesn't crash
   indexer.refreshDatabase(database, QDir::tempPath());
 
-  // ensure all concurrent inserts are complete
-  indexer.waitForImportFinished();
-
   return EXIT_SUCCESS;
 }

Libs/DICOM/Core/ctkDICOMDatabase.cpp

@@ -23,11 +23,9 @@
 // Qt includes
 #include <QDate>
 #include <QDebug>
-#include <QDirIterator>
 #include <QFile>
 #include <QFileInfo>
 #include <QFileSystemWatcher>
-#include <QMutexLocker>
 #include <QSet>
 #include <QSqlError>
 #include <QSqlQuery>
@@ -131,10 +129,6 @@ public:
   /// resets the variables to new inserts won't be fooled by leftover values
   void resetLastInsertedValues();
 
-  /// parallel accesses are not allowed
-  /// - http://qt-project.org/doc/qt-4.8/threads-modules.html#threads-and-the-sql-module
-  QMutex databaseAccessMutex;
-
   /// tagCache table has been checked to exist
   bool TagCacheVerified;
   /// tag cache has independent database to avoid locking issue
@@ -211,8 +205,6 @@ bool ctkDICOMDatabasePrivate::loggedExec(QSqlQuery& query)
 //------------------------------------------------------------------------------
 bool ctkDICOMDatabasePrivate::loggedExec(QSqlQuery& query, const QString& queryString)
 {
-  QMutexLocker lock(&(this->databaseAccessMutex));
-
   bool success;
   if (queryString.compare(""))
     {
@@ -242,7 +234,6 @@ bool ctkDICOMDatabasePrivate::loggedExec(QSqlQuery& query, const QString& queryS
 void ctkDICOMDatabasePrivate::beginTransaction()
 {
   Q_Q(ctkDICOMDatabase);
-  QMutexLocker lock(&(this->databaseAccessMutex));
 
   QSqlQuery transaction( this->Database );
   transaction.prepare( "BEGIN TRANSACTION" );
@@ -253,7 +244,6 @@ void ctkDICOMDatabasePrivate::beginTransaction()
 void ctkDICOMDatabasePrivate::endTransaction()
 {
   Q_Q(ctkDICOMDatabase);
-  QMutexLocker lock(&(this->databaseAccessMutex));
 
   QSqlQuery transaction( this->Database );
   transaction.prepare( "END TRANSACTION" );
@@ -307,7 +297,6 @@ void ctkDICOMDatabase::openDatabase(const QString databaseFile, const QString& c
 
   //Disable synchronous writing to make modifications faster
   {
-  QMutexLocker lock(&(d->databaseAccessMutex));
   QSqlQuery pragmaSyncQuery(d->Database);
   pragmaSyncQuery.exec("PRAGMA synchronous = OFF");
   pragmaSyncQuery.finish();
@@ -394,7 +383,6 @@ ctkDICOMAbstractThumbnailGenerator* ctkDICOMDatabase::thumbnailGenerator(){
 
 //------------------------------------------------------------------------------
 bool ctkDICOMDatabasePrivate::executeScript(const QString script) {
-  QMutexLocker lock(&(this->databaseAccessMutex));
   QFile scriptFile(script);
   scriptFile.open(QIODevice::ReadOnly);
   if  ( !scriptFile.isOpen() )
@@ -556,7 +544,6 @@ void ctkDICOMDatabase::closeDatabase()
 QStringList ctkDICOMDatabase::patients()
 {
   Q_D(ctkDICOMDatabase);
-  QMutexLocker lock(&(d->databaseAccessMutex));
   QSqlQuery query(d->Database);
   query.prepare ( "SELECT UID FROM Patients" );
   query.exec();
@@ -572,7 +559,6 @@ QStringList ctkDICOMDatabase::patients()
 QStringList ctkDICOMDatabase::studiesForPatient(QString dbPatientID)
 {
   Q_D(ctkDICOMDatabase);
-  QMutexLocker lock(&(d->databaseAccessMutex));
   QSqlQuery query(d->Database);
   query.prepare ( "SELECT StudyInstanceUID FROM Studies WHERE PatientsUID = ?" );
   query.bindValue ( 0, dbPatientID );
@@ -589,7 +575,6 @@ QStringList ctkDICOMDatabase::studiesForPatient(QString dbPatientID)
 QStringList ctkDICOMDatabase::seriesForStudy(QString studyUID)
 {
   Q_D(ctkDICOMDatabase);
-  QMutexLocker lock(&(d->databaseAccessMutex));
   QSqlQuery query(d->Database);
   query.prepare ( "SELECT SeriesInstanceUID FROM Series WHERE StudyInstanceUID=?");
   query.bindValue ( 0, studyUID );
@@ -606,7 +591,6 @@ QStringList ctkDICOMDatabase::seriesForStudy(QString studyUID)
 QStringList ctkDICOMDatabase::filesForSeries(QString seriesUID)
 {
   Q_D(ctkDICOMDatabase);
-  QMutexLocker lock(&(d->databaseAccessMutex));
   QSqlQuery query(d->Database);
   query.prepare ( "SELECT Filename FROM Images WHERE SeriesInstanceUID=?");
   query.bindValue ( 0, seriesUID );
@@ -623,7 +607,6 @@ QStringList ctkDICOMDatabase::filesForSeries(QString seriesUID)
 QString ctkDICOMDatabase::fileForInstance(QString sopInstanceUID)
 {
   Q_D(ctkDICOMDatabase);
-  QMutexLocker lock(&(d->databaseAccessMutex));
   QSqlQuery query(d->Database);
   query.prepare ( "SELECT Filename FROM Images WHERE SOPInstanceUID=?");
   query.bindValue ( 0, sopInstanceUID );
@@ -640,7 +623,6 @@ QString ctkDICOMDatabase::fileForInstance(QString sopInstanceUID)
 QString ctkDICOMDatabase::instanceForFile(QString fileName)
 {
   Q_D(ctkDICOMDatabase);
-  QMutexLocker lock(&(d->databaseAccessMutex));
   QSqlQuery query(d->Database);
   query.prepare ( "SELECT SOPInstanceUID FROM Images WHERE Filename=?");
   query.bindValue ( 0, fileName );
@@ -657,7 +639,6 @@ QString ctkDICOMDatabase::instanceForFile(QString fileName)
 QDateTime ctkDICOMDatabase::insertDateTimeForInstance(QString sopInstanceUID)
 {
   Q_D(ctkDICOMDatabase);
-  QMutexLocker lock(&(d->databaseAccessMutex));
   QSqlQuery query(d->Database);
   query.prepare ( "SELECT InsertTimestamp FROM Images WHERE SOPInstanceUID=?");
   query.bindValue ( 0, sopInstanceUID );
@@ -972,7 +953,6 @@ int ctkDICOMDatabasePrivate::insertPatient(const ctkDICOMDataset& ctkDataset)
 //------------------------------------------------------------------------------
 void ctkDICOMDatabasePrivate::insertStudy(const ctkDICOMDataset& ctkDataset, int dbPatientID)
 {
-  QMutexLocker lock(&(this->databaseAccessMutex));
   QString studyInstanceUID(ctkDataset.GetElementAsString(DCM_StudyInstanceUID) );
   QSqlQuery checkStudyExistsQuery (Database);
   checkStudyExistsQuery.prepare ( "SELECT * FROM Studies WHERE StudyInstanceUID = ?" );
@@ -1023,7 +1003,6 @@ void ctkDICOMDatabasePrivate::insertStudy(const ctkDICOMDataset& ctkDataset, int
 //------------------------------------------------------------------------------
 void ctkDICOMDatabasePrivate::insertSeries(const ctkDICOMDataset& ctkDataset, QString studyInstanceUID)
 {
-  QMutexLocker lock(&(this->databaseAccessMutex));
   QString seriesInstanceUID(ctkDataset.GetElementAsString(DCM_SeriesInstanceUID) );
   QSqlQuery checkSeriesExistsQuery (Database);
   checkSeriesExistsQuery.prepare ( "SELECT * FROM Series WHERE SeriesInstanceUID = ?" );
@@ -1139,7 +1118,6 @@ void ctkDICOMDatabasePrivate::insert( const ctkDICOMDataset& ctkDataset, const Q
   fileExists.prepare("SELECT InsertTimestamp,Filename FROM Images WHERE SOPInstanceUID == :sopInstanceUID");
   fileExists.bindValue(":sopInstanceUID",sopInstanceUID);
   {
-  QMutexLocker lock(&(this->databaseAccessMutex));
   bool success = fileExists.exec();
   if (!success)
     {
@@ -1159,7 +1137,6 @@ void ctkDICOMDatabasePrivate::insert( const ctkDICOMDataset& ctkDataset, const Q
     }
   else
     {
-      QMutexLocker lock(&(this->databaseAccessMutex));
       qDebug() << "database filename for " << sopInstanceUID << " is: " << databaseFilename;
       qDebug() << "modified date is: " << fileLastModified;
       qDebug() << "db insert date is: " << databaseInsertTimestamp;
@@ -1363,7 +1340,6 @@ bool ctkDICOMDatabase::isInMemory() const
 bool ctkDICOMDatabase::removeSeries(const QString& seriesInstanceUID)
 {
   Q_D(ctkDICOMDatabase);
-  QMutexLocker lock(&(d->databaseAccessMutex));
 
   // get all images from series
   QSqlQuery fileExists ( d->Database );
@@ -1441,7 +1417,6 @@ bool ctkDICOMDatabase::removeSeries(const QString& seriesInstanceUID)
 bool ctkDICOMDatabase::cleanup()
 {
   Q_D(ctkDICOMDatabase);
-  QMutexLocker lock(&(d->databaseAccessMutex));
   QSqlQuery seriesCleanup ( d->Database );
   seriesCleanup.exec("DELETE FROM Series WHERE ( SELECT COUNT(*) FROM Images WHERE Images.SeriesInstanceUID = Series.SeriesInstanceUID ) = 0;");
   seriesCleanup.exec("DELETE FROM Studies WHERE ( SELECT COUNT(*) FROM Series WHERE Series.StudyInstanceUID = Studies.StudyInstanceUID ) = 0;");
@@ -1453,7 +1428,6 @@ bool ctkDICOMDatabase::cleanup()
 bool ctkDICOMDatabase::removeStudy(const QString& studyInstanceUID)
 {
   Q_D(ctkDICOMDatabase);
-  QMutexLocker lock(&(d->databaseAccessMutex));
 
   QSqlQuery seriesForStudy( d->Database );
   seriesForStudy.prepare("SELECT SeriesInstanceUID FROM Series WHERE StudyInstanceUID = :studyID");
@@ -1481,7 +1455,6 @@ bool ctkDICOMDatabase::removeStudy(const QString& studyInstanceUID)
 bool ctkDICOMDatabase::removePatient(const QString& patientID)
 {
   Q_D(ctkDICOMDatabase);
-  QMutexLocker lock(&(d->databaseAccessMutex));
 
   QSqlQuery studiesForPatient( d->Database );
   studiesForPatient.prepare("SELECT StudyInstanceUID FROM Studies WHERE PatientsUID = :patientsID");
@@ -1522,7 +1495,6 @@ bool ctkDICOMDatabase::tagCacheExists()
   // try to open the database if it's not already open
   if ( !(d->TagCacheDatabase.isOpen()) )
     {
-    QMutexLocker lock(&(d->databaseAccessMutex));
     qDebug() << "TagCacheDatabase not open\n";
     d->TagCacheDatabase = QSqlDatabase::addDatabase("QSQLITE", d->Database.connectionName() + "TagCache");
     d->TagCacheDatabase.setDatabaseName(d->TagCacheDatabaseFilename);

Libs/DICOM/Core/ctkDICOMIndexer.cpp

@@ -31,7 +31,6 @@
 #include <QFileInfo>
 #include <QDebug>
 #include <QPixmap>
-#include <QtConcurrentRun>
 
 // ctkDICOM includes
 #include "ctkLogger.h"
@@ -51,25 +50,6 @@
 #include <dcmtk/dcmimgle/dcmimage.h>  /* for class DicomImage */
 #include <dcmtk/dcmimage/diregist.h>  /* include support for color images */
 
-class AddFileFunctor
-{
-public:
-     AddFileFunctor(ctkDICOMIndexer* indexer, ctkDICOMDatabase& database,
-                    const QString& destinationDirectoryName = "")
-       : Indexer(indexer), Database(database), DestinationDirectoryName(destinationDirectoryName) { }
-
-     bool operator()(const QString &filePath)
-     {
-         Indexer->addFile(Database,filePath,DestinationDirectoryName);
-         return false; // make sure it is removed;
-     }
-
-     ctkDICOMIndexer* Indexer;
-     ctkDICOMDatabase& Database;
-     QString DestinationDirectoryName;
-
- };
-
 
 //------------------------------------------------------------------------------
 static ctkLogger logger("org.commontk.dicom.DICOMIndexer" );
@@ -80,33 +60,14 @@ static ctkLogger logger("org.commontk.dicom.DICOMIndexer" );
 // ctkDICOMIndexerPrivate methods
 
 //------------------------------------------------------------------------------
-ctkDICOMIndexerPrivate::ctkDICOMIndexerPrivate(ctkDICOMIndexer& o) : q_ptr(&o), Canceled(false), CurrentPercentageProgress(-1)
+ctkDICOMIndexerPrivate::ctkDICOMIndexerPrivate(ctkDICOMIndexer& o) : q_ptr(&o), Canceled(false)
 {
   Q_Q(ctkDICOMIndexer);
-  connect(&DirectoryImportWatcher,SIGNAL(progressValueChanged(int)),this,SLOT(OnProgress(int)));
-  connect(&DirectoryImportWatcher,SIGNAL(finished()),q,SIGNAL(indexingComplete()));
-  connect(&DirectoryImportWatcher,SIGNAL(canceled()),q,SIGNAL(indexingComplete()));
 }
 
 //------------------------------------------------------------------------------
 ctkDICOMIndexerPrivate::~ctkDICOMIndexerPrivate()
 {
-  DirectoryImportWatcher.cancel();
-  DirectoryImportWatcher.waitForFinished();
-
-}
-
-void ctkDICOMIndexerPrivate::OnProgress(int)
-{
-  Q_Q(ctkDICOMIndexer);
-
-  int newPercentageProgress = ( 100 * DirectoryImportFuture.progressValue() ) / DirectoryImportFuture.progressMaximum();
-  if (newPercentageProgress != CurrentPercentageProgress)
-    {
-      CurrentPercentageProgress = newPercentageProgress;
-      emit q->progress(newPercentageProgress);
-    }
-
 }
 
 //------------------------------------------------------------------------------
@@ -142,7 +103,7 @@ void ctkDICOMIndexer::addFile(ctkDICOMDatabase& database,
 }
 
 //------------------------------------------------------------------------------
-void ctkDICOMIndexer::addDirectory(ctkDICOMDatabase& ctkDICOMDatabase, 
+void ctkDICOMIndexer::addDirectory(ctkDICOMDatabase& ctkDICOMDatabase,
                                    const QString& directoryName,
                                    const QString& destinationDirectoryName)
 {
@@ -177,7 +138,6 @@ void ctkDICOMIndexer::addListOfFiles(ctkDICOMDatabase& ctkDICOMDatabase,
   int CurrentFileIndex = 0;
   foreach(QString filePath, listOfFiles)
   {
-
     int percent = ( 100 * CurrentFileIndex ) / listOfFiles.size();
     emit this->progress(percent);
     this->addFile(ctkDICOMDatabase, filePath, destinationDirectoryName);
@@ -189,21 +149,6 @@ void ctkDICOMIndexer::addListOfFiles(ctkDICOMDatabase& ctkDICOMDatabase,
       }
   }
   emit this->indexingComplete();
-
-
-#if 0
-  if(!listOfFiles.isEmpty())
-  {
-    if(d->DirectoryImportWatcher.isRunning())
-    {
-      d->DirectoryImportWatcher.cancel();
-      d->DirectoryImportWatcher.waitForFinished();
-    }
-    d->FilesToIndex.append(listOfFiles);
-    d->DirectoryImportFuture = QtConcurrent::filter(d->FilesToIndex,AddFileFunctor(this,ctkDICOMDatabase,destinationDirectoryName));
-    d->DirectoryImportWatcher.setFuture(d->DirectoryImportFuture);
-  }
-#endif
 }
 
 //------------------------------------------------------------------------------
@@ -307,25 +252,17 @@ void ctkDICOMIndexer::refreshDatabase(ctkDICOMDatabase& dicomDatabase, const QSt
     {
     filesytemFiles.insert(dirIt.next());
     }
-  
+
   // TODO: it looks like this function was never finished...
-  // 
+  //
   // I guess the next step is to remove all filesToRemove from the database
   // and also to add filesystemFiles into the database tables
-  */ 
+  */
   }
 
 //----------------------------------------------------------------------------
-void ctkDICOMIndexer::waitForImportFinished()
-{
-  Q_D(ctkDICOMIndexer);
-  d->DirectoryImportWatcher.waitForFinished();
-}
-
-//----------------------------------------------------------------------------
 void ctkDICOMIndexer::cancel()
 {
   Q_D(ctkDICOMIndexer);
-  d->DirectoryImportWatcher.cancel();
   d->Canceled = true;
 }

Libs/DICOM/Core/ctkDICOMIndexer.h

@@ -82,12 +82,6 @@ public:
 
   Q_INVOKABLE void refreshDatabase(ctkDICOMDatabase& database, const QString& directoryName);
 
-  ///
-  /// \brief ensures that the QFuture threads have all finished indexing
-  /// before returning control.
-  ///
-  Q_INVOKABLE void waitForImportFinished();
-
 Q_SIGNALS:
   void foundFilesToIndex(int);
   void indexingFileNumber(int);

Libs/DICOM/Core/ctkDICOMIndexer_p.h

@@ -39,17 +39,9 @@ public:
   ctkDICOMIndexerPrivate(ctkDICOMIndexer&);
   ~ctkDICOMIndexerPrivate();
 
-public Q_SLOTS:
-
-  void OnProgress(int progress);
 public:
-
   ctkDICOMAbstractThumbnailGenerator* thumbnailGenerator;
   bool                    Canceled;
-  QStringList FilesToIndex;
-  QFutureWatcher<void> DirectoryImportWatcher;
-  QFuture<void> DirectoryImportFuture;
-  int CurrentPercentageProgress;
 };