Skip to content

Commit 08eb545

Browse files
committed
fix(filesystem): do not attempt to lock directories on Windows
LockFile() locks a byte range within a file and is not supported for directory handles, where it always fails with ERROR_INVALID_PARAMETER. FILE_FLAG_BACKUP_SEMANTICS lets CreateFileW() open directories, so isFileLocked() reached that failing call for every directory and logged a warning for each one. Discovery checks every entry, directories included, so a sync run logged one bogus warning per directory. On large folder trees those warnings dominate the log volume, and the resulting log rotation discards the records needed to diagnose actual problems. Return the opened handle for directories instead of attempting the lock. CreateFileW() still runs, so a directory held with deny-sharing is still reported as locked; only the attempt that cannot succeed is gone. Measured on a synced folder with 82464 directories, same build and configuration: a full discovery run logged 83189 of these warnings without this change and none with it. The added test covers both halves and fails without the fix. Test suite green on Windows (70 tests). For #10444 Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andreas Bohl <ab@eeloy.com>
1 parent 5cee756 commit 08eb545

2 files changed

Lines changed: 92 additions & 0 deletions

File tree

src/common/filesystembase.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,12 @@ Utility::Handle lockFile(const QString &fileName, FileSystem::LockMode mode)
702702
FILE_ATTRIBUTE_NORMAL | FILE_FLAG_BACKUP_SEMANTICS, nullptr)};
703703

704704
if (out) {
705+
if (attr & FILE_ATTRIBUTE_DIRECTORY) {
706+
// LockFile() is unsupported for directory handles and always fails there,
707+
// and opening the directory already ruled out a sharing violation.
708+
return out;
709+
}
710+
705711
LARGE_INTEGER start;
706712
start.QuadPart = 0;
707713
LARGE_INTEGER end;

test/testlockedfiles.cpp

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*/
1010

1111
#include <QtTest>
12+
#include <QDir>
1213
#include "syncenginetestutils.h"
1314
#include "lockwatcher.h"
1415
#include <syncengine.h>
@@ -34,6 +35,23 @@ HANDLE makeHandle(const QString &file, int shareMode)
3435
}
3536
return handle;
3637
}
38+
39+
// Same as makeHandle(), but FILE_FLAG_BACKUP_SEMANTICS is required to open a directory.
40+
HANDLE makeDirectoryHandle(const QString &directory, int shareMode)
41+
{
42+
const auto fName = FileSystem::longWinPath(directory);
43+
auto handle = CreateFileW(
44+
reinterpret_cast<const wchar_t *>(fName.utf16()),
45+
GENERIC_READ,
46+
shareMode,
47+
nullptr, OPEN_EXISTING,
48+
FILE_ATTRIBUTE_NORMAL | FILE_FLAG_BACKUP_SEMANTICS,
49+
nullptr);
50+
if (handle == INVALID_HANDLE_VALUE) {
51+
qWarning() << GetLastError();
52+
}
53+
return handle;
54+
}
3755
#endif
3856

3957
class TestLockedFiles : public QObject
@@ -112,6 +130,74 @@ private slots:
112130
}
113131

114132
#ifdef Q_OS_WIN
133+
void testDirectoryLockChecks()
134+
{
135+
QTemporaryDir tmp;
136+
QVERIFY(tmp.isValid());
137+
138+
const auto subDirectory = tmp.path() + QStringLiteral("/aDirectory");
139+
QVERIFY(QDir().mkpath(subDirectory));
140+
141+
const auto fileInDirectory = subDirectory + QStringLiteral("/aFile.txt");
142+
{
143+
QFile file(fileInDirectory);
144+
QVERIFY(file.open(QFile::WriteOnly));
145+
QVERIFY(file.write("Nextcloud"));
146+
}
147+
148+
const auto fileHandle = makeHandle(fileInDirectory, 0);
149+
QVERIFY(fileHandle != INVALID_HANDLE_VALUE);
150+
151+
// Logger only forwards fatal messages, so QTest::failOnWarning() would not see
152+
// these. Count by category, which survives rewording of the message.
153+
static int warningCount = 0;
154+
static QStringList warningMessages;
155+
warningCount = 0;
156+
warningMessages.clear();
157+
const auto previousHandler = qInstallMessageHandler([](QtMsgType type, const QMessageLogContext &context, const QString &message) {
158+
if (type == QtWarningMsg && context.category && qstrcmp(context.category, "nextcloud.sync.filesystem") == 0) {
159+
++warningCount;
160+
warningMessages.append(message);
161+
}
162+
});
163+
164+
// Every mode has to stay silent, but only SharedRead is asserted on: Exclusive
165+
// requests deny-sharing, so any unrelated handle an indexer or virus scanner
166+
// holds would rightfully make it report the directory as locked.
167+
QVarLengthArray<bool, 2> sharedReadResults;
168+
for (const auto mode : {FileSystem::LockMode::Shared, FileSystem::LockMode::SharedRead, FileSystem::LockMode::Exclusive}) {
169+
const auto subDirectoryLocked = FileSystem::isFileLocked(subDirectory, mode);
170+
const auto rootDirectoryLocked = FileSystem::isFileLocked(tmp.path(), mode);
171+
if (mode == FileSystem::LockMode::SharedRead) {
172+
sharedReadResults.append(subDirectoryLocked);
173+
sharedReadResults.append(rootDirectoryLocked);
174+
}
175+
}
176+
177+
// Skipping the lock on directories must not hide a locked file inside one.
178+
const auto lockedFileDetected = FileSystem::isFileLocked(fileInDirectory, FileSystem::LockMode::SharedRead);
179+
180+
// A directory held with deny-sharing is still locked; CreateFileW reports that.
181+
const auto directoryHandle = makeDirectoryHandle(subDirectory, 0);
182+
const auto sharedDirectoryDetected = directoryHandle != INVALID_HANDLE_VALUE
183+
&& FileSystem::isFileLocked(subDirectory, FileSystem::LockMode::SharedRead);
184+
185+
qInstallMessageHandler(previousHandler);
186+
if (directoryHandle != INVALID_HANDLE_VALUE) {
187+
CloseHandle(directoryHandle);
188+
}
189+
CloseHandle(fileHandle);
190+
191+
// The failing LockFile() call used to log one warning per directory per run.
192+
QVERIFY2(warningCount == 0, qPrintable(warningMessages.join(QStringLiteral(" || "))));
193+
for (const auto isLocked : sharedReadResults) {
194+
QVERIFY(!isLocked);
195+
}
196+
QVERIFY(lockedFileDetected);
197+
QVERIFY(sharedDirectoryDetected);
198+
QVERIFY(!FileSystem::isFileLocked(fileInDirectory, FileSystem::LockMode::SharedRead));
199+
}
200+
115201
void testLockedFilePropagation()
116202
{
117203
FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };

0 commit comments

Comments
 (0)