fix(filesystem): do not attempt to lock directories on Windows - #10487
Conversation
|
/backport to stable-34.0 |
|
thanks @andreasbohl 馃檶馃徎 |
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 nextcloud#10444 Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andreas Bohl <ab@eeloy.com>
08eb545 to
f747ff6
Compare
|
Artifact containing the AppImage: nextcloud-appimage-pr-10487.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
|
/backport to stable-33.0 please |
|
/backport to stable-4.0 please |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
4 participants
Resolves
Closes #10444 - fixes the log flooding reported there.
Summary
On Windows,
isFileLocked()is called for every entry discovery walks over, directories included.LockFile()locks a byte range within a file and is not supported for directory handles, where it always fails withERROR_INVALID_PARAMETER. That call is reached at all becauseFILE_FLAG_BACKUP_SEMANTICSletsCreateFileW()open directories.This returns the opened handle for directories instead of attempting the lock.
CreateFileW()still runs, so a directory another process holds with deny-sharing is still reported as locked, only the attempt that cannot succeed is gone.Measured on a synced folder with ~80,000 directories, same build and configuration, one full discovery run each: ~80,000 warnings without the change, none with it. The added test asserts both halves and fails without the fix.
I could not reproduce the crash from the issue itself,
isFileLocked()returnsfalseafter logging. But we did hit on our clients the log flooding and the same errors.Checklist
AI (if applicable)