Merge remote-tracking branch 'origin/2.10'

5 files changed, 177 insertions, 77 deletions

diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp
index 426aefa68..02567d8f2 100644
--- a/src/common/filesystembase.cpp
+++ b/src/common/filesystembase.cpp
@@ -454,35 +454,38 @@ bool FileSystem::moveToTrash(const QString &fileName, QString *errorString)
 #endif
 }
 
-bool FileSystem::isFileLocked(const QString &fileName, LockMode mode)
-{
 #ifdef Q_OS_WIN
-    // Check if file exists
+Utility::Handle FileSystem::lockFile(const QString &fileName, LockMode mode)
+{
     const QString fName = longWinPath(fileName);
     auto accessMode = mode == LockMode::Exclusive ? 0 : FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
+    // Check if file exists
     DWORD attr = GetFileAttributesW(reinterpret_cast<const wchar_t *>(fName.utf16()));
     if (attr != INVALID_FILE_ATTRIBUTES) {
         // Try to open the file with as much access as possible..
-        HANDLE win_h = CreateFileW(
+        return Utility::Handle { CreateFileW(
             reinterpret_cast<const wchar_t *>(fName.utf16()),
             GENERIC_READ | GENERIC_WRITE,
             accessMode,
-            NULL, OPEN_EXISTING,
+            nullptr, OPEN_EXISTING,
             FILE_ATTRIBUTE_NORMAL | FILE_FLAG_BACKUP_SEMANTICS,
-            NULL);
+            nullptr) };
+    }
+    return {};
+}
+#endif
 
-        if (win_h == INVALID_HANDLE_VALUE) {
-            if (GetLastError() == ERROR_SHARING_VIOLATION) {
-                return true;
-            }
-            return false;
-        } else {
-            CloseHandle(win_h);
-        }
-    } else {
+
+bool FileSystem::isFileLocked(const QString &fileName, LockMode mode)
+{
+#ifdef Q_OS_WIN
+    const auto handle = lockFile(fileName, mode);
+    if (!handle) {
         const auto error = GetLastError();
-        if (error != ERROR_FILE_NOT_FOUND && error != ERROR_PATH_NOT_FOUND) {
-            qCWarning(lcFileSystem()) << "GetFileAttributesW" << Utility::formatWinError(error);
+        if (error == ERROR_SHARING_VIOLATION) {
+            return true;
+        } else if (error != ERROR_FILE_NOT_FOUND && error != ERROR_PATH_NOT_FOUND) {
+            qCWarning(lcFileSystem()) << Q_FUNC_INFO << Utility::formatWinError(error);
         }
     }
 #else
diff --git a/src/common/filesystembase.h b/src/common/filesystembase.h
index b4ace754c..dd8f09a79 100644
--- a/src/common/filesystembase.h
+++ b/src/common/filesystembase.h
@@ -19,13 +19,15 @@
 #pragma once
 
 #include "config.h"
+#include "ocsynclib.h"
+
+#include "utility.h"
 
 #include <QString>
 #include <ctime>
 #include <QFileInfo>
 #include <QLoggingCategory>
 
-#include <ocsynclib.h>
 
 class QFile;
 
@@ -122,7 +124,17 @@ namespace FileSystem {
      * Warning: The resulting file may have an empty fileName and be unsuitable for use
      * with QFileInfo! Calling seek() on the QFile with >32bit signed values will fail!
      */
-    bool OCSYNC_EXPORT openAndSeekFileSharedRead(QFile *file, QString *error, qint64 seek);
+    bool OCSYNC_EXPORT openAndSeekFileSharedRead(QFile * file, QString * error, qint64 seek);
+
+    enum class LockMode {
+        Shared,
+        Exclusive
+    };
+    Q_ENUM_NS(LockMode);
+    /**
+     * Returns true when a file is locked. (Windows only)
+     */
+    bool OCSYNC_EXPORT isFileLocked(const QString &fileName, LockMode mode);
 
 #ifdef Q_OS_WIN
     /**
@@ -142,17 +154,12 @@ namespace FileSystem {
      *    the windows API functions work with the normal "unixoid" representation too.
      */
     QString OCSYNC_EXPORT pathtoUNC(const QString &str);
-#endif
 
-    enum class LockMode {
-        Shared,
-        Exclusive
-    };
-    Q_ENUM_NS(LockMode);
     /**
-     * Returns true when a file is locked. (Windows only)
+     * This function creates a file handle with the desired LockMode
      */
-    bool OCSYNC_EXPORT isFileLocked(const QString &fileName, LockMode mode);
+    Utility::Handle OCSYNC_EXPORT lockFile(const QString &fileName, LockMode mode);
+#endif
 
     /**
      * Returns whether the file is a shortcut file (ends with .lnk)
diff --git a/src/common/ownsql.cpp b/src/common/ownsql.cpp
index 0877b2787..2e970cece 100644
--- a/src/common/ownsql.cpp
+++ b/src/common/ownsql.cpp
@@ -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 {
@@ -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_OK) {
+                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 || rc != SQLITE_ROW) {
+                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/common/utility.h b/src/common/utility.h
index 8850f5e34..642f184c9 100644
--- a/src/common/utility.h
+++ b/src/common/utility.h
@@ -273,6 +273,59 @@ OCSYNC_EXPORT Q_DECLARE_LOGGING_CATEGORY(lcUtility)
         Q_DISABLE_COPY(NtfsPermissionLookupRAII);
     };
 
+
+    class OCSYNC_EXPORT Handle
+    {
+    public:
+        /**
+         * A RAAI for Windows Handles
+         */
+        Handle() = default;
+        explicit Handle(HANDLE h);
+        explicit Handle(HANDLE h, std::function<void(HANDLE)> &&close);
+
+        Handle(const Handle &) = delete;
+        Handle &operator=(const Handle &) = delete;
+
+        Handle(Handle &&other)
+        {
+            std::swap(_handle, other._handle);
+            std::swap(_close, other._close);
+        }
+
+        Handle &operator=(Handle &&other)
+        {
+            if (this != &other) {
+                std::swap(_handle, other._handle);
+                std::swap(_close, other._close);
+            }
+            return *this;
+        }
+
+        ~Handle();
+
+        HANDLE &handle()
+        {
+            return _handle;
+        }
+
+        void close();
+
+        explicit operator bool() const
+        {
+            return _handle != INVALID_HANDLE_VALUE;
+        }
+
+        operator HANDLE() const
+        {
+            return _handle;
+        }
+
+    private:
+        HANDLE _handle = INVALID_HANDLE_VALUE;
+        std::function<void(HANDLE)> _close;
+    };
+
 #endif
 
     template <class E>
diff --git a/src/common/utility_win.cpp b/src/common/utility_win.cpp
index ad3dfc19d..0344a2902 100644
--- a/src/common/utility_win.cpp
+++ b/src/common/utility_win.cpp
@@ -323,4 +323,31 @@ Utility::NtfsPermissionLookupRAII::~NtfsPermissionLookupRAII()
     qt_ntfs_permission_lookup--;
 }
 
+
+Utility::Handle::Handle(HANDLE h, std::function<void(HANDLE)> &&close)
+    : _handle(h)
+    , _close(std::move(close))
+{
+}
+
+Utility::Handle::Handle(HANDLE h)
+    : _handle(h)
+    , _close(&CloseHandle)
+{
+}
+
+Utility::Handle::~Handle()
+{
+    close();
+}
+
+void Utility::Handle::close()
+{
+    if (_handle != INVALID_HANDLE_VALUE) {
+        _close(_handle);
+        _handle = INVALID_HANDLE_VALUE;
+    }
+}
+
+
 } // namespace OCC