fix: [SDK-4995] harden background notification ingress against ANRs

[fadi-george]

Description

One Line Summary

Harden cold-start notification receivers and background sync jobs against ANRs, stranded work, and queue-blocking failures.

Details

Motivation

Chronic background ANRs continued around FCMBroadcastReceiver despite best-effort dispatcher prewarming. Cold dispatcher initialization, unbounded receiver work, incomplete PendingResult.finish() paths, and SyncJobService lifecycle races could still block Android component deadlines or strand notification work.

Scope

• Gate dispatcher initialization so first dispatch does not construct pools on the caller thread, and recover cleanly if primary or fallback bootstrap fails.
• Add an isolated ingress lane and durable SQLite journal for FCM and notification-dismiss events.
• Wait for WorkManager to persist each drain request before completing the broadcast handoff.
• Chain drain work with APPEND_OR_REPLACE so records arriving during an active drain always receive successor work.
• Isolate failures per journal record. Unknown or expired records are removed, and repeatedly failing records are dropped after 3 attempts or 24 hours without blocking later notifications.
• Guarantee exact-once broadcast completion with an 8-second off-main deadline for FCM, dismiss, boot, and upgrade handoffs.
• Match SyncJobService callbacks by stable job ID and make completion and cancellation ownership race-safe.
• Reset notification generation and restoration enqueue guards when WorkManager rejects a request.
• Add OneSignalDispatchers.launchOnIngress as the shared ingress dispatch entry point. There are no customer-facing API changes.

Testing

Unit testing

Added coverage for:

• Cold concurrent dispatcher initialization and bootstrap fallback failure.
• Cancelled queued ingress completion and off-main broadcast deadlines.
• Durable scheduling acceptance and drain chaining.
• Duplicate ingress deduplication without resetting retry counts.
• Unknown, expired, and repeatedly failing record isolation.
• WorkManager enqueue rejection recovery.
• SyncJobService callbacks using distinct JobParameters instances with matching or different job IDs.

Validated with:

• ./gradlew spotlessCheck detekt --console=plain
• ./coverage/checkCoverage.sh
• Full pull request CI, including unit tests, code quality, demo build, CodeQL, and diff coverage.

Changed-line coverage is 86.9% against an 80% requirement.

Manual testing

Installed the local GMS demo on emulator-5556, queued offline background work, and forced JobScheduler job 2071862118. The sync ran on OneSignal-IO-1, completed through jobFinished, and produced no ANR or crash.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
• I have reviewed this PR myself, ensuring it meets each checklist item

[github-actions]

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff).

Changed Files Coverage

• OneSignalDispatchers.kt: 214/245 touched executable lines (87.3%) (370 touched lines in diff)
• ThreadUtils.kt: 9/20 touched executable lines (45.0%) (31 touched lines in diff)
  • 11 uncovered touched lines in this file
• SyncJobService.kt: 32/42 touched executable lines (76.2%) (60 touched lines in diff)
  • 10 uncovered touched lines in this file
• NotificationGenerationWorkManager.kt: 21/24 touched executable lines (87.5%) (28 touched lines in diff)
• NotificationIngress.kt: 190/214 touched executable lines (88.8%) (424 touched lines in diff)
• NotificationRestoreWorkManager.kt: 17/17 touched executable lines (100.0%) (25 touched lines in diff)
• BootUpReceiver.kt: 5/5 touched executable lines (100.0%) (7 touched lines in diff)
• BroadcastCompletion.kt: 32/35 touched executable lines (91.4%) (63 touched lines in diff)
• FCMBroadcastReceiver.kt: 7/7 touched executable lines (100.0%) (9 touched lines in diff)
• NotificationDismissReceiver.kt: 5/5 touched executable lines (100.0%) (6 touched lines in diff)
• UpgradeReceiver.kt: 5/5 touched executable lines (100.0%) (7 touched lines in diff)
• ⚠️ IOMockHelper.kt: Not in coverage report (may not be compiled/tested)

Overall (aggregate gate)

537/619 touched executable lines covered (86.8% — requires ≥ 80%)

Per-file detail (informational; gate is aggregate above):

• ThreadUtils.kt: 45.0% (11 uncovered touched lines)

• SyncJobService.kt: 76.2% (10 uncovered touched lines)

📥 View workflow run

[cursor]

Multi-model adversarial review (interrogate)

Verdict: request changes. The durable-ingress direction matches the ANR/lost-work intent, but several paths can still strand or drop work, and SyncJobService stop ownership looks broken in production.

Intent

Harden cold-start notification receivers and background sync against ANRs and lost work via gated dispatcher init, a durable FCM/dismiss journal + WorkManager drain, exact-once broadcast completion, and race-safe SyncJobService ownership — without changing customer-facing notification APIs.

Reviewers

• A: claude-fable-5-thinking-xhigh — 14 findings (2 critical)
• B: gpt-5.6-sol-xhigh — 8 findings (5 critical)
• C: cursor-grok-4.5-high-fast — 11 findings (3 critical)
• D: claude-opus-5-thinking-xhigh — 22 findings (6 critical)

Act On

1. ExistingWorkPolicy.KEEP + one-shot store.list() strands mid-drain inserts (A/B/C/D) — drain can finish while newer journal rows have no successor work.
2. SyncJobService.onStopJob uses JobParameters identity (===) (B/D) — Binder-delivered params are distinct instances; stop likely never cancels/reschedules. Tests reuse one mock and miss this.
3. No poison-record / attempt isolation in drain (A/D) — one bad row or IngressKind.valueOf failure head-of-line blocks the queue with unbounded Result.retry().
4. FCM/dismiss handoffs have no timeoutMs (A/B/C/D) — ANR-sensitive receivers rely only on invokeOnCompletion; Boot/Upgrade uniquely get the 8s deadline.
5. Broadcast completes before drain enqueue is durable (B/C) — scheduleDrainBestEffort fire-and-forgets on IO; process death after finish() can leave journal rows unscheduled until a later startup/ingress.

Consider

• Delete-after-process without durable downstream ack → possible double-display on crash (A/C).
• Journal put failure is swallowed → silent drop with no inline fallback (A/D).
• Bootstrap thread has no finally around initialize(); fallback throw can wedge running=true (A/B/D).
• goAsync() makes setAbort() / setSuccessfulResultCode() dead; abort criterion also changed to “persisted” (A/C/D).
• Missing drain/KEEP/poison/JobParameters-distinct tests for the headline guarantees (A/C/D).

Noted

Extra WM hop latency; SerialIO/Ingress now share a 200-cap queue; restore not actually journaled; BundleCodec lossiness; companion restored static; production @ForTest seams.

Dismissed

• “Rewrite dispatchers simpler” as a ship-blocker — complexity is high, but the gated warm path is the stated ANR fix; treat as follow-up hardening, not a revert.
• HMS/ADM not on the journal path — out of this PR’s stated FCM/dismiss scope.
• Dismiss hashCode id collisions — low practical risk given idempotent dismiss processing.

Agreement map

Strongest consensus: KEEP/drain backlog race, missing FCM/dismiss deadlines, and weak verification of drain semantics. Independent high-severity hits from B+D on JobParameters ===. Divergence mostly on how hard to push structural simplification vs. surgical durability fixes.

Inline comments cover the Act On items.

  

_{Sent by Cursor Automation: Untitled}

[abdulraqeeb33]

Hey @fadi-george , went through this PR.

The direction is right. goAsync() doesn’t extend the ~10s budget, and prewarm() doesn’t help if IO is already stuck on init HTTP. Persist cheaply, finish the broadcast, drain off the IO pool. That’s the right shape.

What’s hard to review is that this is three changes:

1. Journal + Ingress lane + 8s deadline. This is the ANR fix, I’d keep it.
2. The GateDispatcher rewrite. If FCM isn’t on IO anymore this isn’t required. I’d split it to a follow-up or drop it.
3. SyncJobService matching on jobId. Small and independent, fine on its own.

One thing I’d still want in (1): timeout enqueueDrain()?.result?.get(). The 8s deadline finishes this PendingResult, but Ingress is a single thread, so a hung WorkManager wait still blocks later pushes.

Can we put (1) behind a flag with the existing Turbine flags, APP_STARTUP, default off, persist the last value so the next process can read it without waiting on init. Don’t use an IMMEDIATE remote flag on cold FCM. That fetch is part of the work we’re trying to get off this path. I wouldn’t flag (2). Flagging (1) also means we keep both receiver paths until we delete it. You could also create a new class where the conditional happens around what path to use.

Impact I’m thinking about: fewer receiver ANRs, but slower/later display from the SQLite + WM hop. Drain is a default OneTimeWorkRequest with no expedited, so on Tecno/Infinix/Xiaomi this can delay or drop, not just add latency. Crash after display and before journal delete can duplicate. HMS/ADM unchanged. No public API.

I’d rather land (1) behind the flag, split out (2)/(3), add the WM timeout, and decide on expedited before we roll it out.