fix: [SDK-4794] prewarm dispatchers at cold-start entry points

[abdulraqeeb33]

Summary

Call OneSignalDispatchers.prewarm() at every cold-start entry point so the IO/Default/SerialIO dispatcher + executor + scope construction happens on the low-priority OneSignal-prewarm daemon instead of on the thread that handles the wake-up. Entry points covered:

• Receivers: FCMBroadcastReceiver, NotificationDismissReceiver, BootUpReceiver, UpgradeReceiver (before goAsync())
• ADM: ADMMessageHandler, ADMMessageHandlerJob (each callback)
• HMS: OneSignalHmsEventBridge.onNewToken / onMessageReceived
• Notification-open trampolines: NotificationOpenedActivityBase, NotificationOpenedActivityHMS
• Already present (unchanged): OneSignalImp.initWithContext / initWithContextSuspend, SyncJobService.onStartJob

prewarm() is idempotent and fire-and-forget, so the calls are cheap. The prewarm() KDoc is reworded to describe it as a best-effort head start, list the full entry-point table, and tell future contributors to prewarm any new entry point.

Why this approach over the two alternatives

Three options were considered for SDK-4507/SDK-4794 cold-start ANRs (the production OTel showed the first launchOnIO/launchOnSerialIO constructing the ThreadPoolExecutor synchronously on the main thread):

1. Wide — prewarm inside the ThreadUtils helpers (suspendifyOnIO, launchOnIO, …). One chokepoint, automatic for all background work.
Rejected: prewarm() is fire-and-forget, so a helper that prewarms and then immediately dispatches can still win the lazy-init race and construct the executor on the caller thread. It made the race more likely won but didn't actually fix it, and coupled every background dispatch to prewarm.

2. Durable — make the dispatcher defer cold construction off the caller thread (queue the first dispatch and replay it on the daemon so construction can never land on a caller thread, regardless of timing).
Rejected for now: it's a guaranteed fix, but it adds ~70 lines of lock/queue/CompletableJob concurrency code to the most safety-critical file in the SDK, with subtle serial-ordering / join() / cancellation semantics and a permanent review + maintenance cost. That is a large charge to close a gap we have not proven still fires.

3. Chosen — prewarm at the cold-start entry points. The expensive construction can only first hit a thread at a small, stable, enumerable set of doorways (the receivers / jobs / activities / bridges above); deep callers run after init on background threads. Prewarming at each doorway — where there is real lead time before the first dispatch (a goAsync() handoff or initWithContext work) — covers the actual production surface (the OTel ANRs were all at entry points) with ~10 one-line calls and zero new concurrency code.

Trade-off accepted: it is best-effort (a pathologically slow device could still lose the race), and a new entry point must remember to call prewarm() — both documented in the prewarm() KDoc. If telemetry later shows the race still firing, option #2 remains available as a follow-up.

Test plan

