Skip to content

Commit 50973fd

Browse files
runningcodeclaude
andcommitted
ref(android): Require an installed ticker for ANR profiling (JAVA-579)
Drop the JavaMonotonicTicker fallback. On Android nanoTime() stops while the device is suspended, so a default that is never meant to be used would measure main-thread stalls against the wrong clock if it ever were. Start the test ticker at a non-zero origin. A fake that starts at zero makes a reading taken against a different origin indistinguishable from a correct one, which is the mistake this whole change exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 76d5e07 commit 50973fd

2 files changed

Lines changed: 40 additions & 8 deletions

File tree

‎sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrProfilingIntegration.java‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import io.sentry.SentryOptions;
1414
import io.sentry.android.core.AppState;
1515
import io.sentry.android.core.SentryAndroidOptions;
16-
import io.sentry.time.JavaMonotonicTicker;
1716
import io.sentry.time.MonotonicTicker;
1817
import io.sentry.time.MonotonicTickerProvider;
1918
import io.sentry.time.Stopwatch;
@@ -47,10 +46,22 @@ public class AnrProfilingIntegration
4746
static final int MAX_NUM_STACKS = (int) (10_000 / POLLING_INTERVAL_MS);
4847

4948
private final AtomicBoolean enabled = new AtomicBoolean(true);
50-
private volatile @NotNull MonotonicTicker ticker = JavaMonotonicTicker.getInstance();
49+
50+
/**
51+
* Null until {@link #register} installs one. There is deliberately no default: the core module's
52+
* ticker reads {@link System#nanoTime()}, which stops while the device is suspended, so falling
53+
* back to it on Android would silently measure stalls against the wrong clock.
54+
*/
55+
private volatile @Nullable MonotonicTicker ticker;
5156

5257
@SuppressWarnings("UnnecessaryLambda")
53-
private final @NotNull Runnable updater = () -> lastMainThreadExecutionNanos = ticker.tickNanos();
58+
private final @NotNull Runnable updater =
59+
() -> {
60+
final @Nullable MonotonicTicker currentTicker = ticker;
61+
if (currentTicker != null) {
62+
lastMainThreadExecutionNanos = currentTicker.tickNanos();
63+
}
64+
};
5465

5566
private final @NotNull AutoClosableReentrantLock lifecycleLock = new AutoClosableReentrantLock();
5667
private final @NotNull AutoClosableReentrantLock profileManagerLock =
@@ -76,8 +87,9 @@ public class AnrProfilingIntegration
7687
* SentryAndroidOptions} cannot supply.
7788
*/
7889
void installTickerFrom(final @NotNull MonotonicTickerProvider provider) {
79-
this.ticker = provider.getMonotonicTicker();
80-
this.lastMainThreadExecutionNanos = ticker.tickNanos();
90+
final @NotNull MonotonicTicker installedTicker = provider.getMonotonicTicker();
91+
this.ticker = installedTicker;
92+
this.lastMainThreadExecutionNanos = installedTicker.tickNanos();
8193
}
8294

8395
@Override
@@ -232,8 +244,12 @@ public void run() {
232244

233245
@ApiStatus.Internal
234246
protected void checkMainThread(final @NotNull Thread mainThread) throws IOException {
247+
final @Nullable MonotonicTicker currentTicker = ticker;
248+
if (currentTicker == null) {
249+
return;
250+
}
235251
final long diff =
236-
TimeUnit.NANOSECONDS.toMillis(ticker.tickNanos() - lastMainThreadExecutionNanos);
252+
TimeUnit.NANOSECONDS.toMillis(currentTicker.tickNanos() - lastMainThreadExecutionNanos);
237253

238254
if (diff < THRESHOLD_SUSPICION_MS) {
239255
mainThreadState = MainThreadState.IDLE;
@@ -262,7 +278,7 @@ protected void checkMainThread(final @NotNull Thread mainThread) throws IOExcept
262278
&& (mainThreadState == MainThreadState.SUSPICIOUS
263279
|| mainThreadState == MainThreadState.ANR_DETECTED)) {
264280
if (numCollectedStacks.get() < MAX_NUM_STACKS) {
265-
final @NotNull Stopwatch stopwatch = Stopwatch.started(ticker);
281+
final @NotNull Stopwatch stopwatch = Stopwatch.started(currentTicker);
266282
final @NotNull AnrStackTrace trace =
267283
new AnrStackTrace(System.currentTimeMillis(), mainThread.getStackTrace());
268284
if (logger.isEnabled(SentryLevel.DEBUG)) {

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.sentry.android.core.anr
22

33
import androidx.test.ext.junit.runners.AndroidJUnit4
4+
import com.google.common.truth.Truth.assertThat
45
import io.sentry.ILogger
56
import io.sentry.IScopes
67
import io.sentry.SentryIntegrationPackageStorage
@@ -9,6 +10,7 @@ import io.sentry.android.core.AppState
910
import io.sentry.android.core.SentryAndroidOptions
1011
import io.sentry.test.getProperty
1112
import io.sentry.time.TestMonotonicTicker
13+
import java.util.concurrent.TimeUnit.HOURS
1214
import java.util.concurrent.TimeUnit.MILLISECONDS
1315
import kotlin.test.AfterTest
1416
import kotlin.test.BeforeTest
@@ -35,7 +37,9 @@ class AnrProfilingIntegrationTest {
3537

3638
@BeforeTest
3739
fun setup() {
38-
ticker = TestMonotonicTicker()
40+
// A tick origin far from zero, as on any device that has been up a while. Starting at zero
41+
// would let a reading taken against a different origin pass unnoticed.
42+
ticker = TestMonotonicTicker(HOURS.toNanos(30))
3943
mockScopes = mock()
4044
mockLogger = mock()
4145
options =
@@ -223,6 +227,18 @@ class AnrProfilingIntegrationTest {
223227
assertEquals(AnrProfilingIntegration.MainThreadState.IDLE, integration.state)
224228
}
225229

230+
@Test
231+
fun `does not report a stall when the ticker origin is far from zero`() {
232+
val integration = AnrProfilingIntegration()
233+
integration.register(mockScopes, options)
234+
integration.installTickerFrom { ticker }
235+
236+
integration.checkMainThread(Thread.currentThread())
237+
238+
assertThat(integration.state).isEqualTo(AnrProfilingIntegration.MainThreadState.IDLE)
239+
assertThat(integration.profileManager.load().stacks).isEmpty()
240+
}
241+
226242
@Test
227243
fun `does not register when options is not SentryAndroidOptions`() {
228244
val plainOptions =

0 commit comments

Comments
 (0)