Skip to content

Commit ae59540

Browse files
runningcodeclaude
andauthored
fix(android): Decide session rotation on a monotonic clock (JAVA-573) (#6096)
* 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> * changelog * docs(android): Say which lock guards sessionEnd (JAVA-573) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(android): Simplify the wall-clock note on isSessionOnScopeStale (JAVA-573) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(changelog): List the session rotation change as breaking (JAVA-573) It changes when a new session starts, so session counts shift. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 48e97fb commit ae59540

4 files changed

Lines changed: 127 additions & 53 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
- Manually constructed `SentryTimberIntegration` and `SentryTimberTree` instances require a
3737
`logsEnabled` argument. Pass `false` to keep Sentry Logs disabled.
3838
- Remove the Apache HttpClient transport and stop publishing `sentry-apache-http-client-5` ([#6214](https://github.com/getsentry/sentry-java/pull/6214))
39+
- Decide whether foregrounding the app starts a new session on a monotonic clock instead of the wall clock, so that a device time change no longer starts a session that should have been resumed, or resumes one that should have ended. This affects session counts ([#6096](https://github.com/getsentry/sentry-java/pull/6096))
3940

4041
### Features
4142

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
5656
scopes,
5757
this.options.getSessionTrackingIntervalMillis(),
5858
this.options.isEnableAutoSessionTracking(),
59-
this.options.isEnableAppLifecycleBreadcrumbs());
59+
this.options.isEnableAppLifecycleBreadcrumbs(),
60+
this.options.getMonotonicTicker(),
61+
this.options.getEpochClock());
6062

6163
AppState.getInstance().addAppStateListener(watcher);
6264
}

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

Lines changed: 64 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,53 +5,52 @@
55
import io.sentry.ISentryLifecycleToken;
66
import io.sentry.SentryLevel;
77
import io.sentry.Session;
8-
import io.sentry.transport.CurrentDateProvider;
9-
import io.sentry.transport.ICurrentDateProvider;
8+
import io.sentry.time.Deadline;
9+
import io.sentry.time.EpochClock;
10+
import io.sentry.time.MonotonicTicker;
1011
import io.sentry.util.AutoClosableReentrantLock;
12+
import java.util.Date;
1113
import java.util.concurrent.Future;
12-
import java.util.concurrent.atomic.AtomicLong;
14+
import java.util.concurrent.TimeUnit;
15+
import java.util.concurrent.atomic.AtomicBoolean;
1316
import org.jetbrains.annotations.NotNull;
1417
import org.jetbrains.annotations.Nullable;
1518
import org.jetbrains.annotations.TestOnly;
1619

1720
final class LifecycleWatcher implements AppState.AppStateListener {
1821

19-
private final AtomicLong lastUpdatedSession = new AtomicLong(0L);
20-
2122
private final long sessionIntervalMillis;
2223

24+
/**
25+
* When the session the app left behind stops being resumable, or null while in the foreground.
26+
*
27+
* <p>Only read or written while holding {@link #endSessionLock}, which is also what lets
28+
* cancelling the pending task and taking this deadline happen as one step.
29+
*/
30+
private @Nullable Deadline sessionEnd;
31+
2332
private @Nullable Future<?> endSessionFuture;
2433
private final @NotNull AutoClosableReentrantLock endSessionLock = new AutoClosableReentrantLock();
2534
private final @NotNull IScopes scopes;
2635
private final boolean enableSessionTracking;
2736
private final boolean enableAppLifecycleBreadcrumbs;
2837

29-
private final @NotNull ICurrentDateProvider currentDateProvider;
30-
31-
LifecycleWatcher(
32-
final @NotNull IScopes scopes,
33-
final long sessionIntervalMillis,
34-
final boolean enableSessionTracking,
35-
final boolean enableAppLifecycleBreadcrumbs) {
36-
this(
37-
scopes,
38-
sessionIntervalMillis,
39-
enableSessionTracking,
40-
enableAppLifecycleBreadcrumbs,
41-
CurrentDateProvider.getInstance());
42-
}
38+
private final @NotNull MonotonicTicker ticker;
39+
private final @NotNull EpochClock epochClock;
4340

4441
LifecycleWatcher(
4542
final @NotNull IScopes scopes,
4643
final long sessionIntervalMillis,
4744
final boolean enableSessionTracking,
4845
final boolean enableAppLifecycleBreadcrumbs,
49-
final @NotNull ICurrentDateProvider currentDateProvider) {
46+
final @NotNull MonotonicTicker ticker,
47+
final @NotNull EpochClock epochClock) {
5048
this.sessionIntervalMillis = sessionIntervalMillis;
5149
this.enableSessionTracking = enableSessionTracking;
5250
this.enableAppLifecycleBreadcrumbs = enableAppLifecycleBreadcrumbs;
5351
this.scopes = scopes;
54-
this.currentDateProvider = currentDateProvider;
52+
this.ticker = ticker;
53+
this.epochClock = epochClock;
5554
}
5655

5756
@Override
@@ -61,40 +60,48 @@ public void onForeground() {
6160
}
6261

6362
private void startSession() {
64-
cancelTask();
65-
66-
final long currentTimeMillis = currentDateProvider.getCurrentTimeMillis();
63+
final @Nullable Deadline sessionEnd = takeSessionEnd();
6764

68-
scopes.configureScope(
69-
scope -> {
70-
if (lastUpdatedSession.get() == 0L) {
71-
final @Nullable Session currentSession = scope.getSession();
72-
if (currentSession != null && currentSession.getStarted() != null) {
73-
lastUpdatedSession.set(currentSession.getStarted().getTime());
74-
}
75-
}
76-
});
77-
78-
final long lastUpdatedSession = this.lastUpdatedSession.get();
7965
final boolean startNewSession =
80-
lastUpdatedSession == 0L
81-
|| (lastUpdatedSession + sessionIntervalMillis) <= currentTimeMillis;
66+
sessionEnd != null ? sessionEnd.hasPassed() : isSessionOnScopeStale();
8267
if (startNewSession) {
8368
if (enableSessionTracking) {
8469
scopes.startSession();
8570
}
8671
}
8772
scopes.getOptions().getReplayController().onAppForegrounded(startNewSession);
88-
this.lastUpdatedSession.set(currentTimeMillis);
73+
}
74+
75+
/**
76+
* Whether the session on the scope is too old to resume, so foregrounding should start a new one.
77+
*
78+
* <p>Used when no background window is pending, which means the session was started by SDK init
79+
* rather than by leaving and returning to the app. Nothing captured a tick back then, and the
80+
* only record of when the session started is {@link Session#getStarted()} — a wall-clock instant,
81+
* because it is sent to Sentry. So this check stays on the wall clock, clock steps included.
82+
*
83+
* <p>TODO [MAJOR]: let a session remember the tick it started on, so this can use a {@link
84+
* Deadline} too. That tick must not be serialized.
85+
*/
86+
private boolean isSessionOnScopeStale() {
87+
final long nowMillis = TimeUnit.NANOSECONDS.toMillis(epochClock.now().epochNanos());
88+
// No session, or one that never recorded a start, leaves nothing to resume.
89+
final @NotNull AtomicBoolean stale = new AtomicBoolean(true);
90+
scopes.configureScope(
91+
scope -> {
92+
final @Nullable Session session = scope.getSession();
93+
final @Nullable Date started = session == null ? null : session.getStarted();
94+
if (started != null) {
95+
stale.set(started.getTime() + sessionIntervalMillis <= nowMillis);
96+
}
97+
});
98+
return stale.get();
8999
}
90100

91101
// App went to background and triggered this callback after 700ms
92102
// as no new screen was shown
93103
@Override
94104
public void onBackground() {
95-
final long currentTimeMillis = currentDateProvider.getCurrentTimeMillis();
96-
this.lastUpdatedSession.set(currentTimeMillis);
97-
98105
scopes.getOptions().getReplayController().onAppBackgrounded();
99106
scheduleEndSession();
100107

@@ -104,6 +111,9 @@ public void onBackground() {
104111
private void scheduleEndSession() {
105112
try (final @NotNull ISentryLifecycleToken ignored = endSessionLock.acquire()) {
106113
cancelTask();
114+
final @NotNull Deadline sessionEnd =
115+
Deadline.after(ticker, sessionIntervalMillis, TimeUnit.MILLISECONDS);
116+
this.sessionEnd = sessionEnd;
107117
final @NotNull Runnable endSession =
108118
() -> {
109119
if (enableSessionTracking) {
@@ -114,11 +124,13 @@ private void scheduleEndSession() {
114124
};
115125

116126
try {
127+
// The executor's own delay stops while the device is suspended, while the deadline keeps
128+
// counting, so this task can only run at or after the deadline. It needs no second check.
117129
endSessionFuture =
118130
scopes
119131
.getOptions()
120132
.getTimerExecutorService()
121-
.schedule(endSession, sessionIntervalMillis);
133+
.schedule(endSession, sessionEnd.remaining(TimeUnit.MILLISECONDS));
122134
} catch (Throwable e) {
123135
scopes
124136
.getOptions()
@@ -131,6 +143,16 @@ private void scheduleEndSession() {
131143
}
132144
}
133145

146+
/** Stops the pending end of session and hands back the deadline it was going to run at. */
147+
private @Nullable Deadline takeSessionEnd() {
148+
try (final @NotNull ISentryLifecycleToken ignored = endSessionLock.acquire()) {
149+
cancelTask();
150+
final @Nullable Deadline sessionEnd = this.sessionEnd;
151+
this.sessionEnd = null;
152+
return sessionEnd;
153+
}
154+
}
155+
134156
private void cancelTask() {
135157
try (final @NotNull ISentryLifecycleToken ignored = endSessionLock.acquire()) {
136158
if (endSessionFuture != null) {

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

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ import io.sentry.SentryLevel
1212
import io.sentry.SentryOptions
1313
import io.sentry.Session
1414
import io.sentry.Session.State
15-
import io.sentry.transport.ICurrentDateProvider
15+
import io.sentry.time.EpochClock
16+
import io.sentry.time.TestMonotonicTicker
17+
import io.sentry.time.Timestamp
18+
import java.util.concurrent.TimeUnit.HOURS
19+
import java.util.concurrent.TimeUnit.MILLISECONDS
20+
import java.util.concurrent.atomic.AtomicLong
1621
import kotlin.test.BeforeTest
1722
import kotlin.test.Test
1823
import kotlin.test.assertEquals
@@ -32,7 +37,10 @@ import org.mockito.kotlin.whenever
3237
class LifecycleWatcherTest {
3338
private class Fixture {
3439
val scopes = mock<IScopes>()
35-
val dateProvider = mock<ICurrentDateProvider>()
40+
val ticker = TestMonotonicTicker()
41+
// the wall clock, which only the staleness of a session already on the scope depends on
42+
val nowMillis = AtomicLong(0L)
43+
val epochClock = EpochClock { Timestamp.ofEpochNanos(MILLISECONDS.toNanos(nowMillis.get())) }
3644
// a real executor so scheduled end-session tasks actually run
3745
val options = SentryOptions().apply { setTimerExecutorService(SentryExecutorService(this)) }
3846
val replayController = mock<ReplayController>()
@@ -60,7 +68,8 @@ class LifecycleWatcherTest {
6068
sessionIntervalMillis,
6169
enableAutoSessionTracking,
6270
enableAppLifecycleBreadcrumbs,
63-
dateProvider,
71+
ticker,
72+
epochClock,
6473
)
6574
}
6675
}
@@ -81,26 +90,67 @@ class LifecycleWatcherTest {
8190
}
8291

8392
@Test
84-
fun `if last started session is after interval, start new session`() {
85-
val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false)
86-
whenever(fixture.dateProvider.currentTimeMillis).thenReturn(1L, 2L)
93+
fun `if the background window has elapsed, start new session`() {
94+
val watcher =
95+
fixture.getSUT(sessionIntervalMillis = 30000L, enableAppLifecycleBreadcrumbs = false)
8796
watcher.onForeground()
97+
watcher.onBackground()
98+
fixture.ticker.advance(30000, MILLISECONDS)
99+
88100
watcher.onForeground()
101+
89102
verify(fixture.scopes, times(2)).startSession()
90103
verify(fixture.replayController, times(2)).onAppForegrounded(true)
91104
}
92105

93106
@Test
94-
fun `if last started session is before interval, it should not start a new session`() {
95-
val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false)
96-
whenever(fixture.dateProvider.currentTimeMillis).thenReturn(2L, 1L)
107+
fun `if the app returns within the background window, it should not start a new session`() {
108+
val watcher =
109+
fixture.getSUT(sessionIntervalMillis = 30000L, enableAppLifecycleBreadcrumbs = false)
97110
watcher.onForeground()
111+
watcher.onBackground()
112+
fixture.ticker.advance(29999, MILLISECONDS)
113+
98114
watcher.onForeground()
115+
99116
verify(fixture.scopes).startSession()
100117
verify(fixture.replayController).onAppForegrounded(true)
101118
verify(fixture.replayController).onAppForegrounded(false)
102119
}
103120

121+
@Test
122+
fun `a wall clock stepping forward during the background window does not rotate the session`() {
123+
val watcher =
124+
fixture.getSUT(sessionIntervalMillis = 30000L, enableAppLifecycleBreadcrumbs = false)
125+
watcher.onForeground()
126+
watcher.onBackground()
127+
128+
// the device syncs its clock an hour forward, which two wall-clock reads used to report as an
129+
// hour spent in the background
130+
fixture.nowMillis.addAndGet(HOURS.toMillis(1))
131+
fixture.ticker.advance(1, MILLISECONDS)
132+
watcher.onForeground()
133+
134+
verify(fixture.scopes).startSession()
135+
verify(fixture.replayController).onAppForegrounded(false)
136+
}
137+
138+
@Test
139+
fun `a wall clock stepping backwards during the background window still rotates the session`() {
140+
val watcher =
141+
fixture.getSUT(sessionIntervalMillis = 30000L, enableAppLifecycleBreadcrumbs = false)
142+
fixture.nowMillis.set(HOURS.toMillis(1))
143+
watcher.onForeground()
144+
watcher.onBackground()
145+
146+
fixture.nowMillis.set(0L)
147+
fixture.ticker.advance(30000, MILLISECONDS)
148+
watcher.onForeground()
149+
150+
verify(fixture.scopes, times(2)).startSession()
151+
verify(fixture.replayController, times(2)).onAppForegrounded(true)
152+
}
153+
104154
@Test
105155
fun `if app goes to background, end session after interval`() {
106156
val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false)
@@ -249,7 +299,6 @@ class LifecycleWatcherTest {
249299

250300
@Test
251301
fun `background-foreground replay`() {
252-
whenever(fixture.dateProvider.currentTimeMillis).thenReturn(1L)
253302
val watcher =
254303
fixture.getSUT(sessionIntervalMillis = 500L, enableAppLifecycleBreadcrumbs = false)
255304
watcher.onForeground()

0 commit comments

Comments
 (0)