• SyncJobServiceTests — onStartJob calls prewarm() before suspendifyOnIO
• FCMBroadcastReceiverTests — prewarm fires for real FCM payloads, skipped for google.com/iid token-update intents
• PrewarmEntryPointTests — BootUpReceiver / UpgradeReceiver / NotificationDismissReceiver prewarm before dispatch
• OneSignalDispatchersTests — existing off-thread + idempotent prewarm coverage still passes
• Note: ADMMessageHandler(Job) and OneSignalHmsEventBridge have no unit-test path (the Amazon/Huawei base classes aren't on the test classpath); those calls are verified by inspection.

Made with Cursor

[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: 11/17 touched executable lines (64.7%) (57 touched lines in diff)
  • 6 uncovered touched lines in this file
• OneSignalImp.kt: 11/18 touched executable lines (61.1%) (44 touched lines in diff)
  • 7 uncovered touched lines in this file
• NotificationOpenedActivityHMS.kt: 0/1 touched executable lines (0.0%) (4 touched lines in diff)
  • 1 uncovered touched lines in this file
• NotificationOpenedActivityBase.kt: 0/1 touched executable lines (0.0%) (4 touched lines in diff)
  • 1 uncovered touched lines in this file
• OneSignalHmsEventBridge.kt: 0/2 touched executable lines (0.0%) (5 touched lines in diff)
  • 2 uncovered touched lines in this file
• BootUpReceiver.kt: 0/1 touched executable lines (0.0%) (5 touched lines in diff)
  • 1 uncovered touched lines in this file
• FCMBroadcastReceiver.kt: 0/1 touched executable lines (0.0%) (6 touched lines in diff)
  • 1 uncovered touched lines in this file
• NotificationDismissReceiver.kt: 0/1 touched executable lines (0.0%) (5 touched lines in diff)
  • 1 uncovered touched lines in this file
• UpgradeReceiver.kt: 0/1 touched executable lines (0.0%) (5 touched lines in diff)
  • 1 uncovered touched lines in this file
• ADMMessageHandler.kt: 0/3 touched executable lines (0.0%) (5 touched lines in diff)
  • 3 uncovered touched lines in this file
• ADMMessageHandlerJob.kt: 0/3 touched executable lines (0.0%) (5 touched lines in diff)
  • 3 uncovered touched lines in this file
• ⚠️ IOMockHelper.kt: Not in coverage report (may not be compiled/tested)

Overall (aggregate gate)

22/49 touched executable lines covered (44.9% — requires ≥ 80%)

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

• OneSignalDispatchers.kt: 64.7% (6 uncovered touched lines)

• OneSignalImp.kt: 61.1% (7 uncovered touched lines)

• NotificationOpenedActivityHMS.kt: 0.0% (1 uncovered touched lines)

• NotificationOpenedActivityBase.kt: 0.0% (1 uncovered touched lines)

• OneSignalHmsEventBridge.kt: 0.0% (2 uncovered touched lines)

• BootUpReceiver.kt: 0.0% (1 uncovered touched lines)

• FCMBroadcastReceiver.kt: 0.0% (1 uncovered touched lines)

• NotificationDismissReceiver.kt: 0.0% (1 uncovered touched lines)

• UpgradeReceiver.kt: 0.0% (1 uncovered touched lines)

• ADMMessageHandler.kt: 0.0% (3 uncovered touched lines)

• ADMMessageHandlerJob.kt: 0.0% (3 uncovered touched lines)

❌ Coverage Check Failed

Aggregate coverage on touched lines is 44.9% (minimum 80%).

📥 View workflow run

[fadi-george]

@claude review

Taking a deeper look

[claude]

LGTM — mechanical extension of the existing idempotent prewarm() to the common ThreadUtils entry points.

Extended reasoning...

Overview

This PR adds a single OneSignalDispatchers.prewarm() call to six ThreadUtils entry points (suspendifyOnMain, suspendifyOnSerialIO, suspendifyWithCompletion, suspendifyWithErrorHandling, launchOnIO, launchOnDefault) and one explicit call at FCMBroadcastReceiver.onReceive before goAsync(). KDoc on prewarm() is updated to document the explicit early callsites. Tests are added/updated in ThreadUtilsFeatureFlagTests, SyncJobServiceTests, and a new FCMBroadcastReceiverTests.

Security risks

None. This is a performance/startup optimization that shifts dispatcher cold-init off the main thread. No auth, crypto, permissions, IPC parsing, or data exposure surface is touched. The FCM receiver continues to short-circuit on google.com/iid token-update intents before the prewarm call, matching prior behavior.

Level of scrutiny

Low. Although the file touches threading infrastructure (which would normally warrant high scrutiny), each individual edit is a one-line invocation of a function that already exists, is documented and verified idempotent (@Volatile prewarmStarted guard + double-checked lock), is fire-and-forget (work runs on a separate OneSignal-prewarm daemon thread at NORM_PRIORITY - 2), and swallows failures with a logged fallback. The behavioral change is just "warm dispatchers a bit earlier in more code paths."

Other factors

• The only bug surfaced is a test-consistency Nit about pre-existing BackgroundManagerTests not stubbing the new prewarm() call on mockkObject(OneSignalDispatchers) — the inline comment notes the race is impractical and CI passes; not blocking.
• The diff-coverage gate failed at 57.1% (FF-off branches of the new ThreadUtils sites aren't directly verified, and the FCM-receiver explicit prewarm shares behavior with the ThreadUtils prewarm it duplicates by design). The uncovered lines are low-risk one-liners.
• New test coverage is solid for the FF-on contract on each entry point and for the FCM receiver's filter behavior on token-update intents.

[fadi-george]

Thanks for chasing this. I think there’s one core issue to address before merge: the new ThreadUtils-level prewarm() calls are immediately followed by OneSignalDispatchers.launchOn*, which touches the same lazy dispatcher/scope chain that prewarm() is trying to initialize on the daemon thread.

Since prewarm() is fire-and-forget, the caller can still win the lazy-init race, or block behind the prewarm thread’s synchronized lazy initialization. In those cases the first cold suspendifyOnIO / suspendifyOnSerialIO caller can still pay the dispatcher construction cost on the main thread, so this may not reliably fix the cold-start ANR path.

Can we either:

• keep prewarm only at genuinely early call sites where there is meaningful lead time before first dispatch, or
• change the implementation so first dispatch cannot initialize the lazy scopes on the caller thread?

I’d also like to see coverage or trace evidence that the first cold helper call no longer initializes IOScope / DefaultScope / SerialIOScope on the caller thread.

Small follow-ups:

• Remove the SDK-4794 reference from source KDoc.
• Clarify the FCM test/comment, since IOMockHelper mocks suspendifyOnIO; the test verifies the explicit receiver prewarm call, not the real ThreadUtils prewarm path or end-to-end cold-init behavior.

[fadi-george]

A few things to fix before merge:

• Tests only verify prewarm() was called, not that it happened before the first dispatch. Please assert ordering.
• afterAny { unmockkAll() } can tear down IOMockHelper’s spec-scoped mocks after the first test. Clean up only test-owned mocks or move cleanup to spec teardown.
• prewarm() catches failures inside the spawned thread, but not Thread(...), priority assignment, or start() failures. Please wrap startup too.
• Minor: remove internal ticket IDs from source/test comments and names.

[fadi-george]

A few remaining things to fix/consider:

• OneSignalDispatchers.prewarm() now catches Throwable, but the new catches are unsuppressed and likely trip detekt’s TooGenericExceptionCaught. Please add the suppression or update the baseline.
• If the daemon starts but lazy construction fails inside the daemon body, prewarmStarted stays true, so later entry points can’t retry. Please reset the guard there too, or track started/completed separately.
• We now test ordering for receivers and SyncJobService, but not activities/HMS/ADM. At least activity prewarm() before suspendifyOnDefault coverage seems worth adding.
• The KDoc should mention that suspendifyOnIO/Default bypass OneSignalDispatchers while ThreadingMode.useBackgroundThreading is false, so prewarm is limited in that state.