[abdulraqeeb33]

Description

One line summary

Split OneSignalDispatchers.kt into focused files and gather its test hooks into one object. No behavior change except one latch fix and one allocation fix.

Details

Stacked on #2805. Each commit stands alone and can be reviewed on its own.

Why

• OneSignalDispatchers.kt is 602 lines on main and 378 at the head of feat: [SDK-5581] gate dispatcher bootstrap behind sdk_dispatcher_bootstrap #2805, with test hooks mixed in among the production code.
• Lane settings (sizes, queues, thread names, priorities, fallbacks) were defined twice, once per pool mode, and could drift.

What

1. 0354c92b3 Each lane is defined once in DispatcherLanes.kt, shared by both pool modes.
2. 7d58f125b The six test hooks move into DispatcherTestHooks.kt. They are still read on the production path, and are null outside tests.
3. c7332599e Pools are chosen under the pools lock, and a degraded choice is logged only after publishing. Before, that log could launch work that re-entered the latch and built a second set of pools. The test in DispatcherFailureLogTests drives this through the telemetry report path and fails (stack overflow) when the report moves back before publishing.
4. 5ccab9251 Comments trimmed to constraints.
5. f5c12ad6c GateDispatcher.dispatch returns one decision instead of three mutable flags. dc5e27ea7 builds the Run decision once per READY transition, so a ready dispatch allocates nothing.
6. 9d8fcc9c9 Diagnostics and pool choice move to DispatcherDiagnostics.kt and PoolChoice.kt.

The re-entry test uses unpublishPoolsForTest() instead of reflecting on poolsRef.

If a pool choice ever re-enters (the pools lock is reentrant), the pools the inner call published are kept and the outer choice is shut down, as on #2805. A test in DispatcherFailureLogTests re-enters from beforePoolChoice and fails without the guard.

Result
OneSignalDispatchers.kt is 206 lines: the public launch API, the latch, and the choice report. The threading package goes from 1,427 lines at the head of #2805 to 1,336.

Testing

• The 16 lane behavior tests run against both pool modes.
• Core (1,308) and notifications (246) suites pass at the head. Commits 1 to 5 each passed the full core suite.

[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

• BootstrapDispatcherPools.kt: 25/25 touched executable lines (100.0%) (44 touched lines in diff)
• DispatcherDiagnostics.kt: 44/45 touched executable lines (97.8%) (81 touched lines in diff)
• DispatcherLanes.kt: 31/32 touched executable lines (96.9%) (67 touched lines in diff)
• ⚠️ DispatcherTestHooks.kt: Not in coverage report (may not be compiled/tested)
• LazyDispatcherPools.kt: 18/18 touched executable lines (100.0%) (19 touched lines in diff)
• OneSignalDispatchers.kt: 20/20 touched executable lines (100.0%) (35 touched lines in diff)
• PoolChoice.kt: 23/24 touched executable lines (95.8%) (47 touched lines in diff)

Overall (aggregate gate)

161/164 touched executable lines covered (98.2% — requires ≥ 80%)

📥 View workflow run

[cursor]

Multi-model review (Opus 5.5, GPT 5.6 Sol, Grok 4.7)

Lane settings match the old dual copies. Cached GateDecision.Run is only set with READY and cleared on close. Post-publish logging is the right place for listener re-entry.

Act on

• publishFirstPools writes poolsRef only after choosePools returns, and poolsLock is reentrant. A same-thread launch during choice publishes generation B, then the outer .also { poolsRef = it.pools } replaces it with A and leaks B. The base publish checked existing != null and shutdown() the loser. The new listener test only launches from recordChoice, which already ran after publish on the base, so it does not pin this window.

Consider

• First-use flag read (StartupFlagsCache / SharedPreferences) now holds poolsLock. The old comment kept that I/O outside the lock so other launchOn* callers were not stuck behind it. Loser pools were never started, so widening the lock is not what prevents a second live set.

Noted

• recordChoice / reportChoiceToTelemetry still have a pre-existing check-then-set that can drop a degraded report (Opus + Sol).
• unpublishPoolsForTest does not reset prewarmStarted or pending telemetry (Grok).
• choosePools now snapshots bootstrapEnabledForTest before beforePoolChoice (Grok). No current test relies on the old order.

Dismissed

• run!! NPE on READY: READY is only set together with run.
• Per-dispatch allocation: the READY path already allocated nothing on the base.

  

_{Sent by Cursor Automation: PR Reviews}

[fadi-george]

LGTM, nothing blocking:

• Non-blocking: OneSignalDispatchers.kt:52, on the re-entry path the inner recordChoice logs while the outer lock is still held. The re-entry test sets bootstrapEnabledForTest so that path isn't covered.
• Nit: DispatcherFailureLogTests.kt:91, the throwing beforePoolChoice hook stays installed, so resetForTest runs one more failed choice before clearing hooks. Null it in finally, or clear hooks before choosing in resetForTest.
• Nit: OneSignalDispatchers.kt:45, currentPools() went from private to internal, which widens raw pool access across the module.