Skip to content

fix(view): release cache refresh locks on errors#62345

Open
joshtrichards wants to merge 4 commits into
masterfrom
jtr/fix-View-getCacheEntry-error-handling
Open

fix(view): release cache refresh locks on errors#62345
joshtrichards wants to merge 4 commits into
masterfrom
jtr/fix-View-getCacheEntry-error-handling

Conversation

@joshtrichards

@joshtrichards joshtrichards commented Jul 20, 2026

Copy link
Copy Markdown
Member
  • Resolves: #

Summary

Fix View::getCacheEntry() so that, after lockFile() succeeds, release of its file and parent-directory refresh locks is attempted in finally.

Previously, the broad LockedException fallback also caught failures during refresh or lock release. That could skip cleanup and return cached metadata, leaving locks held until request shutdown and obscuring the cause of later lock contention.

Lock contention during shallow scanning or initial lock acquisition remains best-effort and returns the existing cache entry. Once lockFile() succeeds, however, getCacheEntry() owns the file and parent-directory refresh locks. A subsequent failure during refresh work or cleanup is no longer treated as initial lock contention: the method attempts release in finally and propagates the failure.

Follow-up ideas:

  • lockFile() does not roll back locks already acquired if acquiring a parent lock fails. Addressing that partial-acquisition cleanup remains out of scope. (Edit: implemented via fix(view): implement partial lock cleanup in lockFile #62349)
  • basicOperation() has similar lock-cleanup responsibilities and could probably benefit from a similar approach. In particular, its current cleanup is inside a catch, so an unlockFile() failure could obscure the original storage exception.

TODO

  • Check existing test assumptions
  • Consider expanding existing coverage
  • Decide about backporting: worthiness + risk(s)

Checklist

AI (if applicable)

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

Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
Assisted-by: GitHub Copilot:GPT-5

Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 21, 2026
@joshtrichards
joshtrichards marked this pull request as ready for review July 21, 2026 14:55
@joshtrichards
joshtrichards requested a review from a team as a code owner July 21, 2026 14:55
@joshtrichards
joshtrichards requested review from come-nc, leftybournes, nfebe and sorbaugh and removed request for a team July 21, 2026 14:55
@joshtrichards joshtrichards added this to the Nextcloud 35 milestone Jul 21, 2026
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.

2 participants