From 220c3a0bba1e30e22d1a804fee67d2fe93b25529 Mon Sep 17 00:00:00 2001 From: Fadi George Date: Tue, 27 Oct 2026 15:47:23 -0700 Subject: [PATCH 1/4] feat: [SDK-5396] send a heartbeat every 30 minutes of foreground time While a sessions API session is focused, enqueue a duration-only UpdateSessionOperation every 30 minutes of active time. The timer stops on unfocus and resumes on focus, so background time doesn't count. Flag off starts no timer. The last heartbeat's duration and time are persisted on the session, so an end after a kill while focused never reports less than the server already has. Co-Authored-By: Claude Opus 5.5 --- .../session/internal/session/SessionModel.kt | 19 ++ .../internal/session/impl/SessionService.kt | 76 ++++++- .../internal/session/SessionServiceTests.kt | 205 +++++++++++++++++- 3 files changed, 294 insertions(+), 6 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt index f76f40408..5c3d9a8f5 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt @@ -64,6 +64,25 @@ class SessionModel : Model() { setLongProperty(::activeDuration.name, value) } + /** + * The duration sent by this session's last heartbeat, in milliseconds. Persisted so an end + * after a kill while focused doesn't report less than the server already has. + */ + var lastHeartbeatDuration: Long + get() = getLongProperty(::lastHeartbeatDuration.name) { 0L } + set(value) { + setLongProperty(::lastHeartbeatDuration.name, value) + } + + /** + * When this session's last heartbeat was sent, in Unix time milliseconds. Null until the first heartbeat. + */ + var lastHeartbeatTime: Long? + get() = getOptLongProperty(::lastHeartbeatTime.name) + set(value) { + setOptLongProperty(::lastHeartbeatTime.name, value) + } + /** * [focusTime] from a monotonic clock ([com.onesignal.core.internal.time.ITime.elapsedRealtimeMillis]). * Used instead of [focusTime] to measure [activeDuration] when [usesSessionsApi]. diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt index 246b48148..48a7c3e4c 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt @@ -5,6 +5,7 @@ import com.onesignal.common.events.EventProducer import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler import com.onesignal.common.modeling.Model import com.onesignal.common.modeling.ModelChangedArgs +import com.onesignal.common.threading.OneSignalDispatchers import com.onesignal.common.threading.runOnSerialIO import com.onesignal.core.internal.application.IApplicationLifecycleHandler import com.onesignal.core.internal.application.IApplicationService @@ -29,6 +30,8 @@ import com.onesignal.user.internal.backend.IdentityConstants import com.onesignal.user.internal.identity.IdentityModelStore import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeoutOrNull import java.util.UUID @@ -44,6 +47,9 @@ import kotlin.reflect.KMutableProperty1 * some amount of time after losing focus. * * The time threshold for a session to expire is a configuration option: [ConfigModel.sessionFocusTimeout]. + * + * While focused, a session that [SessionModel.usesSessionsApi] reports its duration every + * [HEARTBEAT_INTERVAL_MS] of active time, so the server keeps it open and a crash loses little duration. */ internal class SessionService( private val applicationService: IApplicationService, @@ -58,6 +64,11 @@ internal class SessionService( // Tests replace it with a test dispatcher so the end send stays on the virtual clock. internal var ioDispatcher: CoroutineDispatcher = Dispatchers.IO + // Runs on the serial IO lane so it doesn't race the focus handlers. Not a constructor parameter + // for the same reason as ioDispatcher; tests replace it to run the heartbeat on a virtual clock. + internal var launchHeartbeat: (suspend () -> Unit) -> Job = OneSignalDispatchers::launchOnSerialIO + private val heartbeat = Heartbeat() + override val startTime: Long // Pre-bootstrap default returns "now" so call sites computing `_time.currentTimeMillis - startTime` // (e.g. IAM session-duration / SESSION_TIME triggers) see ~0ms elapsed instead of ~58 years @@ -169,6 +180,8 @@ internal class SessionService( val end = if (session.usesSessionsApi) endSessionOperation(session, config?.appId) else null sessionLifeCycleNotifier.fire { it.onSessionEnded(activeDuration) } session.activeDuration = 0L + session.lastHeartbeatDuration = 0L + session.lastHeartbeatTime = null return end } @@ -238,6 +251,37 @@ internal class SessionService( session.focusElapsedRealtime = focusElapsedMs sessionLifeCycleNotifier.fire { it.onSessionActive() } } + if (session.usesSessionsApi) heartbeat.start(session) else heartbeat.stop() + } + + /** Sends the session's cumulative duration every [HEARTBEAT_INTERVAL_MS] of active time while focused. */ + private inner class Heartbeat { + private var job: Job? = null + + fun start(session: SessionModel) { + stop() + job = + launchHeartbeat { + while (true) { + val duration = session.activeDurationAt(time.elapsedRealtimeMillis) + val remaining = session.lastHeartbeatDuration + HEARTBEAT_INTERVAL_MS - duration + if (remaining > 0) { + // Rechecked after waking since delay and elapsedRealtime can disagree. + delay(remaining) + continue + } + Logging.debug("SessionService: heartbeat with activeDuration: $duration") + session.lastHeartbeatDuration = duration + session.lastHeartbeatTime = time.currentTimeMillis + updateSessionOperation(session, config?.appId, duration)?.let { operationRepo.enqueue(it) } + } + } + } + + fun stop() { + job?.cancel() + job = null + } } override fun onUnfocused() { @@ -258,6 +302,7 @@ internal class SessionService( Logging.warn("SessionService.onUnfocused called before bootstrap; ignoring.") return } + heartbeat.stop() // capture the amount of time the app was focused val dt = if (session.usesSessionsApi) { @@ -285,6 +330,9 @@ internal class SessionService( companion object { // Covers the post-create delay when the create is still queued, plus the request itself. private const val END_SESSION_SEND_TIMEOUT_MS = 30_000L + + // Hardcoded until the interval is remotely configured. + internal const val HEARTBEAT_INTERVAL_MS = 30 * 60 * 1000L } } @@ -304,19 +352,39 @@ private fun createSessionOperation( ) } +/** [SessionModel.activeDuration] including the current focus, which hasn't been added yet. */ +private fun SessionModel.activeDurationAt(elapsedRealtimeMs: Long): Long = + activeDuration + (elapsedRealtimeMs - focusElapsedRealtime).coerceAtLeast(0L) + +/** + * An app killed while focused never adds its last focus to [SessionModel.activeDuration] or sets + * [SessionModel.unfocusTime], so fall back to the last focus or heartbeat, whichever is later. + */ private fun endSessionOperation( session: SessionModel, appId: String?, ): UpdateSessionOperation? = - withSessionIds(session, appId, "ending") { resolvedAppId, onesignalId, subscriptionId -> + updateSessionOperation( + session, + appId, + activeDuration = maxOf(session.activeDuration, session.lastHeartbeatDuration), + endTime = maxOf(session.unfocusTime ?: 0L, session.focusTime, session.lastHeartbeatTime ?: 0L), + ) + +private fun updateSessionOperation( + session: SessionModel, + appId: String?, + activeDuration: Long, + endTime: Long? = null, +): UpdateSessionOperation? = + withSessionIds(session, appId, if (endTime != null) "ending" else "updating") { resolvedAppId, onesignalId, subscriptionId -> UpdateSessionOperation( appId = resolvedAppId, sessionId = session.localSessionId, onesignalId = onesignalId, subscriptionId = subscriptionId, - activeDuration = session.activeDuration, - // An app killed while focused has no unfocus after its last focus. - endTime = maxOf(session.unfocusTime ?: 0L, session.focusTime), + activeDuration = activeDuration, + endTime = endTime, ) } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt index b19465312..37c5f6931 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt @@ -41,7 +41,11 @@ import io.mockk.verify import io.mockk.verifyOrder import kotlinx.coroutines.Job import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest // Mocks used by every test in this file @@ -53,10 +57,13 @@ private class Mocks( private val mockSessionModelStore = MockHelper.sessionModelStore() + // Runs the heartbeat in virtual time; elapsedRealtime advances with it. + val heartbeatScope = TestScope() + val time: ITime = mockk().also { every { it.currentTimeMillis } answers { currentTime } - every { it.elapsedRealtimeMillis } answers { elapsedRealtime } + every { it.elapsedRealtimeMillis } answers { elapsedRealtime + heartbeatScope.testScheduler.currentTime } } val featureManager: IFeatureManager = @@ -117,7 +124,16 @@ private class Mocks( featureManager, identityModelStore, operationRepo, - ) + ).also { it.launchHeartbeat = ::launchInHeartbeatScope } + + fun launchInHeartbeatScope(block: suspend () -> Unit): Job = heartbeatScope.launch { block() } + + val heartbeats get() = enqueued.filterIsInstance().filter { !it.isEnd } + + fun advanceTime(ms: Long) { + heartbeatScope.advanceTimeBy(ms) + heartbeatScope.runCurrent() + } val spyCallback = spyk() @@ -127,6 +143,9 @@ private class Mocks( } } +private const val MINUTE_MS = 60_000L +private const val HEARTBEAT_MS = 30 * MINUTE_MS + class SessionServiceTests : FunSpec({ // SessionService.onFocus/onUnfocused dispatch their state mutation through the now-always-async // runOnSerialIO. IOMockHelper.beforeSpec stubs runOnSerialIO to run inline so the tests below @@ -828,6 +847,184 @@ class SessionServiceTests : FunSpec({ sessionModelStore.model.activeDuration shouldBe 200L } + test("heartbeat sends the cumulative duration after 30 minutes in the foreground") { + // Given + val mocks = Mocks(sessionsApiEnabled = true) + val sessionService = mocks.sessionService + sessionService.bootstrap() + sessionService.start() + val sessionModelStore = mocks.sessionModelStore { it.isValid = false } + sessionService.onFocus(false) + + // When + mocks.advanceTime(HEARTBEAT_MS - 1) + + // Then + mocks.heartbeats shouldBe emptyList() + + // When + mocks.advanceTime(1) + + // Then + val heartbeat = mocks.heartbeats.single() + heartbeat.sessionId shouldBe sessionModelStore.model.localSessionId + heartbeat.onesignalId shouldBe Mocks.ONESIGNAL_ID + heartbeat.subscriptionId shouldBe Mocks.SUBSCRIPTION_ID + heartbeat.activeDuration shouldBe HEARTBEAT_MS + heartbeat.endTime shouldBe null + } + + test("heartbeat sends again every 30 minutes in the foreground") { + // Given + val mocks = Mocks(sessionsApiEnabled = true) + val sessionService = mocks.sessionService + sessionService.bootstrap() + sessionService.start() + mocks.sessionModelStore { it.isValid = false } + sessionService.onFocus(false) + + // When + mocks.advanceTime(2 * HEARTBEAT_MS) + + // Then + mocks.heartbeats.map { it.activeDuration } shouldBe listOf(HEARTBEAT_MS, 2 * HEARTBEAT_MS) + } + + test("heartbeat pauses in the background and counts only foreground time") { + // Given + val mocks = Mocks(sessionsApiEnabled = true) + val sessionService = mocks.sessionService + sessionService.bootstrap() + sessionService.start() + mocks.sessionModelStore { it.isValid = false } + sessionService.onFocus(false) + mocks.advanceTime(20 * MINUTE_MS) + sessionService.onUnfocused() + + // When + mocks.advanceTime(2 * HEARTBEAT_MS) + + // Then + mocks.heartbeats shouldBe emptyList() + + // When + sessionService.onFocus(false) + mocks.advanceTime(10 * MINUTE_MS) + + // Then + mocks.heartbeats.single().activeDuration shouldBe HEARTBEAT_MS + } + + test("new session restarts the heartbeat interval") { + // Given + val mocks = Mocks(sessionsApiEnabled = true) + val sessionService = mocks.sessionService + sessionService.bootstrap() + sessionService.start() + mocks.sessionModelStore { it.isValid = false } + sessionService.onFocus(false) + mocks.advanceTime(HEARTBEAT_MS) + sessionService.onUnfocused() + sessionService.backgroundRun() + mocks.enqueued.clear() + + // When + sessionService.onFocus(false) + mocks.advanceTime(HEARTBEAT_MS) + + // Then + mocks.heartbeats.single().activeDuration shouldBe HEARTBEAT_MS + } + + test("cold start after a kill following a heartbeat ends with at least the heartbeat's duration and time") { + // Given + val mocks = Mocks(sessionsApiEnabled = true) + val sessionService = mocks.sessionService + sessionService.bootstrap() + sessionService.start() + mocks.sessionModelStore { it.isValid = false } + sessionService.onFocus(false) + mocks.currentTime += HEARTBEAT_MS + mocks.advanceTime(HEARTBEAT_MS) + val heartbeatTime = mocks.currentTime + + // When + val restarted = + SessionService( + MockHelper.applicationService(), + mocks.configModelStore, + mocks.sessionModelStore(), + mocks.time, + mocks.featureManager, + mocks.identityModelStore, + mocks.operationRepo, + ).also { it.launchHeartbeat = { _ -> mockk(relaxed = true) } } + mocks.enqueued.clear() + restarted.bootstrap() + restarted.start() + restarted.onFocus(false) + + // Then + val end = mocks.enqueued.filterIsInstance().single() + end.isEnd shouldBe true + end.activeDuration shouldBe HEARTBEAT_MS + end.endTime shouldBe heartbeatTime + } + + test("session end clears the last heartbeat") { + // Given + val mocks = Mocks(sessionsApiEnabled = true) + val sessionModelStore = + mocks.sessionModelStore { + it.isValid = true + it.lastHeartbeatDuration = HEARTBEAT_MS + it.lastHeartbeatTime = 999L + } + val sessionService = mocks.sessionService + sessionService.bootstrap() + sessionService.start() + + // When + sessionService.backgroundRun() + + // Then + sessionModelStore.model.lastHeartbeatDuration shouldBe 0L + sessionModelStore.model.lastHeartbeatTime shouldBe null + } + + test("heartbeat does not run when the session uses the legacy path") { + // Given + val mocks = Mocks(sessionsApiEnabled = false) + val sessionService = mocks.sessionService + sessionService.bootstrap() + sessionService.start() + mocks.sessionModelStore { it.isValid = false } + + // When + sessionService.onFocus(false) + mocks.advanceTime(2 * HEARTBEAT_MS) + + // Then + mocks.enqueued shouldBe emptyList() + } + + test("heartbeat without a push subscription ID sends nothing and waits for the next interval") { + // Given + val mocks = Mocks(sessionsApiEnabled = true) + mocks.configModelStore.model.pushSubscriptionId = null + val sessionService = mocks.sessionService + sessionService.bootstrap() + sessionService.start() + mocks.sessionModelStore { it.isValid = false } + sessionService.onFocus(false) + + // When + mocks.advanceTime(2 * HEARTBEAT_MS) + + // Then + mocks.enqueued shouldBe emptyList() + } + test("session model fields survive a reload from preferences") { // Given val prefs = MockPreferencesService() @@ -840,6 +1037,8 @@ class SessionServiceTests : FunSpec({ store.model.usesSessionsApi = true store.model.onesignalId = Mocks.ONESIGNAL_ID store.model.subscriptionId = Mocks.SUBSCRIPTION_ID + store.model.lastHeartbeatDuration = 654L + store.model.lastHeartbeatTime = 987L // When val reloaded = SessionModelStore(prefs).model @@ -853,6 +1052,8 @@ class SessionServiceTests : FunSpec({ reloaded.usesSessionsApi shouldBe true reloaded.onesignalId shouldBe Mocks.ONESIGNAL_ID reloaded.subscriptionId shouldBe Mocks.SUBSCRIPTION_ID + reloaded.lastHeartbeatDuration shouldBe 654L + reloaded.lastHeartbeatTime shouldBe 987L } test("onFocus dispatches the session-mutation body through runOnSerialIO (SDK-4508)") { From 123824ece041a90334c18b81bf0577d000e115d4 Mon Sep 17 00:00:00 2001 From: Fadi George Date: Wed, 28 Oct 2026 10:46:43 -0700 Subject: [PATCH 2/4] fix: [SDK-5396] end at the unfocus, not an earlier heartbeat's time A wall-clock jump saved on a heartbeat could move the session end past a later unfocus. Clear the heartbeat time on unfocus so it only counts for a session killed in the foreground. Co-Authored-By: Claude Opus 5.5 --- .../session/internal/session/SessionModel.kt | 4 +++- .../internal/session/impl/SessionService.kt | 1 + .../internal/session/SessionServiceTests.kt | 23 +++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt index 5c3d9a8f5..21ec1092a 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt @@ -75,7 +75,9 @@ class SessionModel : Model() { } /** - * When this session's last heartbeat was sent, in Unix time milliseconds. Null until the first heartbeat. + * When this session's last heartbeat was sent, in Unix time milliseconds. Null until the first + * heartbeat, and cleared on unfocus, which then marks the end, so a wall-clock jump on the + * heartbeat can't move the end past it. */ var lastHeartbeatTime: Long? get() = getOptLongProperty(::lastHeartbeatTime.name) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt index 48a7c3e4c..d83aa813e 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt @@ -313,6 +313,7 @@ internal class SessionService( } session.activeDuration += dt session.unfocusTime = unfocusTimeMs + session.lastHeartbeatTime = null Logging.log(LogLevel.DEBUG, "SessionService.onUnfocused adding time $dt for total: ${session.activeDuration}") } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt index 37c5f6931..3fe54a0d0 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt @@ -971,6 +971,29 @@ class SessionServiceTests : FunSpec({ end.endTime shouldBe heartbeatTime } + test("session that unfocused after a heartbeat ends at its unfocus even if the heartbeat's clock ran ahead") { + // Given + val mocks = Mocks(sessionsApiEnabled = true) + val sessionService = mocks.sessionService + sessionService.bootstrap() + sessionService.start() + mocks.sessionModelStore { it.isValid = false } + sessionService.onFocus(false) + mocks.currentTime += 2 * HEARTBEAT_MS + mocks.advanceTime(HEARTBEAT_MS) + mocks.currentTime -= HEARTBEAT_MS + val unfocusTime = mocks.currentTime + sessionService.onUnfocused() + + // When + sessionService.backgroundRun() + + // Then + val end = mocks.enqueued.filterIsInstance().single { it.isEnd } + end.endTime shouldBe unfocusTime + end.activeDuration shouldBe HEARTBEAT_MS + } + test("session end clears the last heartbeat") { // Given val mocks = Mocks(sessionsApiEnabled = true) From 93908532dd070711a2a0ab9ae30cfa8c029c2700 Mon Sep 17 00:00:00 2001 From: Fadi George Date: Wed, 28 Oct 2026 10:54:01 -0700 Subject: [PATCH 3/4] fix: [SDK-5396] address review: guard the heartbeat, don't delay an end by a heartbeat's backoff - Catch failures in the heartbeat loop, since the serial IO scope has no exception handler; the heartbeat stops until the next focus. - An end that replaces a backed-off heartbeat keeps its retry count but not its backoff, so it can still send while the background job waits. - Share the focus-duration math between the heartbeat and unfocus, and clarify that lastHeartbeatDuration is set even when nothing was sent. Co-Authored-By: Claude Opus 5.5 --- .../operations/impl/SessionQueuePolicy.kt | 3 +- .../session/internal/session/SessionModel.kt | 5 +- .../internal/session/impl/SessionService.kt | 48 ++++++++++++------- .../operations/SessionOperationQueueTests.kt | 14 ++++++ .../internal/session/SessionServiceTests.kt | 32 ++++++++++++- 5 files changed, 81 insertions(+), 21 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/SessionQueuePolicy.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/SessionQueuePolicy.kt index 8205f23b3..199f23a1a 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/SessionQueuePolicy.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/SessionQueuePolicy.kt @@ -92,7 +92,8 @@ internal class SessionQueuePolicy( val replaced = it.operation as UpdateSessionOperation op.activeDuration = maxOf(op.activeDuration, replaced.activeDuration) incoming.retries = maxOf(incoming.retries, it.retries) - incoming.retryAt = maxOf(incoming.retryAt, it.retryAt) + // An end is sent while the background job waits for it, so it doesn't wait out a heartbeat's backoff. + if (!op.isEnd) incoming.retryAt = maxOf(incoming.retryAt, it.retryAt) val waiters = synchronized(it) { it.waiters.toList().also { _ -> it.waiters.clear() } } incoming.attachOrWake(waiters) } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt index 21ec1092a..47be7b4c7 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt @@ -65,8 +65,9 @@ class SessionModel : Model() { } /** - * The duration sent by this session's last heartbeat, in milliseconds. Persisted so an end - * after a kill while focused doesn't report less than the server already has. + * The duration at this session's last heartbeat, in milliseconds, set even when the heartbeat + * couldn't be built. Persisted so an end after a kill while focused doesn't report less than + * the server already has. */ var lastHeartbeatDuration: Long get() = getLongProperty(::lastHeartbeatDuration.name) { 0L } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt index d83aa813e..ffae943ea 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt @@ -28,6 +28,7 @@ import com.onesignal.session.internal.session.operations.CreateSessionOperation import com.onesignal.session.internal.session.operations.UpdateSessionOperation import com.onesignal.user.internal.backend.IdentityConstants import com.onesignal.user.internal.identity.IdentityModelStore +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -258,26 +259,39 @@ internal class SessionService( private inner class Heartbeat { private var job: Job? = null + @Suppress("TooGenericExceptionCaught") fun start(session: SessionModel) { stop() job = launchHeartbeat { - while (true) { - val duration = session.activeDurationAt(time.elapsedRealtimeMillis) - val remaining = session.lastHeartbeatDuration + HEARTBEAT_INTERVAL_MS - duration - if (remaining > 0) { - // Rechecked after waking since delay and elapsedRealtime can disagree. - delay(remaining) - continue - } - Logging.debug("SessionService: heartbeat with activeDuration: $duration") - session.lastHeartbeatDuration = duration - session.lastHeartbeatTime = time.currentTimeMillis - updateSessionOperation(session, config?.appId, duration)?.let { operationRepo.enqueue(it) } + // The serial IO scope has no exception handler, so an escaping failure would crash + // the app. Stop until the next focus instead, which also can't spin on a failed tick. + try { + tickForever(session) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Logging.error("SessionService: heartbeat stopped", e) } } } + private suspend fun tickForever(session: SessionModel) { + while (true) { + val duration = session.activeDuration + session.focusDurationAt(time.elapsedRealtimeMillis) + val remaining = session.lastHeartbeatDuration + HEARTBEAT_INTERVAL_MS - duration + if (remaining > 0) { + // Rechecked after waking since delay and elapsedRealtime can disagree. + delay(remaining) + continue + } + Logging.debug("SessionService: heartbeat with activeDuration: $duration") + session.lastHeartbeatDuration = duration + session.lastHeartbeatTime = time.currentTimeMillis + updateSessionOperation(session, config?.appId, duration)?.let { operationRepo.enqueue(it) } + } + } + fun stop() { job?.cancel() job = null @@ -306,8 +320,7 @@ internal class SessionService( // capture the amount of time the app was focused val dt = if (session.usesSessionsApi) { - // elapsedRealtime resets on reboot; drop the interval rather than count a negative one. - (unfocusElapsedMs - session.focusElapsedRealtime).coerceAtLeast(0L) + session.focusDurationAt(unfocusElapsedMs) } else { unfocusTimeMs - session.focusTime } @@ -353,9 +366,10 @@ private fun createSessionOperation( ) } -/** [SessionModel.activeDuration] including the current focus, which hasn't been added yet. */ -private fun SessionModel.activeDurationAt(elapsedRealtimeMs: Long): Long = - activeDuration + (elapsedRealtimeMs - focusElapsedRealtime).coerceAtLeast(0L) +/** Time in the current focus, which [SessionModel.activeDuration] doesn't include until unfocus. */ +private fun SessionModel.focusDurationAt(elapsedRealtimeMs: Long): Long = + // elapsedRealtime resets on reboot; drop the interval rather than count a negative one. + (elapsedRealtimeMs - focusElapsedRealtime).coerceAtLeast(0L) /** * An app killed while focused never adds its last focus to [SessionModel.activeDuration] or sets diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/SessionOperationQueueTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/SessionOperationQueueTests.kt index 9b662ad3a..7ed654cae 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/SessionOperationQueueTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/SessionOperationQueueTests.kt @@ -181,6 +181,20 @@ class SessionOperationQueueTests : FunSpec({ incoming.retryAt shouldBe 60_000L } + test("an end that replaces a backed-off update keeps its retries but not its backoff") { + val policy = SessionQueuePolicy() + val backedOff = item(update()).apply { + retries = 3 + retryAt = 60_000L + } + val end = item(update(activeDuration = 2_000L, endTime = 9L)) + + policy.onEnqueue(mutableListOf(backedOff), end) + + end.retries shouldBe 3 + end.retryAt shouldBe 0L + } + test("an update after an end that is executing is dropped") { val policy = SessionQueuePolicy() val queue = mutableListOf() diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt index 3fe54a0d0..9f1ff99a7 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt @@ -126,7 +126,9 @@ private class Mocks( operationRepo, ).also { it.launchHeartbeat = ::launchInHeartbeatScope } - fun launchInHeartbeatScope(block: suspend () -> Unit): Job = heartbeatScope.launch { block() } + var heartbeatJob: Job? = null + + fun launchInHeartbeatScope(block: suspend () -> Unit): Job = heartbeatScope.launch { block() }.also { heartbeatJob = it } val heartbeats get() = enqueued.filterIsInstance().filter { !it.isEnd } @@ -1015,6 +1017,34 @@ class SessionServiceTests : FunSpec({ sessionModelStore.model.lastHeartbeatTime shouldBe null } + test("heartbeat that fails stops without an uncaught exception and restarts on the next focus") { + // Given + val mocks = Mocks(sessionsApiEnabled = true) + every { mocks.operationRepo.enqueue(ofType(), any()) } throws RuntimeException("enqueue failed") + val sessionService = mocks.sessionService + sessionService.bootstrap() + sessionService.start() + mocks.sessionModelStore { it.isValid = false } + sessionService.onFocus(false) + + // When + mocks.advanceTime(2 * HEARTBEAT_MS) + + // Then + verify(exactly = 1) { mocks.operationRepo.enqueue(ofType(), any()) } + val failed = mocks.heartbeatJob!! + failed.isCompleted shouldBe true + failed.isCancelled shouldBe false + + // When + sessionService.onUnfocused() + sessionService.onFocus(false) + mocks.advanceTime(HEARTBEAT_MS) + + // Then + verify(exactly = 2) { mocks.operationRepo.enqueue(ofType(), any()) } + } + test("heartbeat does not run when the session uses the legacy path") { // Given val mocks = Mocks(sessionsApiEnabled = false) From 7cdd96dd89a7f4f5b7814db719f616c766a02040 Mon Sep 17 00:00:00 2001 From: Fadi George Date: Wed, 28 Oct 2026 16:25:34 -0700 Subject: [PATCH 4/4] fix: [SDK-5396] time a heartbeat from its focus on the monotonic clock A wall-clock jump mid-focus could otherwise move a killed session's end past its foreground time. Co-Authored-By: Claude Opus 5.5 --- .../session/internal/session/SessionModel.kt | 6 ++-- .../internal/session/impl/SessionService.kt | 6 ++-- .../internal/session/SessionServiceTests.kt | 34 +++++++++++++++++++ 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt index 47be7b4c7..8768ed3ab 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt @@ -76,9 +76,9 @@ class SessionModel : Model() { } /** - * When this session's last heartbeat was sent, in Unix time milliseconds. Null until the first - * heartbeat, and cleared on unfocus, which then marks the end, so a wall-clock jump on the - * heartbeat can't move the end past it. + * When this session's last heartbeat was sent, in Unix time milliseconds, measured from [focusTime] + * on the monotonic clock so a wall-clock jump mid-focus can't move a killed session's end. Null + * until the first heartbeat, and cleared on unfocus, which then marks the end. */ var lastHeartbeatTime: Long? get() = getOptLongProperty(::lastHeartbeatTime.name) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt index ffae943ea..ea002d6d4 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt @@ -278,7 +278,8 @@ internal class SessionService( private suspend fun tickForever(session: SessionModel) { while (true) { - val duration = session.activeDuration + session.focusDurationAt(time.elapsedRealtimeMillis) + val focusDuration = session.focusDurationAt(time.elapsedRealtimeMillis) + val duration = session.activeDuration + focusDuration val remaining = session.lastHeartbeatDuration + HEARTBEAT_INTERVAL_MS - duration if (remaining > 0) { // Rechecked after waking since delay and elapsedRealtime can disagree. @@ -287,7 +288,8 @@ internal class SessionService( } Logging.debug("SessionService: heartbeat with activeDuration: $duration") session.lastHeartbeatDuration = duration - session.lastHeartbeatTime = time.currentTimeMillis + // Measured from the focus, so a wall-clock jump mid-focus can't move a killed session's end. + session.lastHeartbeatTime = session.focusTime + focusDuration updateSessionOperation(session, config?.appId, duration)?.let { operationRepo.enqueue(it) } } } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt index 9f1ff99a7..79d803d7c 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/SessionServiceTests.kt @@ -973,6 +973,40 @@ class SessionServiceTests : FunSpec({ end.endTime shouldBe heartbeatTime } + test("cold start after a kill following a clock-jumped heartbeat ends at the heartbeat's foreground time") { + // Given + val mocks = Mocks(sessionsApiEnabled = true) + val sessionService = mocks.sessionService + sessionService.bootstrap() + sessionService.start() + mocks.sessionModelStore { it.isValid = false } + sessionService.onFocus(false) + val focusTime = mocks.currentTime + mocks.currentTime += 2 * HEARTBEAT_MS + mocks.advanceTime(HEARTBEAT_MS) + + // When + val restarted = + SessionService( + MockHelper.applicationService(), + mocks.configModelStore, + mocks.sessionModelStore(), + mocks.time, + mocks.featureManager, + mocks.identityModelStore, + mocks.operationRepo, + ).also { it.launchHeartbeat = { _ -> mockk(relaxed = true) } } + mocks.enqueued.clear() + restarted.bootstrap() + restarted.start() + restarted.onFocus(false) + + // Then + val end = mocks.enqueued.filterIsInstance().single() + end.activeDuration shouldBe HEARTBEAT_MS + end.endTime shouldBe focusTime + HEARTBEAT_MS + } + test("session that unfocused after a heartbeat ends at its unfocus even if the heartbeat's clock ran ahead") { // Given val mocks = Mocks(sessionsApiEnabled = true)