Prevent crash on non-unique diffable data source item identifiers - #1585
Open
Matthias-Huehne-Kiteworks wants to merge 1 commit into
Open
Matthias-Huehne-Kiteworks wants to merge 1 commit into
Matthias-Huehne-Kiteworks wants to merge 1 commit into
Conversation
…e diffable item identifiers
CollectionViewSection populated the diffable data source snapshot by appending
all wrapped references from a section's data source unconditionally. If the same
item identifier appeared twice, UICollectionViewDiffableDataSource raised a fatal
NSInternalInconsistencyException ("supplied item identifiers are not unique") and
crashed the app.
An item's diffable identifier is its OCItem.localID (OCItem+OCDataItem.m:
dataItemReference returns self.localID). So the crash means two entries resolving
to the same localID landed in one snapshot. This is the render-layer symptom of a
sync-engine anomaly: removed placeholder items that stayed stuck with active
upload / dangling sync records (fixed at the source in ios-sdk#172). Because such
removed items were only suppressed from query results while in the process of
*deletion* (OCItemSyncActivityDeleting) - not while stuck uploading - a removed
placeholder could linger in / re-appear in a query alongside a re-merged instance
sharing the same localID, producing a duplicate identifier.
Fix (defensive backstop at the UI layer):
- Deduplicate wrapped items before appending them to the snapshot, seeding the
seen-set from the identifiers already in the snapshot so uniqueness holds across
the entire snapshot (not just within one section). Duplicates are dropped and a
warning is logged. The drop is lossless: the surviving reference resolves to the
exact same item (content is looked up lazily by reference), and the check keys on
(localID, section) via WrappedItem equality, so the same item legitimately shown
in two different sections is preserved.
This complements the SDK fix rather than replacing it: ios-sdk#172 removes the
source of the duplicate (no more stuck removed-placeholders with active upload
records), while this change keeps a single stray duplicate from ever crashing the
UI and surfaces it via a log warning.
Also bumps the ios-sdk submodule to 715758b3 (fix/stuck-sync-loop), which carries
the corresponding sync-engine fix.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Problem
The app could crash with a fatal
NSInternalInconsistencyException— "supplied item identifiers are not unique" — raised byUICollectionViewDiffableDataSourcewhen a snapshot was applied.CollectionViewSectionappended all wrapped references from a section's data source to the snapshot unconditionally, so if the same identifier appeared twice, the apply crashed the app.Why it happens / how it relates to the SDK fix
A file's diffable identifier is its
OCItem.localID(OCItem+OCDataItem.m→dataItemReferencereturnsself.localID). So the crash means two entries resolving to the samelocalIDlanded in one snapshot.This is the render-layer symptom of a sync-engine anomaly fixed at the source in owncloud/ios-sdk#172 (endless upload loop for removed placeholder items):
OCItemSyncActivityDeleting) — not while stuck uploading.localID→ duplicate identifier → crash.The two changes are complementary:
Change
In
CollectionViewSectionsnapshot population, deduplicate wrapped items before appending them:Log.warningis emitted for observability.Why dropping is correct and lossless:
ItemRefis only an identity token; the cell content is resolved lazily by reference (dataSource.record(forItemRef:)). The surviving reference therefore resolves to the exact same item — no displayable content is lost, only a duplicate, content-identical phantom row.(localID, section)viaWrappedItemequality, so the same item legitimately shown in two different sections (e.g. a folder listing and a "recents"-style section) is preserved.The authoritative fix for duplicate identifiers stays at the data source / SDK level; this is intentionally a robustness net, not a place to "merge" or pick a winner.
Dependency
Bumps the
ios-sdksubmodule to715758b3(fix/stuck-sync-loop), which carries the corresponding sync-engine fix. Once owncloud/ios-sdk#172 is merged, the submodule pointer should be updated to the merged SDK commit.Validation
Built and deployed to a physical device that was reproducing the stuck-sync state. After resolving the recurring issue dialog once, the sync backlog drains and the UI no longer crashes. The dedup path additionally protects against any future stray duplicate.