Require exclusive locks for vfs and renames

Fixes: #8761

changelog/unreleased/8761

@@ -0,0 +1,6 @@
+Bugfix: Locked files are not correctly synced
+
+We fixed an issue where files locked by office etc, where not correctly synced,
+when Windows Virtual files are enabled.
+
+https://github.com/owncloud/client/issues/8761

src/common/filesystembase.cpp

@@ -438,18 +438,19 @@ bool FileSystem::moveToTrash(const QString &fileName, QString *errorString)
 #endif
 }
 
-bool FileSystem::isFileLocked(const QString &fileName)
+bool FileSystem::isFileLocked(const QString &fileName, LockMode mode)
 {
 #ifdef Q_OS_WIN
     // Check if file exists
     const QString fName = longWinPath(fileName);
+    auto accessMode = mode == LockMode::Exclusive ? 0 : FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
     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,
+            accessMode,
             NULL, OPEN_EXISTING,
             FILE_ATTRIBUTE_NORMAL | FILE_FLAG_BACKUP_SEMANTICS,
             NULL);
@@ -461,6 +462,11 @@ bool FileSystem::isFileLocked(const QString &fileName)
         } else {
             CloseHandle(win_h);
         }
+    } else {
+        const auto error = GetLastError();
+        if (error != ERROR_FILE_NOT_FOUND && error != ERROR_PATH_NOT_FOUND) {
+            qCWarning(lcFileSystem()) << "GetFileAttributesW" << Utility::formatWinError(error);
+        }
     }
 #else
     Q_UNUSED(fileName);
@@ -521,3 +527,5 @@ QString FileSystem::pathtoUNC(const QString &str)
 }
 
 } // namespace OCC
+
+#include "moc_filesystembase.cpp"

src/common/filesystembase.h

@@ -42,6 +42,7 @@ OCSYNC_EXPORT Q_DECLARE_LOGGING_CATEGORY(lcFileSystem)
  * @brief This file contains file system helper
  */
 namespace FileSystem {
+OCSYNC_EXPORT Q_NAMESPACE;
 
     /**
      * @brief Mark the file as hidden  (only has effects on windows)
@@ -130,10 +131,15 @@ namespace FileSystem {
     QString fileSystemForPath(const QString &path);
 #endif
 
+    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);
+    bool OCSYNC_EXPORT isFileLocked(const QString &fileName, LockMode mode);
 
     /**
      * Returns whether the file is a shortcut file (ends with .lnk)

src/gui/folderman.cpp

@@ -556,9 +556,9 @@ void FolderMan::slotScheduleAppRestart()
     qCInfo(lcFolderMan) << "Application restart requested!";
 }
 
-void FolderMan::slotSyncOnceFileUnlocks(const QString &path)
+void FolderMan::slotSyncOnceFileUnlocks(const QString &path, FileSystem::LockMode mode)
 {
-    _lockWatcher->addFile(path);
+    _lockWatcher->addFile(path, mode);
 }
 
 /*

src/gui/folderman.h

@@ -259,7 +259,7 @@ public slots:
      * Automatically detemines the folder that's responsible for the file.
      * See slotWatchedFileUnlocked().
      */
-    void slotSyncOnceFileUnlocks(const QString &path);
+    void slotSyncOnceFileUnlocks(const QString &path, FileSystem::LockMode mode);
 
     // slot to schedule an ETag job (from Folder only)
     void slotScheduleETagJob(const QString &alias, RequestEtagJob *job);

src/gui/lockwatcher.cpp

@@ -32,10 +32,10 @@ LockWatcher::LockWatcher(QObject *parent)
     _timer.start(check_frequency);
 }
 
-void LockWatcher::addFile(const QString &path)
+void LockWatcher::addFile(const QString &path, FileSystem::LockMode mode)
 {
     qCInfo(lcLockWatcher) << "Watching for lock of" << path << "being released";
-    _watchedPaths.insert(path);
+    _watchedPaths.insert(path, mode);
 }
 
 void LockWatcher::setCheckInterval(std::chrono::milliseconds interval)
