From e2cdb56cb6e6d84726b851072c1e7663eb85dfbc Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 6 Nov 2026 14:30:47 +0100 Subject: [PATCH 1/3] fix(android): Don't block the main thread on the frame metrics looper SentryFrameMetricsCollector waited on HandlerThread.getLooper() at the first startCollection(), blocking Activity.onCreate for hundreds of ms on a busy device. Create the handler in onLooperPrepared() instead, and start the thread at init when frames tracking will use it, so its looper is ready before the first frame. Fixes #6443 Co-Authored-By: Claude Opus 5.5 --- .../core/SpanFrameMetricsCollector.java | 4 ++ .../util/SentryFrameMetricsCollector.java | 56 +++++++++++++----- .../core/SpanFrameMetricsCollectorTest.kt | 24 +++++++- .../util/SentryFrameMetricsCollectorTest.kt | 58 +++++++++++++++---- 4 files changed, 117 insertions(+), 25 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SpanFrameMetricsCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/SpanFrameMetricsCollector.java index 074a4a6ea51..9eb854ca0bf 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SpanFrameMetricsCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SpanFrameMetricsCollector.java @@ -74,6 +74,10 @@ public SpanFrameMetricsCollector( this.frameMetricsCollector = frameMetricsCollector; enabled = options.isEnablePerformanceV2() && options.isEnableFramesTracking(); + if (enabled && options.isTracingEnabled()) { + // Start the frame metrics thread now, so it's ready before the first activity's first frame. + frameMetricsCollector.prepare(); + } } @Override diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java index 2c0ae246558..8a9c6e712b6 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java @@ -35,6 +35,7 @@ import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.VisibleForTesting; @ApiStatus.Internal public final class SentryFrameMetricsCollector implements Application.ActivityLifecycleCallbacks { @@ -48,8 +49,10 @@ public final class SentryFrameMetricsCollector implements Application.ActivityLi private final @NotNull ILogger logger; private volatile @Nullable Handler handler; + private volatile boolean handlerThreadStarted = false; private final @NotNull AutoClosableReentrantLock handlerLock = new AutoClosableReentrantLock(); - private @Nullable WeakReference currentWindow; + // volatile: read from the HandlerThread in onHandlerReady while set on the main thread + private volatile @Nullable WeakReference currentWindow; private final @NotNull Map listenerMap = new ConcurrentHashMap<>(); private boolean isAvailable = false; @@ -116,8 +119,8 @@ public SentryFrameMetricsCollector( } isAvailable = true; - // The frame metrics HandlerThread is started lazily on the first startCollection() call. - // Starting it here would block the main thread on HandlerThread.getLooper() during SDK init. + // The frame metrics HandlerThread is started by prepare() or the first startCollection() call, + // and never waited on: see ensureHandlerThreadStarted(). // We have to register the lifecycle callback, even if no profile is started, otherwise when we // start a profile, we wouldn't have the current activity and couldn't get the frameMetrics. @@ -294,26 +297,51 @@ public void onActivityDestroyed(@NotNull Activity activity) {} } /** - * Lazily starts the background HandlerThread used to receive frame metrics. Deferred out of the - * constructor because {@link HandlerThread#getLooper()} blocks the caller (the main thread during - * SDK init) until the thread is ready, and the handler is only needed once collection starts. + * Starts the background HandlerThread ahead of the first {@link #startCollection} call, so its + * looper is ready by the time the first frame is drawn. Does not block. + */ + public void prepare() { + if (isAvailable) { + ensureHandlerThreadStarted(); + } + } + + /** + * Starts the background HandlerThread used to receive frame metrics without waiting for it: the + * handler is created on the new thread in {@link HandlerThread#onLooperPrepared()}. Calling + * {@link HandlerThread#getLooper()} here would block the caller (the main thread) until the new + * thread is scheduled, which takes hundreds of ms on a busy device. */ private void ensureHandlerThreadStarted() { - if (handler != null) { + if (handlerThreadStarted) { return; } try (final @NotNull ISentryLifecycleToken ignored = handlerLock.acquire()) { - if (handler == null) { - final HandlerThread handlerThread = - new HandlerThread("io.sentry.android.core.internal.util.SentryFrameMetricsCollector"); - handlerThread.setUncaughtExceptionHandler( - (thread, e) -> logger.log(SentryLevel.ERROR, "Error during frames measurements.", e)); - handlerThread.start(); - handler = new Handler(handlerThread.getLooper()); + if (handlerThreadStarted) { + return; } + handlerThreadStarted = true; + final HandlerThread handlerThread = + new HandlerThread("io.sentry.android.core.internal.util.SentryFrameMetricsCollector") { + @Override + protected void onLooperPrepared() { + onHandlerReady(new Handler(getLooper())); + } + }; + handlerThread.setUncaughtExceptionHandler( + (thread, e) -> logger.log(SentryLevel.ERROR, "Error during frames measurements.", e)); + handlerThread.start(); } } + @VisibleForTesting + void onHandlerReady(final @NotNull Handler handler) { + this.handler = handler; + // startCollection() or an activity start may have happened before the handler existed, in + // which case trackCurrentWindow() skipped attaching the listener + trackCurrentWindow(); + } + public void stopCollection(final @Nullable String listenerId) { if (!isAvailable) { return; diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SpanFrameMetricsCollectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SpanFrameMetricsCollectorTest.kt index 2b6f19a8d31..d5932b75b90 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SpanFrameMetricsCollectorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SpanFrameMetricsCollectorTest.kt @@ -27,7 +27,10 @@ class SpanFrameMetricsCollectorTest { var timeNanos = 0L var lastKnownChoreographerFrameTimeNanos = 0L - fun getSut(enabled: Boolean = true): SpanFrameMetricsCollector { + fun getSut( + enabled: Boolean = true, + tracesSampleRate: Double? = 1.0, + ): SpanFrameMetricsCollector { whenever(frameMetricsCollector.startCollection(any())) .thenReturn(UUID.randomUUID().toString()) whenever(frameMetricsCollector.getLastKnownFrameStartTimeNanos()).thenAnswer { @@ -37,6 +40,7 @@ class SpanFrameMetricsCollectorTest { options.isEnableFramesTracking = enabled options.isEnablePerformanceV2 = enabled options.dateProvider = SentryAndroidDateProvider() + options.tracesSampleRate = tracesSampleRate return SpanFrameMetricsCollector(options, frameMetricsCollector) } @@ -90,6 +94,24 @@ class SpanFrameMetricsCollectorTest { verify(fixture.frameMetricsCollector, never()).startCollection(any()) } + @Test + fun `When enabled and tracing is enabled, it prepares the frame metrics collector`() { + fixture.getSut() + verify(fixture.frameMetricsCollector).prepare() + } + + @Test + fun `When tracing is disabled, it doesn't prepare the frame metrics collector`() { + fixture.getSut(tracesSampleRate = null) + verify(fixture.frameMetricsCollector, never()).prepare() + } + + @Test + fun `When disabled, it doesn't prepare the frame metrics collector`() { + fixture.getSut(enabled = false) + verify(fixture.frameMetricsCollector, never()).prepare() + } + @Test fun `If disabled but a span is launched, it doesn't register a listener`() { val sut = fixture.getSut(enabled = false) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollectorTest.kt index 334ce229066..fb4dbdc67fd 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollectorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollectorTest.kt @@ -83,17 +83,30 @@ class SentryFrameMetricsCollectorTest { fun getSut( context: Context, buildInfoProvider: BuildInfoProvider = buildInfo, + prepare: Boolean = true, ): SentryFrameMetricsCollector { whenever(activity.window).thenReturn(window) whenever(activity2.window).thenReturn(window2) addOnFrameMetricsAvailableListenerCounter = 0 removeOnFrameMetricsAvailableListenerCounter = 0 - return SentryFrameMetricsCollector( - context, - options, - buildInfoProvider, - windowFrameMetricsManager, - ) + val collector = + SentryFrameMetricsCollector(context, options, buildInfoProvider, windowFrameMetricsManager) + if (prepare) { + collector.prepare() + if (collector.getProperty("handlerThreadStarted")) { + awaitHandler(collector) + } + } + return collector + } + + // the handler is created asynchronously on the HandlerThread once its looper is prepared + fun awaitHandler(collector: SentryFrameMetricsCollector) { + val deadline = System.currentTimeMillis() + 5_000 + while (collector.getProperty("handler") == null) { + check(System.currentTimeMillis() < deadline) { "frame metrics handler was never created" } + Thread.sleep(1) + } } } @@ -142,12 +155,37 @@ class SentryFrameMetricsCollectorTest { @Test fun `handler thread is started lazily on first startCollection`() { - val collector = fixture.getSut(context) - // not started during construction (would block the main thread on getLooper at SDK init) - assertNull(collector.getProperty("handler")) + val collector = fixture.getSut(context, prepare = false) + assertFalse(collector.getProperty("handlerThreadStarted")) collector.startCollection(mock()) - assertNotNull(collector.getProperty("handler")) + assertTrue(collector.getProperty("handlerThreadStarted")) + fixture.awaitHandler(collector) + } + + @Test + fun `prepare starts the handler thread before startCollection`() { + val collector = fixture.getSut(context, prepare = false) + + collector.prepare() + assertTrue(collector.getProperty("handlerThreadStarted")) + fixture.awaitHandler(collector) + } + + @Test + fun `listener is attached once the handler is ready if collection started before it`() { + val collector = fixture.getSut(context, prepare = false) + // pretend the HandlerThread is started but its looper isn't prepared yet + collector.injectForField("handlerThreadStarted", true) + + collector.startCollection(mock()) + collector.onActivityStarted(fixture.activity) + Shadows.shadowOf(Looper.getMainLooper()).idle() + assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) + + collector.onHandlerReady(Handler(Looper.getMainLooper())) + Shadows.shadowOf(Looper.getMainLooper()).idle() + assertEquals(1, fixture.addOnFrameMetricsAvailableListenerCounter) } @Test From 0f21f5513110ef3398b1a0a20c1b970e5932f613 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 6 Nov 2026 14:31:22 +0100 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ff6b1f542e..5c5b1b035f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ - Session Replay: Fix `SentryOkHttpInterceptor` hanging forever on responses whose body has no known length, such as Server-Sent Events or a gzipped response, when network details are enabled for the URL ([#6231](https://github.com/getsentry/sentry-java/pull/6231)) - Session Replay: `SentryOkHttpInterceptor` now captures network request and response details only while Session Replay is recording or buffering ([#6486](https://github.com/getsentry/sentry-java/pull/6486)) - Stop marking an OkHttp call span as failed when a connection attempt to one of the host's addresses fails but OkHttp connects on another ([#6472](https://github.com/getsentry/sentry-java/pull/6472)) +- Stop blocking the main thread while the frame metrics thread starts when the first activity transaction begins ([#6500](https://github.com/getsentry/sentry-java/pull/6500)) + - On a busy device during cold start this could stall the launcher activity's `onCreate` for hundreds of milliseconds. ### Dependencies From 9748ecdb4812003251850bafa55d56825f82c54c Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 6 Nov 2026 16:13:33 +0100 Subject: [PATCH 3/3] fix(android): Read the current frame metrics window once per check trackCurrentWindow() now also runs on the HandlerThread in onLooperPrepared(). Reading the volatile currentWindow twice could NPE if onActivityStopped nulled it in between, killing the thread before Looper.loop() and silently stopping frame metrics. Co-Authored-By: Claude Opus 5.5 --- .../core/internal/util/SentryFrameMetricsCollector.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java index 8a9c6e712b6..bc3bff9b700 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java @@ -349,7 +349,9 @@ public void stopCollection(final @Nullable String listenerId) { if (listenerId != null) { listenerMap.remove(listenerId); } - Window window = currentWindow != null ? currentWindow.get() : null; + // read the volatile field once: onActivityStopped may null it concurrently + final @Nullable WeakReference windowRef = currentWindow; + final @Nullable Window window = windowRef != null ? windowRef.get() : null; if (window != null && listenerMap.isEmpty()) { stopTrackingWindow(window); } @@ -383,7 +385,9 @@ private void setCurrentWindow(final @NotNull Window window) { @SuppressLint("NewApi") private void trackCurrentWindow() { - @Nullable Window window = currentWindow != null ? currentWindow.get() : null; + // read the volatile field once: onActivityStopped may null it concurrently + final @Nullable WeakReference windowRef = currentWindow; + final @Nullable Window window = windowRef != null ? windowRef.get() : null; if (window == null || !isAvailable) { return; }