diff --git a/CHANGELOG.md b/CHANGELOG.md index 44231ff4eed..a936f34b5be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- Reduce main-thread work during `Sentry.init` by resolving the shake-detector accelerometer off the main thread (~1.75ms on a Pixel 10) ([#5784](https://github.com/getsentry/sentry-java/pull/5784)) - Backfill release, environment, distribution, tags, and app version/build—and use the matching replay-on-error sample rate—for `ApplicationExitInfo` ANR and native crash events captured before SDK initialization, without reusing options cached by a later app update ([#5762](https://github.com/getsentry/sentry-java/pull/5762)) ## 8.49.0 diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java index fc34f18152f..b059b0104da 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java @@ -44,11 +44,29 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions : null, "SentryAndroidOptions is required"); - if (!this.options.getFeedbackOptions().isUseShakeGesture()) { + final @NotNull SentryAndroidOptions options = this.options; + + if (!options.getFeedbackOptions().isUseShakeGesture()) { return; } - shakeDetector.init(application, options.getLogger()); + // Re-arm the detector in case this integration is being re-registered after a previous close() + // (e.g. a second Sentry.init reusing the same options), otherwise the closed latch would keep + // shake detection off permanently. + shakeDetector.reopen(); + + // Resolving the accelerometer is the most expensive part of init (the first SensorManager + // access), so warm it up off the main thread. start() re-runs init() on demand, so shake + // detection still works if an activity resumes before this completes. + try { + options + .getExecutorService() + .submit(() -> shakeDetector.init(application, options.getLogger())); + } catch (Throwable t) { + options + .getLogger() + .log(SentryLevel.WARNING, "Failed to submit shake detector initialization.", t); + } addIntegrationToSdkVersion("FeedbackShake"); application.registerActivityLifecycleCallbacks(this); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java index a4c4ae0c4f5..9f4f73d10f4 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java @@ -40,6 +40,7 @@ public final class SentryShakeDetector implements SensorEventListener { private @Nullable Handler handler; private volatile @Nullable Listener listener; private @NotNull ILogger logger; + private boolean closed; private final @NotNull SampleQueue queue = new SampleQueue(); @@ -51,16 +52,29 @@ public SentryShakeDetector(final @NotNull ILogger logger) { this.logger = logger; } + /** + * Re-arms the detector after a previous {@link #close()} so it can be reused when the owning + * integration is registered again (e.g. a second {@code Sentry.init}). + */ + synchronized void reopen() { + closed = false; + } + /** * Initializes the sensor manager and accelerometer sensor. This is separated from start() so the * values can be resolved once and reused across activity transitions. */ - void init(final @NotNull Context context, final @NotNull ILogger logger) { + synchronized void init(final @NotNull Context context, final @NotNull ILogger logger) { this.logger = logger; init(context); } - private void init(final @NotNull Context context) { + private synchronized void init(final @NotNull Context context) { + // A warm-up submitted to the executor can be drained after close() (integrations are closed + // before the executor shuts down), so bail out instead of spinning up a new HandlerThread. + if (closed) { + return; + } if (sensorManager == null) { sensorManager = (SensorManager) context.getSystemService(Context.SENSOR_SERVICE); } @@ -74,7 +88,11 @@ private void init(final @NotNull Context context) { } } - public void start(final @NotNull Context context, final @NotNull Listener shakeListener) { + public synchronized void start( + final @NotNull Context context, final @NotNull Listener shakeListener) { + if (closed) { + return; + } this.listener = shakeListener; init(context); if (sensorManager == null) { @@ -89,7 +107,7 @@ public void start(final @NotNull Context context, final @NotNull Listener shakeL sensorManager.registerListener(this, accelerometer, SensorManager.SENSOR_DELAY_NORMAL, handler); } - public void stop() { + public synchronized void stop() { listener = null; if (sensorManager != null) { sensorManager.unregisterListener(this); @@ -105,7 +123,8 @@ public void stop() { } /** Stops detection and releases the background thread. */ - public void close() { + public synchronized void close() { + closed = true; stop(); if (handlerThread != null) { // quitSafely drains pending messages (including the clear posted by stop) before exiting diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt index bddc9395c0d..170211abf55 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt @@ -2,13 +2,18 @@ package io.sentry.android.core import android.app.Activity import android.app.Application +import android.content.Context import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.Scopes import io.sentry.SentryFeedbackOptions +import io.sentry.test.DeferredExecutorService +import io.sentry.test.ImmediateExecutorService import kotlin.test.BeforeTest import kotlin.test.Test import org.junit.runner.RunWith import org.mockito.kotlin.any +import org.mockito.kotlin.atLeastOnce +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -20,7 +25,11 @@ class FeedbackShakeIntegrationTest { private class Fixture { val application = mock() val scopes = mock() - val options = SentryAndroidOptions().apply { dsn = "https://key@sentry.io/proj" } + val options = + SentryAndroidOptions().apply { + dsn = "https://key@sentry.io/proj" + executorService = ImmediateExecutorService() + } val activity = mock() val formHandler = mock() @@ -49,6 +58,59 @@ class FeedbackShakeIntegrationTest { verify(fixture.application).registerActivityLifecycleCallbacks(any()) } + @Test + fun `resolves the accelerometer sensor off the main thread`() { + val deferredExecutor = DeferredExecutorService() + fixture.options.executorService = deferredExecutor + whenever(fixture.application.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + // Callback registration stays synchronous, but the expensive SensorManager lookup is deferred. + verify(fixture.application).registerActivityLifecycleCallbacks(any()) + verify(fixture.application, never()).getSystemService(eq(Context.SENSOR_SERVICE)) + + deferredExecutor.runAll() + + verify(fixture.application).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `warm-up drained after close does not resolve the sensor`() { + // Integrations are closed before the executor drains, so a queued warm-up can run after + // close(). It must be a no-op rather than resolving the sensor and spinning up a HandlerThread. + val deferredExecutor = DeferredExecutorService() + fixture.options.executorService = deferredExecutor + whenever(fixture.application.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + sut.close() + + deferredExecutor.runAll() + + verify(fixture.application, never()).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `re-registering after close re-arms shake detection`() { + // A second Sentry.init reusing the same integration must revive shake detection rather than + // stay off because of the closed latch. + val deferredExecutor = DeferredExecutorService() + fixture.options.executorService = deferredExecutor + whenever(fixture.application.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + sut.close() + sut.register(fixture.scopes, fixture.options) + + deferredExecutor.runAll() + + verify(fixture.application, atLeastOnce()).getSystemService(eq(Context.SENSOR_SERVICE)) + } + @Test fun `when useShakeGesture is disabled does not register activity lifecycle callbacks`() { val sut = fixture.getSut(useShakeGesture = false)