diff --git a/CHANGELOG.md b/CHANGELOG.md index 09622493a2..dc0eb89f12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- 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 ([#6096](https://github.com/getsentry/sentry-java/pull/6096)) + ## 8.56.0 ### Behavioral Changes diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java index 9fd90b2309..5817dae47c 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java +++ b/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); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java index ca874e714e..f0078bc353 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java @@ -5,53 +5,52 @@ 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. + * + *
Only read or written while holding {@link #endSessionLock}, which is also what lets + * cancelling the pending task and taking this deadline happen as one step. + */ + 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 +60,48 @@ 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 on the scope is too old to resume, so foregrounding should start a new one. + * + *
Used when no background window is pending, which means the session was started by SDK init + * rather than by leaving and returning to the app. Nothing captured a tick back then, and the + * only record of when the session started is {@link Session#getStarted()} — a wall-clock instant, + * because it is sent to Sentry. So this check stays on the wall clock, clock steps included. + * + *
TODO [MAJOR]: let a session remember the tick it started on, so this can use a {@link
+ * Deadline} too. That tick must not be serialized.
+ */
+ 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 +111,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 +124,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 +143,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) {
diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt
index 5f14e029d0..ba8b97d1fc 100644
--- a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt
+++ b/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