Skip to content

fix(files_versions): avoid version snapshot races during cross-storage renames#61254

Open
joshtrichards wants to merge 1 commit into
masterfrom
jtr/fix-crossStorageRenameOverlap
Open

fix(files_versions): avoid version snapshot races during cross-storage renames#61254
joshtrichards wants to merge 1 commit into
masterfrom
jtr/fix-crossStorageRenameOverlap

Conversation

@joshtrichards

@joshtrichards joshtrichards commented Jun 12, 2026

Copy link
Copy Markdown
Member
  • Resolves: # (possibly some existing bug reports; see user-visible symptoms list below)

Summary

Cross-storage moves already have dedicated version handling via VersionStorageMoveListener, which handles moving existing versions across backends.

At the same time, FileEventsListener::write_hook() can still call Storage::store() while that move is in progress. For moves between backends, that is redundant and can race with the move flow by snapshotting the source path while it is already being moved.

Changes:

  • track source paths for active cross-backend moves in FileEventsListener
  • skip Storage::store() in write_hook() for writes under those paths
  • clear the tracked path again when the move completes

I found this while investigating intermittent failures in apps/files_trashbin/tests/StorageTest.php::testKeepFileAndVersionsWhenMovingFileBetweenStorages. The existing test already covers the affected scenario; the problem is the race in the implementation, not missing scenario coverage.

Possible user-visible symptoms:

  • intermittent failure when moving a file between storages
  • generic move/file operation error caused by version creation racing with the move

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

…orage renames

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards

Copy link
Copy Markdown
Member Author

Example failure from an actual CI run (https://github.com/nextcloud/server/actions/runs/27333716311/job/80752712482?pr=61185):

There was 1 error:

1) OCA\Files_Trashbin\Tests\StorageTest::testKeepFileAndVersionsWhenMovingFileBetweenStorages
RuntimeException: Failed to copy to files_versions/test.txt.v1781170610 from cache with source data {...} 

/home/runner/actions-runner/_work/server/server/lib/private/Files/Cache/Cache.php:1262
...
/home/runner/actions-runner/_work/server/server/apps/files_versions/lib/Versions/LegacyVersionsBackend.php:163
/home/runner/actions-runner/_work/server/server/apps/files_versions/lib/Versions/VersionManager.php:96
/home/runner/actions-runner/_work/server/server/apps/files_versions/lib/Storage.php:217
/home/runner/actions-runner/_work/server/server/apps/files_versions/lib/Listener/FileEventsListener.php:210
/home/runner/actions-runner/_work/server/server/apps/files_versions/lib/Listener/FileEventsListener.php:83
...
/home/runner/actions-runner/_work/server/server/apps/files_trashbin/tests/StorageTest.php:507

@joshtrichards

Copy link
Copy Markdown
Member Author

/backport to stable34

@joshtrichards

Copy link
Copy Markdown
Member Author

/backport to stable33

@joshtrichards joshtrichards added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 12, 2026
@joshtrichards joshtrichards marked this pull request as ready for review June 12, 2026 17:34
@joshtrichards joshtrichards requested a review from a team as a code owner June 12, 2026 17:34
@joshtrichards joshtrichards requested review from ArtificialOwl, artonge, come-nc, leftybournes and salmart-dev and removed request for a team June 12, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant