Skip to content

Commit 8d299fc

Browse files
runningcodeclaude
andauthored
perf(android): Init shake detector off the main thread (#5784)
* perf(android): Init shake detector off the main thread (JAVA-618) FeedbackShakeIntegration.register() resolved the accelerometer via SensorManager synchronously on the calling thread, which under auto-init is the main thread. On a Pixel 10 this first SensorManager access measured ~1.75ms, making it the single most expensive integration in the Sentry.init register loop. Submit the pre-warm init() to the executor service instead. start() already re-runs the idempotent init() on demand, so shake detection still works if an activity resumes before the warm-up completes. SentryShakeDetector's lifecycle methods are now synchronized so the executor warm-up and a main-thread start() cannot race on the sensor fields. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: Add changelog for shake-detector init off main thread (JAVA-618) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(android): Guard shake detector against warm-up after close (JAVA-618) A warm-up init submitted to the executor could be drained after the integration's close() ran, since integrations shut down before the executor. That re-resolved the sensor and leaked a HandlerThread. Guard init()/start() with a closed latch so a late warm-up is a no-op. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(android): Re-arm shake detector on re-register (JAVA-618) The closed latch added to neutralize a warm-up drained after close() was permanent, so re-registering the same integration (e.g. a second Sentry.init reusing the same options) left shake detection off with no recovery. register() now re-arms the detector via reopen(), which the stale-warm-up path (init()) deliberately does not, preserving the drain-after-close guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 0e94865 commit 8d299fc

4 files changed

Lines changed: 108 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
### Fixes
66

7+
- 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))
78
- 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))
89
- `SentryTagModifierNode.isImportantForBounds` now matches the default behavior and returns `true` ([#5789](https://github.com/getsentry/sentry-java/pull/5789))
910
- Prevent a `StackOverflowError` when a `beforeSend`, `beforeBreadcrumb`, `beforeSendLog`, or `beforeEnvelope` callback triggers another capture (directly or through a logging integration such as Timber) ([#5737](https://github.com/getsentry/sentry-java/pull/5737))

sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,29 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
4444
: null,
4545
"SentryAndroidOptions is required");
4646

47-
if (!this.options.getFeedbackOptions().isUseShakeGesture()) {
47+
final @NotNull SentryAndroidOptions options = this.options;
48+
49+
if (!options.getFeedbackOptions().isUseShakeGesture()) {
4850
return;
4951
}
5052

51-
shakeDetector.init(application, options.getLogger());
53+
// Re-arm the detector in case this integration is being re-registered after a previous close()
54+
// (e.g. a second Sentry.init reusing the same options), otherwise the closed latch would keep
55+
// shake detection off permanently.
56+
shakeDetector.reopen();
57+
58+
// Resolving the accelerometer is the most expensive part of init (the first SensorManager
59+
// access), so warm it up off the main thread. start() re-runs init() on demand, so shake
60+
// detection still works if an activity resumes before this completes.
61+
try {
62+
options
63+
.getExecutorService()
64+
.submit(() -> shakeDetector.init(application, options.getLogger()));
65+
} catch (Throwable t) {
66+
options
67+
.getLogger()
68+
.log(SentryLevel.WARNING, "Failed to submit shake detector initialization.", t);
69+
}
5270

5371
addIntegrationToSdkVersion("FeedbackShake");
5472
application.registerActivityLifecycleCallbacks(this);

sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public final class SentryShakeDetector implements SensorEventListener {
4040
private @Nullable Handler handler;
4141
private volatile @Nullable Listener listener;
4242
private @NotNull ILogger logger;
43+
private boolean closed;
4344

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

@@ -51,16 +52,29 @@ public SentryShakeDetector(final @NotNull ILogger logger) {
5152
this.logger = logger;
5253
}
5354

55+
/**
56+
* Re-arms the detector after a previous {@link #close()} so it can be reused when the owning
57+
* integration is registered again (e.g. a second {@code Sentry.init}).
58+
*/
59+
synchronized void reopen() {
60+
closed = false;
61+
}
62+
5463
/**
5564
* Initializes the sensor manager and accelerometer sensor. This is separated from start() so the
5665
* values can be resolved once and reused across activity transitions.
5766
*/
58-
void init(final @NotNull Context context, final @NotNull ILogger logger) {
67+
synchronized void init(final @NotNull Context context, final @NotNull ILogger logger) {
5968
this.logger = logger;
6069
init(context);
6170
}
6271

63-
private void init(final @NotNull Context context) {
72+
private synchronized void init(final @NotNull Context context) {
73+
// A warm-up submitted to the executor can be drained after close() (integrations are closed
74+
// before the executor shuts down), so bail out instead of spinning up a new HandlerThread.
75+
if (closed) {
76+
return;
77+
}
6478
if (sensorManager == null) {
6579
sensorManager = (SensorManager) context.getSystemService(Context.SENSOR_SERVICE);
6680
}
@@ -74,7 +88,11 @@ private void init(final @NotNull Context context) {
7488
}
7589
}
7690

77-
public void start(final @NotNull Context context, final @NotNull Listener shakeListener) {
91+
public synchronized void start(
92+
final @NotNull Context context, final @NotNull Listener shakeListener) {
93+
if (closed) {
94+
return;
95+
}
7896
this.listener = shakeListener;
7997
init(context);
8098
if (sensorManager == null) {
@@ -89,7 +107,7 @@ public void start(final @NotNull Context context, final @NotNull Listener shakeL
89107
sensorManager.registerListener(this, accelerometer, SensorManager.SENSOR_DELAY_NORMAL, handler);
90108
}
91109

92-
public void stop() {
110+
public synchronized void stop() {
93111
listener = null;
94112
if (sensorManager != null) {
95113
sensorManager.unregisterListener(this);
@@ -105,7 +123,8 @@ public void stop() {
105123
}
106124

107125
/** Stops detection and releases the background thread. */
108-
public void close() {
126+
public synchronized void close() {
127+
closed = true;
109128
stop();
110129
if (handlerThread != null) {
111130
// quitSafely drains pending messages (including the clear posted by stop) before exiting

sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,18 @@ package io.sentry.android.core
22

33
import android.app.Activity
44
import android.app.Application
5+
import android.content.Context
56
import androidx.test.ext.junit.runners.AndroidJUnit4
67
import io.sentry.Scopes
78
import io.sentry.SentryFeedbackOptions
9+
import io.sentry.test.DeferredExecutorService
10+
import io.sentry.test.ImmediateExecutorService
811
import kotlin.test.BeforeTest
912
import kotlin.test.Test
1013
import org.junit.runner.RunWith
1114
import org.mockito.kotlin.any
15+
import org.mockito.kotlin.atLeastOnce
16+
import org.mockito.kotlin.eq
1217
import org.mockito.kotlin.mock
1318
import org.mockito.kotlin.never
1419
import org.mockito.kotlin.verify
@@ -20,7 +25,11 @@ class FeedbackShakeIntegrationTest {
2025
private class Fixture {
2126
val application = mock<Application>()
2227
val scopes = mock<Scopes>()
23-
val options = SentryAndroidOptions().apply { dsn = "https://key@sentry.io/proj" }
28+
val options =
29+
SentryAndroidOptions().apply {
30+
dsn = "https://key@sentry.io/proj"
31+
executorService = ImmediateExecutorService()
32+
}
2433
val activity = mock<Activity>()
2534
val formHandler = mock<SentryFeedbackOptions.IFormHandler>()
2635

@@ -49,6 +58,59 @@ class FeedbackShakeIntegrationTest {
4958
verify(fixture.application).registerActivityLifecycleCallbacks(any())
5059
}
5160

61+
@Test
62+
fun `resolves the accelerometer sensor off the main thread`() {
63+
val deferredExecutor = DeferredExecutorService()
64+
fixture.options.executorService = deferredExecutor
65+
whenever(fixture.application.getSystemService(any())).thenReturn(null)
66+
67+
val sut = fixture.getSut(useShakeGesture = true)
68+
sut.register(fixture.scopes, fixture.options)
69+
70+
// Callback registration stays synchronous, but the expensive SensorManager lookup is deferred.
71+
verify(fixture.application).registerActivityLifecycleCallbacks(any())
72+
verify(fixture.application, never()).getSystemService(eq(Context.SENSOR_SERVICE))
73+
74+
deferredExecutor.runAll()
75+
76+
verify(fixture.application).getSystemService(eq(Context.SENSOR_SERVICE))
77+
}
78+
79+
@Test
80+
fun `warm-up drained after close does not resolve the sensor`() {
81+
// Integrations are closed before the executor drains, so a queued warm-up can run after
82+
// close(). It must be a no-op rather than resolving the sensor and spinning up a HandlerThread.
83+
val deferredExecutor = DeferredExecutorService()
84+
fixture.options.executorService = deferredExecutor
85+
whenever(fixture.application.getSystemService(any())).thenReturn(null)
86+
87+
val sut = fixture.getSut(useShakeGesture = true)
88+
sut.register(fixture.scopes, fixture.options)
89+
sut.close()
90+
91+
deferredExecutor.runAll()
92+
93+
verify(fixture.application, never()).getSystemService(eq(Context.SENSOR_SERVICE))
94+
}
95+
96+
@Test
97+
fun `re-registering after close re-arms shake detection`() {
98+
// A second Sentry.init reusing the same integration must revive shake detection rather than
99+
// stay off because of the closed latch.
100+
val deferredExecutor = DeferredExecutorService()
101+
fixture.options.executorService = deferredExecutor
102+
whenever(fixture.application.getSystemService(any())).thenReturn(null)
103+
104+
val sut = fixture.getSut(useShakeGesture = true)
105+
sut.register(fixture.scopes, fixture.options)
106+
sut.close()
107+
sut.register(fixture.scopes, fixture.options)
108+
109+
deferredExecutor.runAll()
110+
111+
verify(fixture.application, atLeastOnce()).getSystemService(eq(Context.SENSOR_SERVICE))
112+
}
113+
52114
@Test
53115
fun `when useShakeGesture is disabled does not register activity lifecycle callbacks`() {
54116
val sut = fixture.getSut(useShakeGesture = false)

0 commit comments

Comments
 (0)