Merge pull request #4656 from owncloud/close_journal_fix

Close journal fix - do not keep the journal open or reopen on error.

src/libsync/owncloudpropagator.cpp

@@ -762,11 +762,13 @@ void CleanupPollsJob::start()
 
     auto info = _pollInfos.first();
     _pollInfos.pop_front();
-    SyncFileItemPtr item(new SyncFileItem(
-            _journal->getFileRecord(info._file).toSyncFileItem()));
-    PollJob *job = new PollJob(_account, info._url, item, _journal, _localPath, this);
-    connect(job, SIGNAL(finishedSignal()), SLOT(slotPollFinished()));
-    job->start();
+    SyncJournalFileRecord record = _journal->getFileRecord(info._file);
+    SyncFileItemPtr item(new SyncFileItem(record.toSyncFileItem()));
+    if (record.isValid()) {
+        PollJob *job = new PollJob(_account, info._url, item, _journal, _localPath, this);
+        connect(job, SIGNAL(finishedSignal()), SLOT(slotPollFinished()));
+        job->start();
+    }
 }
 
 void CleanupPollsJob::slotPollFinished()

src/libsync/propagateremotemove.cpp

@@ -154,15 +154,21 @@ void PropagateRemoteMove::finalize()
 {
     SyncJournalFileRecord oldRecord =
             _propagator->_journal->getFileRecord(_item->_originalFile);
+    // if reading from db failed still continue hoping that deleteFileRecord
+    // reopens the db successfully.
+    // The db is only queried to transfer the content checksum from the old
+    // to the new record. It is not a problem to skip it here.
     _propagator->_journal->deleteFileRecord(_item->_originalFile);
 
     SyncJournalFileRecord record(*_item, _propagator->getFilePath(_item->_renameTarget));
     record._path = _item->_renameTarget;
-    record._contentChecksum = oldRecord._contentChecksum;
-    record._contentChecksumType = oldRecord._contentChecksumType;
-    if (record._fileSize != oldRecord._fileSize) {
-        qDebug() << "Warning: file sizes differ on server vs csync_journal: " << record._fileSize << oldRecord._fileSize;
-        record._fileSize = oldRecord._fileSize; // server might have claimed different size, we take the old one from the DB
+    if (oldRecord.isValid()) {
+        record._contentChecksum = oldRecord._contentChecksum;
+        record._contentChecksumType = oldRecord._contentChecksumType;
+        if (record._fileSize != oldRecord._fileSize) {
+            qDebug() << "Warning: file sizes differ on server vs csync_journal: " << record._fileSize << oldRecord._fileSize;
+            record._fileSize = oldRecord._fileSize; // server might have claimed different size, we take the old one from the DB
+        }
     }
 
     if (!_propagator->_journal->setFileRecord(record)) {

src/libsync/propagatorjobs.cpp

@@ -235,8 +235,10 @@ void PropagateLocalRename::start()
 
     SyncJournalFileRecord record(*_item, targetFile);
     record._path = _item->_renameTarget;
-    record._contentChecksum = oldRecord._contentChecksum;
-    record._contentChecksumType = oldRecord._contentChecksumType;
+    if (oldRecord.isValid()) {
+        record._contentChecksum = oldRecord._contentChecksum;
+        record._contentChecksumType = oldRecord._contentChecksumType;
+    }
 
     if (!_item->_isDirectory) { // Directories are saved at the end
         if (!_propagator->_journal->setFileRecord(record)) {

src/libsync/syncengine.cpp

@@ -519,7 +519,7 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote )
 
             // If the 'W' remote permission changed, update the local filesystem
             SyncJournalFileRecord prev = _journal->getFileRecord(item->_file);
-            if (prev._remotePerm.contains('W') != item->_remotePerm.contains('W')) {
+            if (prev.isValid() && prev._remotePerm.contains('W') != item->_remotePerm.contains('W')) {
                 const bool isReadOnly = !item->_remotePerm.contains('W');
                 FileSystem::setFileReadOnlyWeak(filePath, isReadOnly);
             }
@@ -1018,6 +1018,7 @@ void SyncEngine::finalize(bool success)
     _thread.wait();
 
     csync_commit(_csync_ctx);
+    _journal->close();
 
     qDebug() << "CSync run took " << _stopWatch.addLapTime(QLatin1String("Sync Finished"));
     _stopWatch.stop();

src/libsync/syncjournaldb.cpp

@@ -770,7 +770,7 @@ bool SyncJournalDb::deleteFileRecord(const QString& filename, bool recursively)
 }
 
 
-SyncJournalFileRecord SyncJournalDb::getFileRecord( const QString& filename )
+SyncJournalFileRecord SyncJournalDb::getFileRecord(const QString& filename)
 {
     QMutexLocker locker(&_mutex);
 
@@ -784,6 +784,8 @@ SyncJournalFileRecord SyncJournalDb::getFileRecord( const QString& filename )
         if (!_getFileRecordQuery->exec()) {
             QString err = _getFileRecordQuery->error();
             qDebug() << "Error creating prepared statement: " << _getFileRecordQuery->lastQuery() << ", Error:" << err;;
+            locker.unlock();
+            close();
             return rec;
         }
 
@@ -806,7 +808,10 @@ SyncJournalFileRecord SyncJournalDb::getFileRecord( const QString& filename )
             }
         } else {
             QString err = _getFileRecordQuery->error();
-            qDebug() << "No journal entry found for " << filename;
+            qDebug() << "No journal entry found for " << filename << "Error: " << err;
+            locker.unlock();
+            close();
+            locker.relock();
         }
         _getFileRecordQuery->reset_and_clear_bindings();
     }

src/libsync/syncjournaldb.h

@@ -38,7 +38,10 @@ class OWNCLOUDSYNC_EXPORT SyncJournalDb : public QObject
 public:
     explicit SyncJournalDb(const QString& path, QObject *parent = 0);
     virtual ~SyncJournalDb();
-    SyncJournalFileRecord getFileRecord( const QString& filename );
+
+    // to verify that the record could be queried successfully check
+    // with SyncJournalFileRecord::isValid()
+    SyncJournalFileRecord getFileRecord(const QString& filename);
     bool setFileRecord( const SyncJournalFileRecord& record );
 
     /// Like setFileRecord, but preserves checksums