Skip to content

Commit

Permalink
Don't load folder if we encounter a db error
Browse files Browse the repository at this point in the history
Fixes: #9147
  • Loading branch information
TheOneRing committed Mar 1, 2022
1 parent 83d238f commit 2a30e71
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 151 deletions.
7 changes: 7 additions & 0 deletions changelog/unreleased/9147
Original file line number Diff line number Diff line change
@@ -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
110 changes: 60 additions & 50 deletions src/common/ownsql.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,35 @@
*/

#include <QDateTime>
#include <QLoggingCategory>
#include <QString>
#include <QDir>
#include <QFile>
#include <QFileInfo>
#include <QDir>
#include <QLoggingCategory>
#include <QString>

#include "ownsql.h"
#include "common/utility.h"
#include "common/asserts.h"
#include "common/utility.h"
#include "ownsql.h"

#include <sqlite3.h>

#define SQLITE_SLEEP_TIME_USEC 100000
#define SQLITE_REPEAT_COUNT 20
#include <chrono>
#include <thread>

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 {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -298,27 +312,26 @@ bool SqlQuery::isPragma()

bool SqlQuery::exec()
{
qCDebug(lcSql) << "SQL exec" << _sql;

if (!_stmt) {
qCWarning(lcSql) << "Can't exec query, statement unprepared.";
return false;
}

// 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) {
Expand All @@ -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;
}
Expand Down
153 changes: 78 additions & 75 deletions src/gui/folder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit 2a30e71

Please sign in to comment.