Merge pull request #9627 from nextcloud/backport/9566/stable-4.0

[stable-4.0] Bugfix/better handling of locked files during upload

Author: mgallien

src/common/filesystembase.cpp

@@ -671,32 +671,70 @@ bool FileSystem::moveToTrash(const QString &fileName, QString *errorString)
     return true;
 }
 
-bool FileSystem::isFileLocked(const QString &fileName)
+namespace {
+
+/**
+ * This function creates a file handle with the desired LockMode
+ */
+#if defined Q_OS_WIN
+Utility::Handle lockFile(const QString &fileName, FileSystem::LockMode mode)
 {
-#ifdef Q_OS_WIN
+    const QString fName = FileSystem::longWinPath(fileName);
+    int shareMode = 0;
+    int accessMode = GENERIC_READ | GENERIC_WRITE;
+    switch (mode) {
+    case FileSystem::LockMode::Exclusive:
+        shareMode = 0;
+        break;
+    case FileSystem::LockMode::Shared:
+        shareMode = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
+        break;
+    case FileSystem::LockMode::SharedRead:
+        shareMode = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
+        accessMode = GENERIC_READ;
+        break;
+    }
     // Check if file exists
-    const QString fName = longWinPath(fileName);
     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(
-            reinterpret_cast<const wchar_t *>(fName.utf16()),
-            GENERIC_READ | GENERIC_WRITE,
-            FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
-            nullptr, OPEN_EXISTING,
-            FILE_ATTRIBUTE_NORMAL | FILE_FLAG_BACKUP_SEMANTICS,
-            nullptr);
-
-        if (win_h == INVALID_HANDLE_VALUE) {
-            /* could not be opened, so locked? */
-            /* 32 == ERROR_SHARING_VIOLATION */
+        auto out = Utility::Handle{CreateFileW(reinterpret_cast<const wchar_t *>(fName.utf16()), accessMode, shareMode, nullptr, OPEN_EXISTING,
+                                               FILE_ATTRIBUTE_NORMAL | FILE_FLAG_BACKUP_SEMANTICS, nullptr)};
+
+        if (out) {
+            LARGE_INTEGER start;
+            start.QuadPart = 0;
+            LARGE_INTEGER end;
+            end.QuadPart = -1;
+            if (LockFile(out.handle(), start.LowPart, start.HighPart, end.LowPart, end.HighPart)) {
+                return out;
+            } else {
+                return {};
+            }
+        }
+        return out;
+    }
+    return {};
+}
+#endif
+
+}
+
+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_SHARING_VIOLATION || error == ERROR_LOCK_VIOLATION) {
             return true;
-        } else {
-            CloseHandle(win_h);
+        } else if (error != ERROR_FILE_NOT_FOUND && error != ERROR_PATH_NOT_FOUND) {
+            qCWarning(lcFileSystem()) << Q_FUNC_INFO << Utility::formatWinError(error);
         }
     }
 #else
     Q_UNUSED(fileName);
+    Q_UNUSED(mode);
 #endif
     return false;
 }

src/common/filesystembase.h