@@ -52,16 +52,18 @@ void LockWatcher::checkFiles()
 {
     QSet<QString> unlocked;
 
-    foreach (const QString &path, _watchedPaths) {
-        if (!FileSystem::isFileLocked(path)) {
-            qCInfo(lcLockWatcher) << "Lock of" << path << "was released";
-            emit fileUnlocked(path);
-            unlocked.insert(path);
+    for (auto it = _watchedPaths.cbegin(); it != _watchedPaths.constEnd(); ++it) {
+        if (!FileSystem::isFileLocked(it.key(), it.value())) {
+            qCInfo(lcLockWatcher) << "Lock of" << it.key() << "was released";
+            emit fileUnlocked(it.key());
+            unlocked.insert(it.key());
         }
     }
 
     // Doing it this way instead of with a QMutableSetIterator
     // ensures that calling back into addFile from connected
     // slots isn't a problem.
-    _watchedPaths.subtract(unlocked);
+    for (const auto &removed : unlocked) {
+        _watchedPaths.remove(removed);
+    }
 }

src/gui/lockwatcher.h

@@ -15,6 +15,7 @@
 #pragma once
 
 #include "config.h"
+#include "filesystem.h"
 
 #include <QList>
 #include <QObject>
@@ -51,7 +52,7 @@ class LockWatcher : public QObject
      * If the file is not locked later on, the fileUnlocked signal will be
      * emitted once.
      */
-    void addFile(const QString &path);
+    void addFile(const QString &path, OCC::FileSystem::LockMode mode);
 
     /** Adjusts the default interval for checking whether the lock is still present */
     void setCheckInterval(std::chrono::milliseconds interval);
@@ -68,7 +69,7 @@ private slots:
     void checkFiles();
 
 private:
-    QSet<QString> _watchedPaths;
+    QMap<QString, FileSystem::LockMode> _watchedPaths;
     QTimer _timer;
 };
 }

src/libsync/owncloudpropagator.cpp

