diff --git a/changelog/unreleased/9147 b/changelog/unreleased/9147 new file mode 100644 index 00000000000..588345ff514 --- /dev/null +++ b/changelog/unreleased/9147 @@ -0,0 +1,7 @@ +Bugfix: Fix crash if a database error occurs + +We no longer crash if a database error occurs on startup, +instead the folder will enter an error sate similar to the case +that the folder does not exist. + +https://github.com/owncloud/client/issues/9147 diff --git a/src/common/ownsql.cpp b/src/common/ownsql.cpp index 0877b2787fe..5cf9eb1fa68 100644 --- a/src/common/ownsql.cpp +++ b/src/common/ownsql.cpp @@ -17,26 +17,35 @@ */ #include -#include -#include +#include #include #include -#include +#include +#include -#include "ownsql.h" -#include "common/utility.h" #include "common/asserts.h" +#include "common/utility.h" +#include "ownsql.h" + #include -#define SQLITE_SLEEP_TIME_USEC 100000 -#define SQLITE_REPEAT_COUNT 20 +#include +#include + +using namespace std::chrono_literals; -#define SQLITE_DO(A) \ - if (1) { \ - _errId = (A); \ - if (_errId != SQLITE_OK && _errId != SQLITE_DONE && _errId != SQLITE_ROW) { \ - _error = QString::fromUtf8(sqlite3_errmsg(_db)); \ - } \ +namespace { +constexpr auto SQLITE_SLEEP_TIME = 500ms; +constexpr int SQLITE_REPEAT_COUNT = 20; + +} + +#define SQLITE_DO(A) \ + if (1) { \ + _errId = (A); \ + if (_errId != SQLITE_OK && _errId != SQLITE_DONE && _errId != SQLITE_ROW) { \ + _error = QString::fromUtf8(sqlite3_errmsg(_db)); \ + } \ } namespace OCC { @@ -132,28 +141,29 @@ bool SqlDatabase::openOrCreateReadWrite(const QString &filename) auto checkResult = checkDb(); if (checkResult != CheckDbResult::Ok) { + close(); if (checkResult == CheckDbResult::CantPrepare) { // When disk space is low, preparing may fail even though the db is fine. // Typically CANTOPEN or IOERR. qint64 freeSpace = Utility::freeDiskSpace(QFileInfo(filename).dir().absolutePath()); if (freeSpace != -1 && freeSpace < 1000000) { qCWarning(lcSql) << "Can't prepare consistency check and disk space is low:" << freeSpace; - close(); - return false; - } - - // Even when there's enough disk space, it might very well be that the - // file is on a read-only filesystem and can't be opened because of that. - if (_errId == SQLITE_CANTOPEN) { + } else if (_errId == SQLITE_CANTOPEN) { + // Even when there's enough disk space, it might very well be that the + // file is on a read-only filesystem and can't be opened because of that. qCWarning(lcSql) << "Can't open db to prepare consistency check, aborting"; - close(); - return false; + } else if (_errId == SQLITE_LOCKED || _errId == SQLITE_BUSY) { + qCWarning(lcSql) << "Can't open db to prepare consistency check, the db is locked aborting" << _errId << _error; } + return false; } qCCritical(lcSql) << "Consistency check failed, removing broken db" << filename; - close(); - QFile::remove(filename); + QFile fileToRemove(filename); + if (!fileToRemove.remove()) { + qCCritical(lcSql) << "Failed to remove broken db" << filename << ":" << fileToRemove.errorString(); + return false; + } return openHelper(filename, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE); } @@ -254,20 +264,24 @@ int SqlQuery::prepare(const QByteArray &sql, bool allow_failure) finish(); } if (!_sql.isEmpty()) { - int n = 0; - int rc; - do { + int rc = {}; + for (int n = 0; n < SQLITE_REPEAT_COUNT; ++n) { + qCDebug(lcSql) << "SQL prepare" << _sql << "Try:" << n; rc = sqlite3_prepare_v2(_db, _sql.constData(), -1, &_stmt, nullptr); - if ((rc == SQLITE_BUSY) || (rc == SQLITE_LOCKED)) { - n++; - OCC::Utility::usleep(SQLITE_SLEEP_TIME_USEC); + if (rc != SQLITE_DONE) { + qCWarning(lcSql) << "SQL prepare failed" << _sql << QString::fromUtf8(sqlite3_errmsg(_db)); + if ((rc == SQLITE_BUSY) || (rc == SQLITE_LOCKED)) { + std::this_thread::sleep_for(SQLITE_SLEEP_TIME); + continue; + } } - } while ((n < SQLITE_REPEAT_COUNT) && ((rc == SQLITE_BUSY) || (rc == SQLITE_LOCKED))); + break; + } _errId = rc; if (_errId != SQLITE_OK) { _error = QString::fromUtf8(sqlite3_errmsg(_db)); - qCWarning(lcSql) << "Sqlite prepare statement error:" << _error << "in" << _sql; + qCWarning(lcSql) << "Sqlite prepare statement error:" << _errId << _error << "in" << _sql; OC_ENFORCE_X(allow_failure, "SQLITE Prepare error"); } else { OC_ASSERT(_stmt); @@ -298,8 +312,6 @@ bool SqlQuery::isPragma() bool SqlQuery::exec() { - qCDebug(lcSql) << "SQL exec" << _sql; - if (!_stmt) { qCWarning(lcSql) << "Can't exec query, statement unprepared."; return false; @@ -307,18 +319,19 @@ bool SqlQuery::exec() // Don't do anything for selects, that is how we use the lib :-| if (!isSelect() && !isPragma()) { - int rc, n = 0; - do { + int rc = 0; + for (int n = 0; n < SQLITE_REPEAT_COUNT; ++n) { + qCDebug(lcSql) << "SQL exec" << _sql << "Try:" << n; rc = sqlite3_step(_stmt); - if (rc == SQLITE_LOCKED) { - rc = sqlite3_reset(_stmt); /* This will also return SQLITE_LOCKED */ - n++; - OCC::Utility::usleep(SQLITE_SLEEP_TIME_USEC); - } else if (rc == SQLITE_BUSY) { - OCC::Utility::usleep(SQLITE_SLEEP_TIME_USEC); - n++; + if (rc != SQLITE_DONE) { + qCWarning(lcSql) << "SQL exec failed" << _sql << QString::fromUtf8(sqlite3_errmsg(_db)); + if (rc == SQLITE_LOCKED || rc == SQLITE_BUSY) { + std::this_thread::sleep_for(SQLITE_SLEEP_TIME); + continue; + } + break; } - } while ((n < SQLITE_REPEAT_COUNT) && ((rc == SQLITE_BUSY) || (rc == SQLITE_LOCKED))); + } _errId = rc; if (_errId != SQLITE_DONE && _errId != SQLITE_ROW) { @@ -343,13 +356,10 @@ auto SqlQuery::next() -> NextResult { const bool firstStep = !sqlite3_stmt_busy(_stmt); - int n = 0; - forever { + for (int n = 0; n < SQLITE_REPEAT_COUNT; ++n) { _errId = sqlite3_step(_stmt); - if (n < SQLITE_REPEAT_COUNT && firstStep && (_errId == SQLITE_LOCKED || _errId == SQLITE_BUSY)) { - sqlite3_reset(_stmt); // not necessary after sqlite version 3.6.23.1 - n++; - OCC::Utility::usleep(SQLITE_SLEEP_TIME_USEC); + if (firstStep && (_errId == SQLITE_LOCKED || _errId == SQLITE_BUSY)) { + std::this_thread::sleep_for(SQLITE_SLEEP_TIME); } else { break; } diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 3e42d50c8bd..26d86568a8d 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -90,75 +90,71 @@ Folder::Folder(const FolderDefinition &definition, } _syncResult.setStatus(status); - // check if the local path exists - const auto folderOk = checkLocalPath(); - _syncResult.setFolder(_definition.alias); - - // those errors should not persist over sessions - _journal.wipeErrorBlacklistCategory(SyncJournalErrorBlacklistRecord::Category::LocalSoftError); - - _engine.reset(new SyncEngine(_accountState->account(), path(), remotePath(), &_journal)); - // pass the setting if hidden files are to be ignored, will be read in csync_update - _engine->setIgnoreHiddenFiles(_definition.ignoreHiddenFiles); - - ConfigFile::setupDefaultExcludeFilePaths(_engine->excludedFiles()); - if (!reloadExcludes()) - qCWarning(lcFolder, "Could not read system exclude file"); - - connect(_accountState.data(), &AccountState::isConnectedChanged, this, &Folder::canSyncChanged); - connect(_engine.data(), &SyncEngine::rootEtag, this, &Folder::etagRetrievedFromSyncEngine); - - connect(_engine.data(), &SyncEngine::started, this, &Folder::slotSyncStarted, Qt::QueuedConnection); - connect(_engine.data(), &SyncEngine::finished, this, &Folder::slotSyncFinished, Qt::QueuedConnection); - - connect(_engine.data(), &SyncEngine::aboutToRemoveAllFiles, - this, &Folder::slotAboutToRemoveAllFiles); - connect(_engine.data(), &SyncEngine::transmissionProgress, this, &Folder::slotTransmissionProgress); - connect(_engine.data(), &SyncEngine::itemCompleted, - this, &Folder::slotItemCompleted); - connect(_engine.data(), &SyncEngine::newBigFolder, - this, &Folder::slotNewBigFolderDiscovered); - connect(_engine.data(), &SyncEngine::seenLockedFile, FolderMan::instance(), &FolderMan::slotSyncOnceFileUnlocks); - connect(_engine.data(), &SyncEngine::aboutToPropagate, - this, &Folder::slotLogPropagationStart); - connect(_engine.data(), &SyncEngine::syncError, this, &Folder::slotSyncError); - - _scheduleSelfTimer.setSingleShot(true); - _scheduleSelfTimer.setInterval(SyncEngine::minimumFileAgeForUpload); - connect(&_scheduleSelfTimer, &QTimer::timeout, - this, &Folder::slotScheduleThisFolder); - - connect(ProgressDispatcher::instance(), &ProgressDispatcher::folderConflicts, - this, &Folder::slotFolderConflicts); - connect(_engine.data(), &SyncEngine::excluded, this, [this](const QString &path, CSYNC_EXCLUDE_TYPE reason) { - Q_EMIT ProgressDispatcher::instance()->excluded(this, path, reason); - }); - - _localDiscoveryTracker.reset(new LocalDiscoveryTracker); - connect(_engine.data(), &SyncEngine::finished, - _localDiscoveryTracker.data(), &LocalDiscoveryTracker::slotSyncFinished); - connect(_engine.data(), &SyncEngine::itemCompleted, - _localDiscoveryTracker.data(), &LocalDiscoveryTracker::slotItemCompleted); - - // Potentially upgrade suffix vfs to windows vfs - OC_ENFORCE(_vfs); - if (_definition.virtualFilesMode == Vfs::WithSuffix - && _definition.upgradeVfsMode) { - if (isVfsPluginAvailable(Vfs::WindowsCfApi)) { - if (auto winvfs = createVfsFromPlugin(Vfs::WindowsCfApi)) { - // Wipe the existing suffix files from fs and journal - SyncEngine::wipeVirtualFiles(path(), _journal, *_vfs); - - // Then switch to winvfs mode - _vfs.reset(winvfs.release()); - _definition.virtualFilesMode = Vfs::WindowsCfApi; + // check if the local path exists + if (checkLocalPath()) { + // those errors should not persist over sessions + _journal.wipeErrorBlacklistCategory(SyncJournalErrorBlacklistRecord::Category::LocalSoftError); + + _engine.reset(new SyncEngine(_accountState->account(), path(), remotePath(), &_journal)); + // pass the setting if hidden files are to be ignored, will be read in csync_update + _engine->setIgnoreHiddenFiles(_definition.ignoreHiddenFiles); + + ConfigFile::setupDefaultExcludeFilePaths(_engine->excludedFiles()); + if (!reloadExcludes()) + qCWarning(lcFolder, "Could not read system exclude file"); + + connect(_accountState.data(), &AccountState::isConnectedChanged, this, &Folder::canSyncChanged); + connect(_engine.data(), &SyncEngine::rootEtag, this, &Folder::etagRetrievedFromSyncEngine); + + connect(_engine.data(), &SyncEngine::started, this, &Folder::slotSyncStarted, Qt::QueuedConnection); + connect(_engine.data(), &SyncEngine::finished, this, &Folder::slotSyncFinished, Qt::QueuedConnection); + + connect(_engine.data(), &SyncEngine::aboutToRemoveAllFiles, + this, &Folder::slotAboutToRemoveAllFiles); + connect(_engine.data(), &SyncEngine::transmissionProgress, this, &Folder::slotTransmissionProgress); + connect(_engine.data(), &SyncEngine::itemCompleted, + this, &Folder::slotItemCompleted); + connect(_engine.data(), &SyncEngine::newBigFolder, + this, &Folder::slotNewBigFolderDiscovered); + connect(_engine.data(), &SyncEngine::seenLockedFile, FolderMan::instance(), &FolderMan::slotSyncOnceFileUnlocks); + connect(_engine.data(), &SyncEngine::aboutToPropagate, + this, &Folder::slotLogPropagationStart); + connect(_engine.data(), &SyncEngine::syncError, this, &Folder::slotSyncError); + + _scheduleSelfTimer.setSingleShot(true); + _scheduleSelfTimer.setInterval(SyncEngine::minimumFileAgeForUpload); + connect(&_scheduleSelfTimer, &QTimer::timeout, + this, &Folder::slotScheduleThisFolder); + + connect(ProgressDispatcher::instance(), &ProgressDispatcher::folderConflicts, + this, &Folder::slotFolderConflicts); + connect(_engine.data(), &SyncEngine::excluded, this, [this](const QString &path, CSYNC_EXCLUDE_TYPE reason) { + Q_EMIT ProgressDispatcher::instance()->excluded(this, path, reason); + }); + + _localDiscoveryTracker.reset(new LocalDiscoveryTracker); + connect(_engine.data(), &SyncEngine::finished, + _localDiscoveryTracker.data(), &LocalDiscoveryTracker::slotSyncFinished); + connect(_engine.data(), &SyncEngine::itemCompleted, + _localDiscoveryTracker.data(), &LocalDiscoveryTracker::slotItemCompleted); + + // Potentially upgrade suffix vfs to windows vfs + OC_ENFORCE(_vfs); + if (_definition.virtualFilesMode == Vfs::WithSuffix + && _definition.upgradeVfsMode) { + if (isVfsPluginAvailable(Vfs::WindowsCfApi)) { + if (auto winvfs = createVfsFromPlugin(Vfs::WindowsCfApi)) { + // Wipe the existing suffix files from fs and journal + SyncEngine::wipeVirtualFiles(path(), _journal, *_vfs); + + // Then switch to winvfs mode + _vfs.reset(winvfs.release()); + _definition.virtualFilesMode = Vfs::WindowsCfApi; + } } + saveToSettings(); } - saveToSettings(); - } - - if (folderOk) { // Initialize the vfs plugin startVfs(); } @@ -192,10 +188,13 @@ bool Folder::checkLocalPath() _canonicalLocalPath.append('/'); } + QString error; if (fi.isDir() && fi.isReadable() && fi.isWritable()) { qCDebug(lcFolder) << "Checked local path ok"; + if (!_journal.open()) { + error = tr("%1 failed to open the database.").arg(_definition.localPath); + } } else { - QString error; // Check directory again if (!FileSystem::fileExists(_definition.localPath, fi)) { error = tr("Local folder %1 does not exist.").arg(_definition.localPath); @@ -206,11 +205,12 @@ bool Folder::checkLocalPath() } else if (!fi.isWritable()) { error = tr("%1 is not writable.").arg(_definition.localPath); } - if (!error.isEmpty()) { - _syncResult.appendErrorString(error); - _syncResult.setStatus(SyncResult::SetupError); - return false; - } + } + qCWarning(lcFolder) << error; + if (!error.isEmpty()) { + _syncResult.appendErrorString(error); + _syncResult.setStatus(SyncResult::SetupError); + return false; } return true; } @@ -278,7 +278,7 @@ QString Folder::cleanPath() const bool Folder::isSyncRunning() const { - return _engine->isSyncRunning() || _vfs->isHydrating(); + return !hasSetupError() && (_engine->isSyncRunning() || _vfs->isHydrating()); } QString Folder::remotePath() const @@ -544,14 +544,13 @@ void Folder::startVfs() connect(_vfs.data(), &Vfs::doneHydrating, this, &Folder::slotHydrationDone); connect(&_engine->syncFileStatusTracker(), &SyncFileStatusTracker::fileStatusChanged, - _vfs.data(), &Vfs::fileStatusChanged); + _vfs.data(), &Vfs::fileStatusChanged); connect(_vfs.data(), &Vfs::started, this, [this] { // Immediately mark the sqlite temporaries as excluded. They get recreated // on db-open and need to get marked again every time. QString stateDbFile = _journal.databaseFilePath(); - _journal.open(); _vfs->fileStatusChanged(stateDbFile + QStringLiteral("-wal"), SyncFileStatus::StatusExcluded); _vfs->fileStatusChanged(stateDbFile + QStringLiteral("-shm"), SyncFileStatus::StatusExcluded); _vfsIsReady = true; @@ -828,6 +827,10 @@ void Folder::slotTerminateSync() void Folder::wipeForRemoval() { + // we can't acces those variables + if (hasSetupError()) { + return; + } // prevent interaction with the db etc _vfsIsReady = false; diff --git a/src/gui/folder.h b/src/gui/folder.h index f4ad3058853..e130dc74177 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -188,6 +188,11 @@ class Folder : public QObject */ bool isReady() const; + bool hasSetupError() const + { + return _syncResult.status() == SyncResult::SetupError; + } + /** * Returns true if the folder needs sync poll interval wise, and can * sync due to its internal state diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 2d9ae5731ba..7597d2637df 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -193,24 +193,26 @@ void FolderMan::unloadFolder(Folder *f) { Q_ASSERT(f); + _folderMap.remove(f->alias()); _socketApi->slotUnregisterPath(f->alias()); - _folderMap.remove(f->alias()); - disconnect(f, &Folder::syncStarted, - this, &FolderMan::slotFolderSyncStarted); - disconnect(f, &Folder::syncFinished, - this, &FolderMan::slotFolderSyncFinished); - disconnect(f, &Folder::syncStateChange, - this, &FolderMan::slotForwardFolderSyncStateChange); - disconnect(f, &Folder::syncPausedChanged, - this, &FolderMan::slotFolderSyncPaused); - disconnect(&f->syncEngine().syncFileStatusTracker(), &SyncFileStatusTracker::fileStatusChanged, - _socketApi.data(), &SocketApi::broadcastStatusPushMessage); - disconnect(f, &Folder::watchedFileChangedExternally, - &f->syncEngine().syncFileStatusTracker(), &SyncFileStatusTracker::slotPathTouched); + if (!f->hasSetupError()) { + disconnect(f, &Folder::syncStarted, + this, &FolderMan::slotFolderSyncStarted); + disconnect(f, &Folder::syncFinished, + this, &FolderMan::slotFolderSyncFinished); + disconnect(f, &Folder::syncStateChange, + this, &FolderMan::slotForwardFolderSyncStateChange); + disconnect(f, &Folder::syncPausedChanged, + this, &FolderMan::slotFolderSyncPaused); + disconnect(&f->syncEngine().syncFileStatusTracker(), &SyncFileStatusTracker::fileStatusChanged, + _socketApi.data(), &SocketApi::broadcastStatusPushMessage); + disconnect(f, &Folder::watchedFileChangedExternally, + &f->syncEngine().syncFileStatusTracker(), &SyncFileStatusTracker::slotPathTouched); - f->syncEngine().disconnect(f); + f->syncEngine().disconnect(f); + } } void FolderMan::unloadAndDeleteAllFolders() @@ -1123,18 +1125,20 @@ Folder *FolderMan::addFolderInternal( } // See matching disconnects in unloadFolder(). - connect(folder, &Folder::syncStarted, this, &FolderMan::slotFolderSyncStarted); - connect(folder, &Folder::syncFinished, this, &FolderMan::slotFolderSyncFinished); - connect(folder, &Folder::syncStateChange, this, &FolderMan::slotForwardFolderSyncStateChange); - connect(folder, &Folder::syncPausedChanged, this, &FolderMan::slotFolderSyncPaused); - connect(folder, &Folder::canSyncChanged, this, &FolderMan::slotFolderCanSyncChanged); - connect(&folder->syncEngine().syncFileStatusTracker(), &SyncFileStatusTracker::fileStatusChanged, - _socketApi.data(), &SocketApi::broadcastStatusPushMessage); - connect(folder, &Folder::watchedFileChangedExternally, - &folder->syncEngine().syncFileStatusTracker(), &SyncFileStatusTracker::slotPathTouched); - - folder->registerFolderWatcher(); - registerFolderWithSocketApi(folder); + if (!folder->hasSetupError()) { + connect(folder, &Folder::syncStarted, this, &FolderMan::slotFolderSyncStarted); + connect(folder, &Folder::syncFinished, this, &FolderMan::slotFolderSyncFinished); + connect(folder, &Folder::syncStateChange, this, &FolderMan::slotForwardFolderSyncStateChange); + connect(folder, &Folder::syncPausedChanged, this, &FolderMan::slotFolderSyncPaused); + connect(folder, &Folder::canSyncChanged, this, &FolderMan::slotFolderCanSyncChanged); + connect(&folder->syncEngine().syncFileStatusTracker(), &SyncFileStatusTracker::fileStatusChanged, + _socketApi.data(), &SocketApi::broadcastStatusPushMessage); + connect(folder, &Folder::watchedFileChangedExternally, + &folder->syncEngine().syncFileStatusTracker(), &SyncFileStatusTracker::slotPathTouched); + + folder->registerFolderWatcher(); + registerFolderWithSocketApi(folder); + } return folder; }