fix(android): Decide session rotation on a monotonic clock (JAVA-573)

Whether foregrounding rotates the session was a comparison between two
System.currentTimeMillis() readings taken up to 30 seconds apart. The gap
between two wall-clock reads is not the time that passed — the device syncs
its clock and the interval comes out too long, too short, or negative — so a
step forward started a session that should have been resumed, and a step back
resumed one that should have ended. Android steps the wall clock most often in
the first seconds after boot, which is exactly where a cold start's background
window lives.

The background window is a duration, so it now lives on a Deadline over
options.getMonotonicTicker(), which on Android is CLOCK_BOOTTIME and so keeps
counting through deep sleep. One deadline serves both halves of the window: the
end-session task is scheduled for its remaining(), and a foreground arriving
first asks whether it hasPassed(). Measuring one window in two places is what
allowed two readings to disagree.

The staleness of a session already on the scope stays on the wall clock and is
now named for it. That path is only reached before the app has been backgrounded
in this process, and the only record of when that session started is
Session.getStarted() — a serialized epoch instant, which no tick can be
compared against. A TODO [MAJOR] marks the real fix: a session that records the
tick it started on.

Passing the clocks in from AppLifecycleIntegration, which already holds the
options being registered, removes the second constructor rather than adding a
third parameter to it.

Two tests were pinning the old behavior: `if last started session is before
interval` stubbed the clock with (2, 1) and passed only because the wall clock
ran backwards. They become a real background-foreground cycle on a
TestMonotonicTicker, with one regression test per step direction.

This leaves the other half of JAVA-573 open: a session that slept through its
window is still stamped when the SDK noticed rather than when the window fell
due, so its duration is inflated by the suspend. That value is serialized, and
#6091 is the template for fixing it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: runningcode

sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java

@@ -56,7 +56,9 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
                 scopes,
                 this.options.getSessionTrackingIntervalMillis(),
                 this.options.isEnableAutoSessionTracking(),
-                this.options.isEnableAppLifecycleBreadcrumbs());
+                this.options.isEnableAppLifecycleBreadcrumbs(),
+                this.options.getMonotonicTicker(),
+                this.options.getEpochClock());
 
         AppState.getInstance().addAppStateListener(watcher);
       }

sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java

@@ -5,53 +5,53 @@
 import io.sentry.ISentryLifecycleToken;
 import io.sentry.SentryLevel;
 import io.sentry.Session;
-import io.sentry.transport.CurrentDateProvider;
-import io.sentry.transport.ICurrentDateProvider;
+import io.sentry.time.Deadline;
+import io.sentry.time.EpochClock;
+import io.sentry.time.MonotonicTicker;
 import io.sentry.util.AutoClosableReentrantLock;
+import java.util.Date;
 import java.util.concurrent.Future;
