fix(android): Don't block the main thread on the frame metrics looper

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
 

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

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<Window> currentWindow;
+  // volatile: read from the HandlerThread in onHandlerReady while set on the main thread
+  private volatile @Nullable WeakReference<Window> currentWindow;
   private final @NotNull Map<String, FrameMetricsCollectorListener> 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,34 +297,61 @@ 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;
     }
     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<Window> windowRef = currentWindow;
+    final @Nullable Window window = windowRef != null ? windowRef.get() : null;
     if (window != null && listenerMap.isEmpty()) {
       stopTrackingWindow(window);
     }
@@ -355,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<Window> windowRef = currentWindow;
+    final @Nullable Window window = windowRef != null ? windowRef.get() : null;
     if (window == null || !isAvailable) {
       return;
     }

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)

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<Boolean>("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?>("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?>("handler"))
+    val collector = fixture.getSut(context, prepare = false)
+    assertFalse(collector.getProperty<Boolean>("handlerThreadStarted"))
 
     collector.startCollection(mock())
-    assertNotNull(collector.getProperty<Handler?>("handler"))
+    assertTrue(collector.getProperty<Boolean>("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<Boolean>("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