From bbbc1eb14dc5b2d7e284b203beb6049deb2675ef Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Tue, 22 Jun 2021 09:26:23 +0200 Subject: [PATCH] Check a wehter file is locked before we move Fixes: #8765, #8766 --- changelog/unreleased/8761 | 4 ++++ src/common/filesystembase.cpp | 33 +++++++++++++++++++---------- src/libsync/owncloudpropagator.cpp | 15 +++++++------ src/libsync/propagatedownload.cpp | 27 ++++++++++++++--------- src/libsync/propagateremotemove.cpp | 5 ----- src/libsync/propagatorjobs.cpp | 10 ++++++--- 6 files changed, 58 insertions(+), 36 deletions(-) diff --git a/changelog/unreleased/8761 b/changelog/unreleased/8761 index 52d6f09d949..112e851c9d1 100644 --- a/changelog/unreleased/8761 +++ b/changelog/unreleased/8761 @@ -4,3 +4,7 @@ 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 +https://github.com/owncloud/client/issues/8765 +https://github.com/owncloud/client/issues/8766 +https://github.com/owncloud/client/pull/8763 +https://github.com/owncloud/client/pull/8768 diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index 8cb1b53dd7b..4ae24c87054 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -133,10 +133,13 @@ bool FileSystem::rename(const QString &originFileName, bool success = false; QString error; #ifdef Q_OS_WIN - QString orig = longWinPath(originFileName); - QString dest = longWinPath(destinationFileName); - - if (isLnkFile(originFileName) || isLnkFile(destinationFileName)) { + const QString orig = longWinPath(originFileName); + const QString dest = longWinPath(destinationFileName); + if (FileSystem::isFileLocked(dest, FileSystem::LockMode::Exclusive)) { + error = QCoreApplication::translate("FileSystem", "Can't rename %1, the file is currently in use").arg(destinationFileName); + } else if (FileSystem::isFileLocked(orig, FileSystem::LockMode::Exclusive)) { + error = QCoreApplication::translate("FileSystem", "Can't rename %1, the file is currently in use").arg(originFileName); + } else if (isLnkFile(originFileName) || isLnkFile(destinationFileName)) { success = MoveFileEx((wchar_t *)orig.utf16(), (wchar_t *)dest.utf16(), MOVEFILE_COPY_ALLOWED | MOVEFILE_WRITE_THROUGH); @@ -168,6 +171,7 @@ bool FileSystem::uncheckedRenameReplace(const QString &originFileName, const QString &destinationFileName, QString *errorString) { + Q_ASSERT(errorString); #ifndef Q_OS_WIN bool success; QFile orig(originFileName); @@ -195,13 +199,20 @@ bool FileSystem::uncheckedRenameReplace(const QString &originFileName, if (!QFileInfo(destinationFileName).isWritable()) { setFileReadOnly(destinationFileName, false); } - - BOOL ok; - QString orig = longWinPath(originFileName); - QString dest = longWinPath(destinationFileName); - - ok = MoveFileEx((wchar_t *)orig.utf16(), - (wchar_t *)dest.utf16(), + const QString orig = longWinPath(originFileName); + const QString dest = longWinPath(destinationFileName); + if (FileSystem::isFileLocked(dest, FileSystem::LockMode::Exclusive)) { + *errorString = QCoreApplication::translate("FileSystem", "Can't rename %1, the file is currently in use").arg(destinationFileName); + qCWarning(lcFileSystem) << "Renaming failed: " << *errorString; + return false; + } + if (FileSystem::isFileLocked(orig, FileSystem::LockMode::Exclusive)) { + *errorString = QCoreApplication::translate("FileSystem", "Can't rename %1, the file is currently in use").arg(originFileName); + qCWarning(lcFileSystem) << "Renaming failed: " << *errorString; + return false; + } + const BOOL ok = MoveFileEx(reinterpret_cast(orig.utf16()), + reinterpret_cast(dest.utf16()), MOVEFILE_REPLACE_EXISTING + MOVEFILE_COPY_ALLOWED + MOVEFILE_WRITE_THROUGH); if (!ok) { *errorString = Utility::formatWinError(GetLastError()); diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 149dbe0485c..8dec507ebd9 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -691,15 +691,16 @@ bool OwncloudPropagator::createConflict(const SyncFileItemPtr &item, emit touchedFile(fn); emit touchedFile(conflictFilePath); + // If the file is locked, we want to retry this sync when it + // becomes available again. + if (FileSystem::isFileLocked(fn, FileSystem::LockMode::Exclusive)) { + emit seenLockedFile(fn, FileSystem::LockMode::Exclusive); + if (error) + *error = tr("File %1 is locked").arg(fn); + return false; + } if (!FileSystem::rename(fn, conflictFilePath, &renameError)) { // If the rename fails, don't replace it. - - // If the file is locked, we want to retry this sync when it - // becomes available again. - if (FileSystem::isFileLocked(fn, FileSystem::LockMode::Exclusive)) { - emit seenLockedFile(fn, FileSystem::LockMode::Exclusive); - } - if (error) *error = renameError; return false; diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index e2492254493..9b4c7ec3657 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -458,7 +458,14 @@ void PropagateDownloadFile::startDownload() done(SyncFileItem::NormalError, tr("File %1 can not be downloaded because of a local file name clash!").arg(QDir::toNativeSeparators(_item->_file))); return; } - + // If the file is locked, we want to retry this sync when it + // becomes available again + const auto targetFile = propagator()->fullLocalPath(_item->_file); + if (FileSystem::isFileLocked(targetFile, FileSystem::LockMode::Exclusive)) { + emit propagator()->seenLockedFile(targetFile, FileSystem::LockMode::Exclusive); + done(SyncFileItem::SoftError, tr("File %1 is locked").arg(QDir::toNativeSeparators(_item->_file))); + return; + } propagator()->reportProgress(*_item, 0); QString tmpFileName; @@ -940,20 +947,20 @@ void PropagateDownloadFile::downloadFinished() return; } } + // If the file is locked, we want to retry this sync when it + // becomes available again + if (FileSystem::isFileLocked(fn, FileSystem::LockMode::Exclusive)) { + emit propagator()->seenLockedFile(fn, FileSystem::LockMode::Exclusive); + done(SyncFileItem::SoftError, tr("File %1 is locked").arg(fn)); + return; + } QString error; emit propagator()->touchedFile(fn); // The fileChanged() check is done above to generate better error messages. if (!FileSystem::uncheckedRenameReplace(_tmpFile.fileName(), fn, &error)) { - 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, FileSystem::LockMode::Exclusive)) { - emit propagator()->seenLockedFile(fn, FileSystem::LockMode::Exclusive); - } else { - propagator()->_anotherSyncNeeded = true; - } - + qCWarning(lcPropagateDownload) << "Rename failed:" << _tmpFile.fileName() << "=>" << fn; + propagator()->_anotherSyncNeeded = true; done(SyncFileItem::SoftError, error); return; } diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index 7f45eb0509f..58d021ca7b3 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -80,9 +80,6 @@ void PropagateRemoteMove::start() QString origin = propagator()->adjustRenamedPath(_item->_file); qCDebug(lcPropagateRemoteMove) << origin << _item->_renameTarget; - - QString targetFile(propagator()->fullLocalPath(_item->_renameTarget)); - if (origin == _item->_renameTarget) { // The parent has been renamed already so there is nothing more to do. finalize(); @@ -147,8 +144,6 @@ void PropagateRemoteMove::start() << folderTargetAlt << "to" << folderTarget; } } - qCDebug(lcPropagateRemoteMove) << remoteSource << remoteDestination; - _job = new MoveJob(propagator()->account(), remoteSource, remoteDestination, this); connect(_job.data(), &MoveJob::finishedSignal, this, &PropagateRemoteMove::slotMoveJobFinished); propagator()->_activeJobList.append(this); diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 3e926588bee..39612dfa778 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -215,11 +215,15 @@ void PropagateLocalRename::start() // it would have to come out the localFileNameClash function done(SyncFileItem::NormalError, tr("File %1 can not be renamed to %2 because of a local file name clash") - .arg(QDir::toNativeSeparators(_item->_file)) - .arg(QDir::toNativeSeparators(_item->_renameTarget))); + .arg(QDir::toNativeSeparators(_item->_file), + QDir::toNativeSeparators(_item->_renameTarget))); + return; + } + if (FileSystem::isFileLocked(existingFile, FileSystem::LockMode::Exclusive)) { + emit propagator()->seenLockedFile(existingFile, FileSystem::LockMode::Exclusive); + done(SyncFileItem::SoftError, tr("Could not rename %1 to %2, file is locked").arg(existingFile, targetFile)); return; } - emit propagator()->touchedFile(existingFile); emit propagator()->touchedFile(targetFile); QString renameError;