Skip to content

Commit 715758b

Browse files
[fix/stuck-sync-loop] Break endless upload loop for removed placeholder items
The sync pipeline could get stuck in an endless loop: the Status view kept showing repeating "[CORE, Replay] Found removedItems=(…)" entries for placeholder items (synthetic "_placeholder_" fileID/eTag) that were flagged `removed` but still carried active upload sync records, while their filenames accumulated ever more timestamp suffixes. Resolving the recurring Cancel/Retry issue dialog did not break the loop. Root cause: when an item that still had a pending/queued upload was removed, several gaps combined into a permanent deadlock: - OCSyncActionUpload rescheduled uploads for removed items, hit the "already exists" case, raised a keep-both issue and looped. - OCSyncActionDelete left the item's pending upload records running. - scrubItemSyncStatus skipped removed items entirely, so a removed item with active-but-non-existent sync record IDs was never scrubbed and never vacuumed (activeSyncRecordIDs.count > 0) - a state it could never leave. - Deleting a placeholder issued a doomed server DELETE with a synthetic fileID. Fixes: - OCSyncActionUpload.m: scheduleWithContext: bails out early when the item has meanwhile been removed (looked up via retrieveCacheItemForLocalID:, which only returns non-removed items). It deschedules the record with OCErrorCancelled and returns ProcessNext instead of re-uploading. New helper _localItemHasBeenRemoved. This is the core loop-breaker. - OCCore+SyncEngine.m: scrubItemSyncStatus no longer ignores removed items. The existing dangling-record check still protects items with live records, so removed items whose sync records no longer exist now get their sync activity cleared, letting the Vacuum item policy purge them. This drains any pre-existing stuck state on next launch - no account reset required. - OCSyncActionDelete.m: preflightWithContext: cancels all pending upload sync records for the item (and contained items) via the new helper _cancelPendingUploadsForItem:excludingSyncRecordID:, dropping each cancelled record's reference before commit. scheduleWithContext: short-circuits placeholder deletions locally instead of issuing a server DELETE that would fail. Verified on a physical device carrying the actual stuck state: resolving the issue dialog once now cascades through the backlog, the lanes drain and the leftover items are scrubbed and vacuumed. The loop no longer recurs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7648ca8 commit 715758b

3 files changed

Lines changed: 123 additions & 14 deletions

File tree

‎ownCloudSDK/Core/Sync/Actions/Delete/OCSyncActionDelete.m‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ - (void)preflightWithContext:(OCSyncContext *)syncContext
7474
// Item itself
7575
[itemToDelete addSyncRecordID:syncContext.syncRecord.recordID activity:OCItemSyncActivityDeleting];
7676

77+
// Cancel any pending upload (transfer) sync records for the item being deleted. Otherwise a queued
78+
// or issue-parked upload keeps looping and - via keep-both issue resolution - recreates placeholders
79+
// after the item is already gone, leaving the sync pipeline stuck forever.
80+
[self _cancelPendingUploadsForItem:itemToDelete excludingSyncRecordID:syncContext.syncRecord.recordID];
81+
7782
syncContext.removedItems = @[ itemToDelete ];
7883

7984
// Contained (associated) items
@@ -87,6 +92,9 @@ - (void)preflightWithContext:(OCSyncContext *)syncContext
8792
{
8893
[item addSyncRecordID:syncContext.syncRecord.recordID activity:OCItemSyncActivityDeleting];
8994

95+
// Cancel pending uploads for contained items too
96+
[self _cancelPendingUploadsForItem:item excludingSyncRecordID:syncContext.syncRecord.recordID];
97+
9098
OCLogDebug(@"Preflight: delete contained %@", OCLogPrivate(item.path));
9199

92100
[removedItems addObject:item];
@@ -108,6 +116,51 @@ - (void)preflightWithContext:(OCSyncContext *)syncContext
108116
}
109117
}
110118

