Repository navigation
Make change delivery acknowledgements safe against late and concurrent requests - #11054
Merged
Merged
Conversation
removeChangeDeliverySessions(containerKey:) passed the lazy sequence which `map` returns on Realm results to an `IN` predicate. Realm rejects that with the Objective-C exception "IN clause requires an array of items", so prime() and reset() raised it whenever a session still existed for the container. `try?` does not catch it. Without the fix, the new test reports this exception as an unexpected failure. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
An enumerator acknowledges a change batch only after finishEnumeratingChanges returns: on the main actor for updates and on the reporting thread for trash deletions. Four enumerator tests asserted the database state written by that acknowledgement right after enumerateChanges() returned, so they failed whenever the assertion ran first. Measured on master, testTrashChangeEnumeration failed 7 of 40 runs and testWorkingSetChangesBatchMixingUpdatesAndDeletions 2 of 30. Wait for that state with an XCTNSPredicateExpectation instead, as RemoteChangePropagationTests already does. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
An enumerator acknowledges a change batch after finishEnumeratingChanges returns. File Provider can request the next batch before that, also on a new enumerator, and that request acknowledges the reported batch itself. Three defects made this unsafe: - acknowledgeChangeDeliveryBatch acknowledged whichever batch was pending, with the caller's deletion list. A late acknowledgement thus committed the next batch before it was delivered, and that batch's deleted items were never removed. This also happened across sessions on a reused enumerator. - It read the pending state outside the write transaction and applied the values it read inside it. - A continuation anchor was looked up with two queries, for the committed and for the pending anchor. An acknowledgement committing in between moved the anchor from one to the other, both queries missed it, and the request failed with NSFileProviderErrorSyncAnchorExpired. Identify a batch by its session and end position, check and apply its acknowledgement in one write transaction, and derive its deletions from the stored batch. Prepare a batch in one transaction as well and return a batch which is already pending, so a repeated request replays it. Look up a continuation anchor in one query. Remove the session when a stored change cannot be decoded, so no undelivered batch stays pending. testWorkingSetChangesResumeAcrossNewEnumeratorInstances failed 10 of 30 runs on master with NSFileProviderErrorSyncAnchorExpired and passed 40 of 40 with this change. The new late-acknowledgement tests in ChangeDeliveryBufferTests and the stale-acknowledgement test in FilesDatabaseManagerTests fail on master when adapted to its interface. The single-query lookup has no deterministic test, only the loop above. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
i2h3
added
os: 🍎 macOS
github-project-automation
Bot
moved this to 🧭 Planning evaluation (don't pick)
in 💻 Desktop Clients team
Collaborator
Author
|
/backport to stable-35.0 |
i2h3
moved this from 🧭 Planning evaluation (don't pick)
to 🏗️ In progress
in 💻 Desktop Clients team
|
claucambra
approved these changes
Oct 13, 2026
github-project-automation
Bot
moved this from 🏗️ In progress
to ☑️ Done
in 💻 Desktop Clients team
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The change delivery from #10822 reports a batch with finishEnumeratingChanges
and acknowledges it afterwards. File Provider can request the next batch before that acknowledgement runs, also on a new enumerator. This caused flakyEnumeratorTests` in CI, for example on #10981.Checklist
AI (if applicable)