-import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.TestOnly;
 
 final class LifecycleWatcher implements AppState.AppStateListener {
 
-  private final AtomicLong lastUpdatedSession = new AtomicLong(0L);
-
   private final long sessionIntervalMillis;
 
+  /**
+   * When the session the app left behind stops being resumable, or null while in the foreground.
+   *
+   * <p>One deadline decides both halves of the background window: when the scheduled task ends the
+   * session, and whether a foreground arriving first is soon enough to keep it. Measuring that one
+   * window in two places used to mean two clock readings, which a clock step could make disagree.
+   */
+  private @Nullable Deadline sessionEnd;
+
   private @Nullable Future<?> endSessionFuture;
   private final @NotNull AutoClosableReentrantLock endSessionLock = new AutoClosableReentrantLock();
   private final @NotNull IScopes scopes;
   private final boolean enableSessionTracking;
   private final boolean enableAppLifecycleBreadcrumbs;
 
-  private final @NotNull ICurrentDateProvider currentDateProvider;
-
-  LifecycleWatcher(
-      final @NotNull IScopes scopes,
-      final long sessionIntervalMillis,
-      final boolean enableSessionTracking,
-      final boolean enableAppLifecycleBreadcrumbs) {
-    this(
-        scopes,
-        sessionIntervalMillis,
-        enableSessionTracking,
-        enableAppLifecycleBreadcrumbs,
-        CurrentDateProvider.getInstance());
-  }
+  private final @NotNull MonotonicTicker ticker;
+  private final @NotNull EpochClock epochClock;
 
   LifecycleWatcher(
       final @NotNull IScopes scopes,
       final long sessionIntervalMillis,
       final boolean enableSessionTracking,
       final boolean enableAppLifecycleBreadcrumbs,
-      final @NotNull ICurrentDateProvider currentDateProvider) {
+      final @NotNull MonotonicTicker ticker,
+      final @NotNull EpochClock epochClock) {
     this.sessionIntervalMillis = sessionIntervalMillis;
     this.enableSessionTracking = enableSessionTracking;
     this.enableAppLifecycleBreadcrumbs = enableAppLifecycleBreadcrumbs;
     this.scopes = scopes;
-    this.currentDateProvider = currentDateProvider;
+    this.ticker = ticker;
+    this.epochClock = epochClock;
   }
 
   @Override
@@ -61,40 +61,50 @@ public void onForeground() {
   }
 
   private void startSession() {
-    cancelTask();
-
-    final long currentTimeMillis = currentDateProvider.getCurrentTimeMillis();
+    final @Nullable Deadline sessionEnd = takeSessionEnd();
 
-    scopes.configureScope(
-        scope -> {
-          if (lastUpdatedSession.get() == 0L) {
-            final @Nullable Session currentSession = scope.getSession();
-            if (currentSession != null && currentSession.getStarted() != null) {
-              lastUpdatedSession.set(currentSession.getStarted().getTime());
-            }
-          }
-        });
-
-    final long lastUpdatedSession = this.lastUpdatedSession.get();
     final boolean startNewSession =
-        lastUpdatedSession == 0L
-            || (lastUpdatedSession + sessionIntervalMillis) <= currentTimeMillis;
+        sessionEnd != null ? sessionEnd.hasPassed() : isSessionOnScopeStale();
     if (startNewSession) {
       if (enableSessionTracking) {
         scopes.startSession();
       }
     }
     scopes.getOptions().getReplayController().onAppForegrounded(startNewSession);
-    this.lastUpdatedSession.set(currentTimeMillis);
+  }
+
+  /**
+   * Whether the session already bound to the scope is old enough that foregrounding should rotate
+   * it.
+   *
+   * <p>Only reached before the app has been backgrounded in this process — a session started during
+   * SDK init, most often. There is no tick to compare against at that point: the only record of
+   * when that session started is {@link Session#getStarted()}, which is a wall-clock instant
+   * because it is serialized and sent, so this comparison has to be a wall-clock one and inherits
+   * the clock steps that come with it.
+   *
+   * <p>TODO [MAJOR]: have a session record the tick it started on, so this can be a {@link
+   * Deadline} like the background window is. That tick would have to stay out of the payload.
+   */
+  private boolean isSessionOnScopeStale() {
+    final long nowMillis = TimeUnit.NANOSECONDS.toMillis(epochClock.now().epochNanos());
+    // No session, or one that never recorded a start, leaves nothing to resume.
+    final @NotNull AtomicBoolean stale = new AtomicBoolean(true);
+    scopes.configureScope(
+        scope -> {
+          final @Nullable Session session = scope.getSession();
+          final @Nullable Date started = session == null ? null : session.getStarted();
+          if (started != null) {
+            stale.set(started.getTime() + sessionIntervalMillis <= nowMillis);
+          }
+        });
+    return stale.get();
   }
 
   // App went to background and triggered this callback after 700ms
   // as no new screen was shown
   @Override
   public void onBackground() {
-    final long currentTimeMillis = currentDateProvider.getCurrentTimeMillis();
-    this.lastUpdatedSession.set(currentTimeMillis);
-
     scopes.getOptions().getReplayController().onAppBackgrounded();
     scheduleEndSession();
 
@@ -104,6 +114,9 @@ public void onBackground() {
   private void scheduleEndSession() {
     try (final @NotNull ISentryLifecycleToken ignored = endSessionLock.acquire()) {
       cancelTask();
+      final @NotNull Deadline sessionEnd =
+          Deadline.after(ticker, sessionIntervalMillis, TimeUnit.MILLISECONDS);
+      this.sessionEnd = sessionEnd;
       final @NotNull Runnable endSession =
           () -> {
             if (enableSessionTracking) {
@@ -114,11 +127,13 @@ private void scheduleEndSession() {
           };
 
       try {
+        // The executor's own delay stops while the device is suspended, while the deadline keeps
+        // counting, so this task can only run at or after the deadline. It needs no second check.
         endSessionFuture =
             scopes
                 .getOptions()
                 .getTimerExecutorService()
-                .schedule(endSession, sessionIntervalMillis);
+                .schedule(endSession, sessionEnd.remaining(TimeUnit.MILLISECONDS));
       } catch (Throwable e) {
         scopes
             .getOptions()
@@ -131,6 +146,16 @@ private void scheduleEndSession() {
     }
   }
 
+  /** Stops the pending end of session and hands back the deadline it was going to run at. */
+  private @Nullable Deadline takeSessionEnd() {
+    try (final @NotNull ISentryLifecycleToken ignored = endSessionLock.acquire()) {
+      cancelTask();
+      final @Nullable Deadline sessionEnd = this.sessionEnd;
+      this.sessionEnd = null;
+      return sessionEnd;
+    }
+  }
+
   private void cancelTask() {
     try (final @NotNull ISentryLifecycleToken ignored = endSessionLock.acquire()) {
       if (endSessionFuture != null) {

sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt

@@ -12,7 +12,12 @@ import io.sentry.SentryLevel
 import io.sentry.SentryOptions
 import io.sentry.Session
 import io.sentry.Session.State
-import io.sentry.transport.ICurrentDateProvider
+import io.sentry.time.EpochClock
+import io.sentry.time.TestMonotonicTicker
+import io.sentry.time.Timestamp
+import java.util.concurrent.TimeUnit.HOURS
+import java.util.concurrent.TimeUnit.MILLISECONDS
+import java.util.concurrent.atomic.AtomicLong
 import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
@@ -32,7 +37,10 @@ import org.mockito.kotlin.whenever
 class LifecycleWatcherTest {
   private class Fixture {
     val scopes = mock<IScopes>()
-    val dateProvider = mock<ICurrentDateProvider>()
+    val ticker = TestMonotonicTicker()
+    // the wall clock, which only the staleness of a session already on the scope depends on
+    val nowMillis = AtomicLong(0L)
+    val epochClock = EpochClock { Timestamp.ofEpochNanos(MILLISECONDS.toNanos(nowMillis.get())) }
     // a real executor so scheduled end-session tasks actually run
     val options = SentryOptions().apply { setTimerExecutorService(SentryExecutorService(this)) }
     val replayController = mock<ReplayController>()
@@ -60,7 +68,8 @@ class LifecycleWatcherTest {
         sessionIntervalMillis,
         enableAutoSessionTracking,
         enableAppLifecycleBreadcrumbs,
-        dateProvider,
+        ticker,
+        epochClock,
       )
     }
   }
@@ -81,26 +90,67 @@ class LifecycleWatcherTest {
   }
 
   @Test
-  fun `if last started session is after interval, start new session`() {
-    val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false)
-    whenever(fixture.dateProvider.currentTimeMillis).thenReturn(1L, 2L)
+  fun `if the background window has elapsed, start new session`() {
+    val watcher =
+      fixture.getSUT(sessionIntervalMillis = 30000L, enableAppLifecycleBreadcrumbs = false)
     watcher.onForeground()
+    watcher.onBackground()
+    fixture.ticker.advance(30000, MILLISECONDS)
+
     watcher.onForeground()
+
     verify(fixture.scopes, times(2)).startSession()
     verify(fixture.replayController, times(2)).onAppForegrounded(true)
   }
 
   @Test
-  fun `if last started session is before interval, it should not start a new session`() {
-    val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false)
-    whenever(fixture.dateProvider.currentTimeMillis).thenReturn(2L, 1L)
+  fun `if the app returns within the background window, it should not start a new session`() {
+    val watcher =
+      fixture.getSUT(sessionIntervalMillis = 30000L, enableAppLifecycleBreadcrumbs = false)
     watcher.onForeground()
+    watcher.onBackground()
+    fixture.ticker.advance(29999, MILLISECONDS)
+
     watcher.onForeground()
+
     verify(fixture.scopes).startSession()
     verify(fixture.replayController).onAppForegrounded(true)
     verify(fixture.replayController).onAppForegrounded(false)
   }
 
+  @Test
+  fun `a wall clock stepping forward during the background window does not rotate the session`() {
+    val watcher =
+      fixture.getSUT(sessionIntervalMillis = 30000L, enableAppLifecycleBreadcrumbs = false)
+    watcher.onForeground()
+    watcher.onBackground()
+
+    // the device syncs its clock an hour forward, which two wall-clock reads used to report as an
+    // hour spent in the background
+    fixture.nowMillis.addAndGet(HOURS.toMillis(1))
+    fixture.ticker.advance(1, MILLISECONDS)
+    watcher.onForeground()
+
+    verify(fixture.scopes).startSession()
+    verify(fixture.replayController).onAppForegrounded(false)
+  }
+
+  @Test
+  fun `a wall clock stepping backwards during the background window still rotates the session`() {
+    val watcher =
+      fixture.getSUT(sessionIntervalMillis = 30000L, enableAppLifecycleBreadcrumbs = false)
+    fixture.nowMillis.set(HOURS.toMillis(1))
+    watcher.onForeground()
+    watcher.onBackground()
+
+    fixture.nowMillis.set(0L)
+    fixture.ticker.advance(30000, MILLISECONDS)
+    watcher.onForeground()
+
+    verify(fixture.scopes, times(2)).startSession()
+    verify(fixture.replayController, times(2)).onAppForegrounded(true)
+  }
+
   @Test
   fun `if app goes to background, end session after interval`() {
     val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false)
@@ -249,7 +299,6 @@ class LifecycleWatcherTest {
 
   @Test
   fun `background-foreground replay`() {
-    whenever(fixture.dateProvider.currentTimeMillis).thenReturn(1L)
     val watcher =
       fixture.getSUT(sessionIntervalMillis = 500L, enableAppLifecycleBreadcrumbs = false)
     watcher.onForeground()