119+
- (void)_cancelPendingUploadsForItem:(OCItem *)item excludingSyncRecordID:(OCSyncRecordID)excludedSyncRecordID
120+
{
121+
// Cancels all pending upload (transfer) sync records referencing the given item. Runs inside the
122+
// (already protected) preflight sync block, so the internal _descheduleSyncRecord: is used directly.
123+
124+
// Enumerate the *current* sync record IDs from the database (self.localItem can be stale and miss
125+
// records that were added after it was fetched). Falls back to the item's own list if unavailable.
126+
__block NSArray<OCSyncRecordID> *activeSyncRecordIDs = item.activeSyncRecordIDs;
127+
128+
if (item.localID != nil)
129+
{
130+
[self.core.vault.database retrieveCacheItemForLocalID:item.localID completionHandler:^(OCDatabase *db, NSError *error, OCSyncAnchor syncAnchor, OCItem *currentItem) {
131+
if (currentItem.activeSyncRecordIDs != nil)
132+
{
133+
activeSyncRecordIDs = currentItem.activeSyncRecordIDs;
134+
}
135+
}];
136+
}
137+
138+
if ((activeSyncRecordIDs = [activeSyncRecordIDs copy]) == nil) { return; }
139+
140+
for (OCSyncRecordID syncRecordID in activeSyncRecordIDs)
141+
{
142+
if ((excludedSyncRecordID != nil) && [syncRecordID isEqual:excludedSyncRecordID]) { continue; }
143+
144+
__block OCSyncRecord *syncRecord = nil;
145+
146+
[self.core.vault.database retrieveSyncRecordForID:syncRecordID completionHandler:^(OCDatabase *db, NSError *error, OCSyncRecord *record) {
147+
syncRecord = record;
148+
}];
149+
150+
if ([syncRecord.actionIdentifier isEqual:OCSyncActionIdentifierUpload])
151+
{
152+
OCLogWarning(@"Cancelling pending upload (record %@) because item %@ is being deleted", syncRecord.recordID, OCLogPrivate(item.path));
153+
154+
// Drop the reference from this item instance *before* the delete's own item update is committed,
155+
// so no dangling sync record ID for the just-cancelled upload is persisted. (Fix B scrubs any
156+
// that might still slip through, e.g. on other item instances.)
157+
[item removeSyncRecordID:syncRecordID activity:OCItemSyncActivityUploading];
158+
159+
[self.core _descheduleSyncRecord:syncRecord completeWithError:OCError(OCErrorCancelled) parameter:nil];
160+
}
161+
}
162+
}
163+
111164
- (void)descheduleWithContext:(OCSyncContext *)syncContext
112165
{
113166
OCItem *itemToRestore;
@@ -161,6 +214,29 @@ - (OCCoreSyncInstruction)scheduleWithContext:(OCSyncContext *)syncContext
161214

162215
if ((item = self.archivedServerItem) != nil)
163216
{
217+
// Placeholder items exist only locally (they were never uploaded to the server). There is nothing
218+
// to delete remotely - and issuing a server DELETE with a synthetic "_placeholder_" fileID would
219+
// just fail (and could get stuck on a delete issue). Complete the deletion locally instead.
220+
if (item.isPlaceholder)
221+
{
222+
OCLogDebug(@"Completing deletion of placeholder item %@ locally (never existed on the server)", OCLogPrivate(self.localItem.path));
223+
224+
// Notify the caller / result handler that the (local) deletion succeeded (mirrors the server-result path)
225+
[syncContext completeWithError:nil core:self.core item:self.localItem parameter:self.localItem];
226+
227+
[self.localItem removeSyncRecordID:syncContext.syncRecord.recordID activity:OCItemSyncActivityDeleting];
228+
syncContext.removedItems = @[ self.localItem ];
229+
230+
// Remove file(s) locally
231+
[self.core deleteDirectoryForItem:self.localItem];
232+
233+
// Action complete and can be removed (any contained/associated items were already soft-removed
234+
// during preflight; their dangling delete record references are cleaned up by sync status scrubbing)
235+
[syncContext transitionToState:OCSyncRecordStateCompleted withWaitConditions:nil];
236+
237+
return (OCCoreSyncInstructionDeleteLast);
238+
}
239+
164240
OCProgress *progress;
165241

166242
if ((progress = [self.core.connection deleteItem:item requireMatch:self.requireMatch resultTarget:[self.core _eventTargetWithSyncRecord:syncContext.syncRecord]]) != nil)

‎ownCloudSDK/Core/Sync/Actions/Upload/OCSyncActionUpload.m‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,19 @@ - (OCCoreSyncInstruction)scheduleWithContext:(OCSyncContext *)syncContext
157157
OCItem *parentItem, *uploadItem;
158158
NSURL *uploadURL;
159159

160+
// Loop-breaker: if the item to upload has meanwhile been removed (f.ex. deleted locally while the
161+
// upload was still queued or parked on an issue), there is nothing left to upload. Re-uploading it
162+
// would recreate a placeholder and - via keep-both issue resolution - spin up an endless sync loop.
163+
// Cancel the record instead and move on.
164+
if ([self _localItemHasBeenRemoved])
165+
{
166+
OCLogWarning(@"Cancelling upload of removed item (name=%@, localID=%@): item no longer exists in cache", self.localItem.name, self.localItem.localID);
167+
168+
[self.core _descheduleSyncRecord:syncContext.syncRecord completeWithError:OCError(OCErrorCancelled) parameter:nil];
169+
170+
return (OCCoreSyncInstructionProcessNext);
171+
}
172+
160173
if (((remoteFileName = self.filename) != nil) &&
161174
((parentItem = self.parentItem) != nil) &&
162175
((uploadItem = self.localItem) != nil) &&
@@ -406,6 +419,25 @@ - (OCCoreSyncInstruction)handleResultWithContext:(OCSyncContext *)syncContext
406419
return (resultInstruction);
407420
}
408421

422+
#pragma mark - Removal check
423+
- (BOOL)_localItemHasBeenRemoved
424+
{
425+
// Determine the current state of the item to upload from the database (rather than the archived,
426+
// possibly stale self.localItem). retrieveCacheItemForLocalID: only returns non-removed items, so a
427+
// nil result means the item has been removed (or hard-deleted) in the meantime.
428+
__block BOOL removed = NO;
429+
OCLocalID localItemLocalID;
430+
431+
if ((localItemLocalID = self.localItem.localID) != nil)
432+
{
433+
[self.core.vault.database retrieveCacheItemForLocalID:localItemLocalID completionHandler:^(OCDatabase *db, NSError *error, OCSyncAnchor syncAnchor, OCItem *item) {
434+
removed = ((item == nil) || item.removed);
435+
}];
436+
}
437+
438+
return (removed);
439+
}
440+
409441
#pragma mark - Issue resolution
410442
- (OCItem *)_preExistingItem
411443
{

‎ownCloudSDK/Core/Sync/OCCore+SyncEngine.m‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2077,21 +2077,22 @@ - (void)scrubItemSyncStatus
20772077
{
20782078
for (OCItem *item in items)
20792079
{
2080-
// Ignore removed items
2081-
if (!item.removed)
2080+
// Check if item has any sync record ID of an actually existing sync record
2081+
if (![syncRecordIDs intersectsSet:[NSSet setWithArray:item.activeSyncRecordIDs]])
20822082
{
2083-
// Check if item has any sync record ID of an actually existing sync record
2084-
if (![syncRecordIDs intersectsSet:[NSSet setWithArray:item.activeSyncRecordIDs]])
2085-
{
2086-
// No valid sync record IDs
2087-
OCLogWarning(@"Resetting sync information for %@ (live sync records: %@)", item, syncRecordIDs);
2088-
2089-
item.activeSyncRecordIDs = nil;
2090-
item.syncActivityCounts = nil;
2091-
item.syncActivity = OCItemSyncActivityNone;
2092-
2093-
[updateItems addObject:item];
2094-
}
2083+
// No valid sync record IDs => the sync records referenced by this item no longer exist.
2084+
// This also covers removed items that still carry dangling sync record IDs: previously
2085+
// they were ignored here, so a removed item with active-but-non-existent sync records was
2086+
// never scrubbed AND never vacuumed (activeSyncRecordIDs.count > 0) - a permanent deadlock
2087+
// that keeps the sync pipeline busy. Clearing the sync activity here lets the Vacuum item
2088+
// policy purge such removed items.
2089+
OCLogWarning(@"Resetting sync information for %@ (removed=%d, live sync records: %@)", item, item.removed, syncRecordIDs);
2090+
2091+
item.activeSyncRecordIDs = nil;
2092+
item.syncActivityCounts = nil;
2093+
item.syncActivity = OCItemSyncActivityNone;
2094+
2095+
[updateItems addObject:item];
20952096
}
20962097
}
20972098
}

0 commit comments

Comments
 (0)