@@ -35,6 +35,8 @@ OCSYNC_EXPORT Q_DECLARE_LOGGING_CATEGORY(lcFileSystem)
  * @brief This file contains file system helper
  */
 namespace FileSystem {
+    OCSYNC_EXPORT Q_NAMESPACE;
+
     enum class FolderPermissions {
         ReadOnly,
         ReadWrite,
@@ -178,10 +180,17 @@ namespace FileSystem {
     bool OCSYNC_EXPORT setAclPermission(const QString &path, FileSystem::FolderPermissions permissions);
 #endif
 
+    enum class LockMode {
+        Shared,
+        Exclusive,
+        SharedRead,
+    };
+    Q_ENUM_NS(LockMode);
+
     /**
      * Returns true when a file is locked. (Windows only)
      */
-    bool OCSYNC_EXPORT isFileLocked(const QString &fileName);
+    bool OCSYNC_EXPORT isFileLocked(const QString &fileName, LockMode mode);
 
     /**
      * Returns whether the file is a shortcut file (ends with .lnk)

src/common/utility.h

@@ -49,6 +49,51 @@ namespace Utility {
      */
     OCSYNC_EXPORT QVector<ProcessInfosForOpenFile> queryProcessInfosKeepingFileOpen(const QString &filePath);
 
+#ifdef Q_OS_WIN
+    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
+
     OCSYNC_EXPORT int rand();
     OCSYNC_EXPORT void sleep(int sec);
     OCSYNC_EXPORT void usleep(int usec);

src/common/utility_win.cpp

@@ -609,4 +609,29 @@ void Utility::LocalFreeDeleter::operator()(void *p) const
     ::LocalFree(reinterpret_cast<HLOCAL>(p));
 }
 
+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

src/gui/lockwatcher.cpp

@@ -45,7 +45,7 @@ void LockWatcher::checkFiles()
     QSet<QString> unlocked;
 
     for (const auto &path : std::as_const(_watchedPaths)) {
-        if (!FileSystem::isFileLocked(path)) {
+        if (!FileSystem::isFileLocked(path, FileSystem::LockMode::SharedRead)) {
             qCInfo(lcLockWatcher) << "Lock of" << path << "was released";
             emit fileUnlocked(path);
             unlocked.insert(path);

src/libsync/bulkpropagatorjob.cpp

@@ -241,7 +241,7 @@ void BulkPropagatorJob::triggerUpload()
 
             // If the file is currently locked, we want to retry the sync
             // when it becomes available again.
-            if (FileSystem::isFileLocked(singleFile._localPath)) {
+            if (FileSystem::isFileLocked(singleFile._localPath, FileSystem::LockMode::SharedRead)) {
                 emit propagator()->seenLockedFile(singleFile._localPath);
             }
 

src/libsync/owncloudpropagator.cpp

@@ -981,7 +981,7 @@ bool OwncloudPropagator::createConflict(const SyncFileItemPtr &item,
 
         // If the file is locked, we want to retry this sync when it
         // becomes available again.
-        if (FileSystem::isFileLocked(fn)) {
+        if (FileSystem::isFileLocked(fn, FileSystem::LockMode::SharedRead)) {
             emit seenLockedFile(fn);
         }
 
@@ -1055,7 +1055,7 @@ OCC::Optional<QString> OwncloudPropagator::createCaseClashConflict(const SyncFil
 
         // If the file is locked, we want to retry this sync when it
         // becomes available again.
-        if (FileSystem::isFileLocked(filename)) {
+        if (FileSystem::isFileLocked(filename, FileSystem::LockMode::SharedRead)) {
             emit seenLockedFile(filename);
         }
 

src/libsync/propagatedownload.cpp

@@ -1329,7 +1329,7 @@ void PropagateDownloadFile::downloadFinished()
         qCWarning(lcPropagateDownload) << QStringLiteral("Rename failed: %1 => %2").arg(_tmpFile.fileName()).arg(filename);
         // If the file is locked, we want to retry this sync when it
         // becomes available again, otherwise try again directly
-        if (FileSystem::isFileLocked(filename)) {
+        if (FileSystem::isFileLocked(filename, FileSystem::LockMode::SharedRead)) {
             emit propagator()->seenLockedFile(filename);
         } else {
             propagator()->_anotherSyncNeeded = true;

src/libsync/propagateupload.cpp

@@ -36,6 +36,7 @@
 namespace OCC {
 
 Q_LOGGING_CATEGORY(lcPutJob, "nextcloud.sync.networkjob.put", QtInfoMsg)
+Q_LOGGING_CATEGORY(lcUploadDevice, "nextcloud.sync.uploaddevice", QtInfoMsg)
 Q_LOGGING_CATEGORY(lcPollJob, "nextcloud.sync.networkjob.poll", QtInfoMsg)
 Q_LOGGING_CATEGORY(lcPropagateUpload, "nextcloud.sync.propagator.upload", QtInfoMsg)
 Q_LOGGING_CATEGORY(lcPropagateUploadV1, "nextcloud.sync.propagator.upload.v1", QtInfoMsg)
@@ -549,6 +550,7 @@ qint64 UploadDevice::readData(char *data, qint64 maxlen)
         setErrorString({});
         return c;
     } else if (c < 0) {
+        qCWarning(lcUploadDevice) << _file.errorString() << c;
         setErrorString(_file.errorString());
         return -1;
     }

src/libsync/propagateuploadng.cpp

@@ -352,12 +352,12 @@ void PropagateUploadFileNG::startNextChunk()
 
     const auto fileName = _fileToUpload._path;
     auto device = std::make_unique<UploadDevice>(fileName, _sent, _currentChunkSize, &propagator()->_bandwidthManager);
-    if (!device->open(QIODevice::ReadOnly)) {
+    if (auto isLocked = FileSystem::isFileLocked(fileName, FileSystem::LockMode::SharedRead); isLocked || !device->open(QIODevice::ReadOnly)) {
         qCWarning(lcPropagateUploadNG) << "Could not prepare upload device: " << device->errorString();
 
         // If the file is currently locked, we want to retry the sync
         // when it becomes available again.
-        if (FileSystem::isFileLocked(fileName)) {
+        if (isLocked) {
             emit propagator()->seenLockedFile(fileName);
         }
         // Soft error because this is likely caused by the user modifying his files while syncing