@@ -696,8 +696,8 @@ 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)) {
-            emit seenLockedFile(fn);
+        if (FileSystem::isFileLocked(fn, FileSystem::LockMode::Exclusive)) {
+            emit seenLockedFile(fn, FileSystem::LockMode::Exclusive);
         }
 
         if (error)

src/libsync/owncloudpropagator.h

@@ -583,7 +583,7 @@ private slots:
     void finished(bool success);
 
     /** Emitted when propagation has problems with a locked file. */
-    void seenLockedFile(const QString &fileName);
+    void seenLockedFile(const QString &fileName, FileSystem::LockMode mode);
 
     /** Emitted when propagation touches a file.
      *

src/libsync/propagatedownload.cpp

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

src/libsync/propagateuploadng.cpp

@@ -373,14 +373,16 @@ void PropagateUploadFileNG::startNextChunk()
 
     auto device = std::unique_ptr<UploadDevice>(new UploadDevice(
             fileName, _currentChunkOffset, _currentChunkSize, &propagator()->_bandwidthManager));
+    // If the file is currently locked, we want to retry the sync
+    // when it becomes available again.
+    const auto lockMode = propagator()->syncOptions().requiredLockMode();
+    if (FileSystem::isFileLocked(fileName, lockMode)) {
+        emit propagator()->seenLockedFile(fileName, lockMode);
+        abortWithError(SyncFileItem::SoftError, tr("File is locked"));
+        return;
+    }
     if (!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)) {
-            emit propagator()->seenLockedFile(fileName);
-        }
         // Soft error because this is likely caused by the user modifying his files while syncing
         abortWithError(SyncFileItem::SoftError, device->errorString());
         return;

src/libsync/propagateuploadtus.cpp

@@ -59,20 +59,23 @@ Q_LOGGING_CATEGORY(lcPropagateUploadTUS, "sync.propagator.upload.tus", QtDebugMs
 UploadDevice *PropagateUploadFileTUS::prepareDevice(const quint64 &chunkSize)
 {
     const QString localFileName = propagator()->fullLocalPath(_item->_file);
-    auto device = new UploadDevice(localFileName, _currentOffset, chunkSize, &propagator()->_bandwidthManager);
+    auto device = std::make_unique<UploadDevice>(localFileName, _currentOffset, chunkSize, &propagator()->_bandwidthManager);
+    // If the file is currently locked, we want to retry the sync
+    // when it becomes available again.
+    const auto lockMode = propagator()->syncOptions().requiredLockMode();
+    if (FileSystem::isFileLocked(localFileName, lockMode)) {
+        emit propagator()->seenLockedFile(localFileName, lockMode);
+        abortWithError(SyncFileItem::SoftError, tr("File is locked"));
+        return nullptr;
+    }
     if (!device->open(QIODevice::ReadOnly)) {
         qCWarning(lcPropagateUploadTUS) << "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(localFileName)) {
-            emit propagator()->seenLockedFile(localFileName);
-        }
         // Soft error because this is likely caused by the user modifying his files while syncing
         abortWithError(SyncFileItem::SoftError, device->errorString());
         return nullptr;
     }
-    return device;
+    return device.release();
 }
 
 

src/libsync/propagateuploadv1.cpp

@@ -131,14 +131,16 @@ void PropagateUploadFileV1::startNextChunk()
     const QString fileName = propagator()->fullLocalPath(_item->_file);
     auto device = std::unique_ptr<UploadDevice>(new UploadDevice(
             fileName, chunkStart, currentChunkSize, &propagator()->_bandwidthManager));
+    // If the file is currently locked, we want to retry the sync
+    // when it becomes available again.
+    const auto lockMode = propagator()->syncOptions().requiredLockMode();
+    if (FileSystem::isFileLocked(fileName, lockMode)) {
+        emit propagator()->seenLockedFile(fileName, lockMode);
+        abortWithError(SyncFileItem::SoftError, tr("File is locked"));
+        return;
+    }
     if (!device->open(QIODevice::ReadOnly)) {
         qCWarning(lcPropagateUploadV1) << "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)) {
-            emit propagator()->seenLockedFile(fileName);
-        }
         // Soft error because this is likely caused by the user modifying his files while syncing
         abortWithError(SyncFileItem::SoftError, device->errorString());
         return;

src/libsync/syncengine.h

@@ -168,7 +168,7 @@ class OWNCLOUDSYNC_EXPORT SyncEngine : public QObject
      *
      * Forwarded from OwncloudPropagator::seenLockedFile.
      */
-    void seenLockedFile(const QString &fileName);
+    void seenLockedFile(const QString &fileName, FileSystem::LockMode mode);
 
 private slots:
     void slotFolderDiscovered(bool local, const QString &folder);

src/libsync/syncoptions.h

@@ -15,6 +15,7 @@
 #pragma once
 
 #include "owncloudlib.h"
+#include "common/filesystembase.h"
 #include "common/vfs.h"
 
 #include <QRegularExpression>
@@ -104,6 +105,15 @@ class OWNCLOUDSYNC_EXPORT SyncOptions
      */
     void setPathPattern(const QString &pattern);
 
+
+    /**
+     * The required file lock mode
+     */
+    FileSystem::LockMode requiredLockMode() const
+    {
+        return _vfs->mode() == Vfs::WindowsCfApi ? FileSystem::LockMode::Exclusive : FileSystem::LockMode::Shared;
+    }
+
 private:
     /**
      * Only sync files that mathc the expression

test/testlockedfiles.cpp

@@ -61,8 +61,8 @@ private slots:
         }
         QVERIFY(QFile::exists(tmpFile));
 
-        QVERIFY(!FileSystem::isFileLocked(tmpFile));
-        watcher.addFile(tmpFile);
+        QVERIFY(!FileSystem::isFileLocked(tmpFile, FileSystem::LockMode::Shared));
+        watcher.addFile(tmpFile, FileSystem::LockMode::Shared);
         QVERIFY(watcher.contains(tmpFile));
 
         QEventLoop loop;
@@ -75,8 +75,8 @@ private slots:
 
 #ifdef Q_OS_WIN
         auto h = makeHandle(tmpFile, 0);
-        QVERIFY(FileSystem::isFileLocked(tmpFile));
-        watcher.addFile(tmpFile);
+        QVERIFY(FileSystem::isFileLocked(tmpFile, FileSystem::LockMode::Shared));
+        watcher.addFile(tmpFile, FileSystem::LockMode::Shared);
 
         count = 0;
         file.clear();
@@ -88,7 +88,7 @@ private slots:
         QVERIFY(watcher.contains(tmpFile));
 
         CloseHandle(h);
-        QVERIFY(!FileSystem::isFileLocked(tmpFile));
+        QVERIFY(!FileSystem::isFileLocked(tmpFile, FileSystem::LockMode::Shared));
 
         QThread::msleep(120);
         qApp->processEvents();