Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Comment thread
cursor[bot] marked this conversation as resolved.

addIntegrationToSdkVersion("FeedbackShake");
application.registerActivityLifecycleCallbacks(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment thread
runningcode marked this conversation as resolved.

private final @NotNull SampleQueue queue = new SampleQueue();

Expand All @@ -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);
}
Expand All @@ -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) {
Expand All @@ -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);
Expand All @@ -105,7 +123,8 @@ public void stop() {
}

/** Stops detection and releases the background thread. */
public void close() {
public synchronized void close() {
closed = true;
Comment thread
cursor[bot] marked this conversation as resolved.
stop();
if (handlerThread != null) {
// quitSafely drains pending messages (including the clear posted by stop) before exiting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,7 +25,11 @@ class FeedbackShakeIntegrationTest {
private class Fixture {
val application = mock<Application>()
val scopes = mock<Scopes>()
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<Activity>()
val formHandler = mock<SentryFeedbackOptions.IFormHandler>()

Expand Down Expand Up @@ -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)
Expand Down
Loading