Repository navigation
fix(sync): prevent client sync stalls from empty PING responses and undrained backlogs - #103
Merged
ralflang merged 2 commits intoAug 26, 2026
Merged
Conversation
When a parallel request modified the sync cache during a PING (COLLECTION_ERR_STALE), the handler exited without emitting any WBXML, returning HTTP 200 with a 0-byte body. MS-ASCMD requires every PING response to carry a status, and some clients (e.g. Nine) treat the empty body as a dead connection, back off, and stall their sync loop. Fall through to the normal response path so the client receives STATUS_NOCHANGES and re-issues the PING cleanly.
When a windowed SYNC response ships MOREAVAILABLE but the client never returns to continue draining (e.g. its sync loop died on a transient connection error), the PING watermark only tracks new activity and never reports the old backlog, so the pending items stay undelivered until unrelated new mail arrives. Track a per-collection backlog flag in the sync cache whenever a response is windowed, clear it when a SYNC completes without pending changes, and let pollForChanges report the collection as changed when the flag has not been refreshed for BACKLOG_GRACE_PERIOD seconds. A BACKLOG_TRIGGER_MAX cap prevents a stale flag from producing an endless PING/SYNC loop; the counter resets on every windowed response. Also reset $forceChanges per collection iteration in the Sync handler; it previously leaked from one collection to the next.
Contributor
Author
|
@ralflang : ready for review and merge |
🔍 CI ResultsOverall: ❌ 12/12 lanes failed TL;DR: ❌ Quality issues: PHPStan: 243 unique errors in 11 lanes; PHP-CS-Fixer: 31 files. Summary by PHP Version
Quality Metrics
❌ Failed Lanesphp8.0-dev
php8.0-stable
php8.1-dev
php8.1-stable
php8.2-dev
php8.2-stable
php8.3-dev
php8.3-stable
php8.4-dev
php8.4-stable
php8.5-dev
php8.5-stable
CI powered by horde-components • View full results |
Contributor
Author
|
Ping @ralflang |
1 similar comment
Contributor
Author
|
Ping @ralflang |
ralflang
approved these changes
Aug 26, 2026
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
2 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.
Summary
invalidated the sync cache (
COLLECTION_ERR_STALE); it now emits a validStatus-1 WBXML response as required by MS-ASCMD.
last response shipped MOREAVAILABLE and the client never returned, PING
reports the collection as changed so the client resumes its SYNC loop.
Motivation
Addresses #102. Device logs show a repeating stall pattern with Nine for
Android: a transient connection error aborts the client's windowed SYNC loop
mid-drain, the subsequent PING gets an empty 200 response (interpreted as a
dead connection, triggering back-off), and once the client finally re-PINGs,
the server correctly-but-unhelpfully reports "no changes" because the ping
watermark only tracks new activity — the undrained backlog stays stuck until
unrelated new mail arrives.
Changes
Request/Ping.php: fall through to the normal response path onCOLLECTION_ERR_STALEinstead of returning without output.SyncCache.php: new per-collection backlog flag with timestamp and triggercounter (
setBacklogFlag,resetBacklogFlag,hasStaleBacklog,getBacklogPingCount,incrementBacklogPingCount).Collections.php:pollForChanges()reports a collection as changed whenits backlog flag is older than
BACKLOG_GRACE_PERIOD(60s), capped atBACKLOG_TRIGGER_MAX(3) triggers per flag. An actively draining clientrefreshes the flag with every windowed response and is never affected.
Request/Sync.php: set the backlog flag on windowed/over-window responses,clear it when a sync completes without pending changes; reset
$forceChangesper collection iteration (previously leaked acrosscollections).
Expected exception types in new code paths: none added; existing
Horde_ActiveSync_Exceptionhandling is unchanged.Test plan
PingStaleCacheResponseTest: stale-cache PING produces a non-emptyWBXML body containing Status 1 (real encoder/decoder round-trip).
BacklogPingRecoveryTest: backlog flag lifecycle, grace periodbehavior, and trigger cap.
unrelated to this change).
syncing after the next PING cycle.