diff --git a/CHANGELOG.md b/CHANGELOG.md index 90ddcf1f068..32ce70bcab6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ ### Improvements - Speed up Session Replay and screenshot masking for large `maskViewClasses`/`unmaskViewClasses` lists by using constant-time class lookups ([#6410](https://github.com/getsentry/sentry-java/pull/6410)) +- Skip ANR profiling while a debugger is connected ([#6389](https://github.com/getsentry/sentry-java/pull/6389)) ### Fixes diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index e8b95794ea0..62d9b0d4960 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -65,6 +65,11 @@ public final class io/sentry/android/core/AndroidDateUtils { public static fun getCurrentSentryDateTime ()Lio/sentry/SentryDate; } +public final class io/sentry/android/core/AndroidDebugManager : io/sentry/IDebugManager { + public fun ()V + public fun isDebuggerConnected ()Z +} + public final class io/sentry/android/core/AndroidFatalLogger : io/sentry/ILogger { public fun ()V public fun (Ljava/lang/String;)V diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ANRWatchDog.java b/sentry-android-core/src/main/java/io/sentry/android/core/ANRWatchDog.java index b726dd0c881..c8f1bf5e361 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ANRWatchDog.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ANRWatchDog.java @@ -29,14 +29,15 @@ import android.app.ActivityManager; import android.content.Context; -import android.os.Debug; import android.os.SystemClock; +import io.sentry.IDebugManager; import io.sentry.ILogger; import io.sentry.SentryLevel; import io.sentry.transport.ICurrentDateProvider; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; /** A watchdog timer thread that detects when the UI thread has frozen. */ @@ -58,6 +59,7 @@ final class ANRWatchDog extends Thread { private final AtomicBoolean reported = new AtomicBoolean(false); private final @NotNull Context context; + private final @Nullable IDebugManager debugManager; @SuppressWarnings("UnnecessaryLambda") private final Runnable ticker; @@ -67,7 +69,8 @@ final class ANRWatchDog extends Thread { boolean reportInDebug, @NotNull ANRListener listener, @NotNull ILogger logger, - final @NotNull Context context) { + final @NotNull Context context, + final @Nullable IDebugManager debugManager) { // avoid method refs on Android due to some issues with older AGP setups // noinspection Convert2MethodRef this( @@ -78,7 +81,8 @@ final class ANRWatchDog extends Thread { listener, logger, new MainLooperHandler(), - context); + context, + debugManager); } @TestOnly @@ -90,7 +94,8 @@ final class ANRWatchDog extends Thread { @NotNull ANRListener listener, @NotNull ILogger logger, @NotNull MainLooperHandler uiHandler, - final @NotNull Context context) { + final @NotNull Context context, + final @Nullable IDebugManager debugManager) { super("|ANR-WatchDog|"); @@ -102,6 +107,7 @@ final class ANRWatchDog extends Thread { this.logger = logger; this.uiHandler = uiHandler; this.context = context; + this.debugManager = debugManager; this.ticker = () -> { lastKnownActiveUiTimestampMs = timeProvider.getCurrentTimeMillis(); @@ -145,7 +151,7 @@ public void run() { // If the main thread has not handled ticker, it is blocked. ANR. if (unresponsiveDurationMs > timeoutIntervalMillis) { - if (!reportInDebug && (Debug.isDebuggerConnected() || Debug.waitingForDebugger())) { + if (!reportInDebug && debugManager != null && debugManager.isDebuggerConnected()) { logger.log( SentryLevel.DEBUG, "An ANR was detected but ignored because the debugger is connected."); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidDebugManager.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidDebugManager.java new file mode 100644 index 00000000000..75afd02f24c --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidDebugManager.java @@ -0,0 +1,14 @@ +package io.sentry.android.core; + +import android.os.Debug; +import io.sentry.IDebugManager; +import org.jetbrains.annotations.ApiStatus; + +@ApiStatus.Internal +public final class AndroidDebugManager implements IDebugManager { + + @Override + public boolean isDebuggerConnected() { + return Debug.isDebuggerConnected() || Debug.waitingForDebugger(); + } +} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 7626acfb1ba..8ba216386b2 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -257,6 +257,9 @@ static void initializeIntegrationsAndProcessors( if (options.getThreadChecker() instanceof NoOpThreadChecker) { options.setThreadChecker(AndroidThreadChecker.getInstance()); } + if (options.getDebugManager() == null) { + options.setDebugManager(new AndroidDebugManager()); + } if (options.getSocketTagger() instanceof NoOpSocketTagger) { options.setSocketTagger(AndroidSocketTagger.getInstance()); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java index f37d433d308..b9fd0b781d8 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java @@ -100,7 +100,8 @@ private void startAnrWatchdog( options.isAnrReportInDebug(), error -> reportANR(scopes, options, error), options.getLogger(), - context); + context, + options.getDebugManager()); anrWatchDog.start(); options.getLogger().log(SentryLevel.DEBUG, "AnrIntegration installed."); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrProfilingIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrProfilingIntegration.java index 3cb0bea4b0f..bc41f3faf6a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrProfilingIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrProfilingIntegration.java @@ -5,6 +5,7 @@ import android.os.Handler; import android.os.Looper; import android.os.SystemClock; +import io.sentry.IDebugManager; import io.sentry.ILogger; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; @@ -54,6 +55,7 @@ public class AnrProfilingIntegration private volatile @Nullable AnrProfileManager profileManager; private volatile @NotNull ILogger logger = NoOpLogger.getInstance(); private volatile @Nullable SentryAndroidOptions options; + private volatile @Nullable IDebugManager debugManager; private volatile @Nullable Thread thread = null; private volatile boolean sampled = false; private volatile boolean inForeground = false; @@ -67,6 +69,7 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions (options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null, "SentryAndroidOptions is required"); this.logger = options.getLogger(); + this.debugManager = this.options.getDebugManager(); if (this.options.isAnrProfilingEnabled()) { if (this.options.getCacheDirPath() == null) { @@ -211,6 +214,15 @@ public void run() { @ApiStatus.Internal protected void checkMainThread(final @NotNull Thread mainThread) throws IOException { + if (debugManager != null && debugManager.isDebuggerConnected()) { + // The main thread can sit at a breakpoint for a long time, which is not an ANR. Reset the + // timestamp, so the paused time does not count against us after the debugger detaches. + mainThreadState = MainThreadState.IDLE; + sampled = false; + updater.run(); + return; + } + final long now = SystemClock.uptimeMillis(); final long diff = now - lastMainThreadExecutionTime; diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ANRWatchDogTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ANRWatchDogTest.kt index 2f3b6791c5b..02522f725f1 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ANRWatchDogTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ANRWatchDogTest.kt @@ -43,7 +43,17 @@ class ANRWatchDogTest { val interval = 10L val sut = - ANRWatchDog(timeProvider, interval, 1L, true, { a -> anr = a }, mock(), handler, mock()) + ANRWatchDog( + timeProvider, + interval, + 1L, + true, + { a -> anr = a }, + mock(), + handler, + mock(), + AndroidDebugManager(), + ) val es = Executors.newSingleThreadExecutor() try { es.submit { sut.run() } @@ -80,7 +90,17 @@ class ANRWatchDogTest { val interval = 10L val sut = - ANRWatchDog(timeProvider, interval, 1L, true, { a -> anr = a }, mock(), handler, mock()) + ANRWatchDog( + timeProvider, + interval, + 1L, + true, + { a -> anr = a }, + mock(), + handler, + mock(), + AndroidDebugManager(), + ) val es = Executors.newSingleThreadExecutor() try { es.submit { sut.run() } @@ -122,7 +142,17 @@ class ANRWatchDogTest { whenever(am.processesInErrorState).thenReturn(anrs) val sut = - ANRWatchDog(timeProvider, interval, 1L, true, { a -> anr = a }, mock(), handler, context) + ANRWatchDog( + timeProvider, + interval, + 1L, + true, + { a -> anr = a }, + mock(), + handler, + context, + AndroidDebugManager(), + ) val es = Executors.newSingleThreadExecutor() try { es.submit { sut.run() } @@ -168,7 +198,17 @@ class ANRWatchDogTest { whenever(am.processesInErrorState).thenReturn(anrs) val sut = - ANRWatchDog(timeProvider, interval, 1L, true, { a -> anr = a }, mock(), handler, context) + ANRWatchDog( + timeProvider, + interval, + 1L, + true, + { a -> anr = a }, + mock(), + handler, + context, + AndroidDebugManager(), + ) val es = Executors.newSingleThreadExecutor() try { es.submit { sut.run() } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 99b99b81120..7c7acf8ef3b 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -900,6 +900,13 @@ class AndroidOptionsInitializerTest { assertTrue { fixture.sentryOptions.threadChecker is AndroidThreadChecker } } + @Test + fun `AndroidDebugManager is set to options`() { + fixture.initSut() + + assertTrue { fixture.sentryOptions.debugManager is AndroidDebugManager } + } + @Test fun `AndroidSocketTagger is set to options`() { fixture.initSut() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/anr/AnrProfilingIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/anr/AnrProfilingIntegrationTest.kt index 2ae48fb3253..6d78a8f70c8 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/anr/AnrProfilingIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/anr/AnrProfilingIntegrationTest.kt @@ -196,6 +196,35 @@ class AnrProfilingIntegrationTest { assertEquals(AnrProfilingIntegration.MAX_NUM_STACKS, integration.numCollectedStacks.get()) } + @Test + fun `debugger pauses are treated as idle`() { + val mainThread = Thread.currentThread() + SystemClock.setCurrentTimeMillis(1_000) + var debuggerConnected = true + + val androidOptions = + SentryAndroidOptions().apply { + cacheDirPath = tmpDir.root.absolutePath + setLogger(mockLogger) + anrProfilingSampleRate = 1.0 + debugManager = { debuggerConnected } + } + + val integration = AnrProfilingIntegration() + integration.register(mockScopes, androidOptions) + + SystemClock.setCurrentTimeMillis(20_000) + integration.checkMainThread(mainThread) + assertEquals(AnrProfilingIntegration.MainThreadState.IDLE, integration.state) + assertEquals(0, integration.numCollectedStacks.get()) + + // the time spent paused must not count towards an ANR once the debugger detaches + debuggerConnected = false + SystemClock.setCurrentTimeMillis(20_100) + integration.checkMainThread(mainThread) + assertEquals(AnrProfilingIntegration.MainThreadState.IDLE, integration.state) + } + @Test fun `background foreground transitions don't trigger an ANR`() { val mainThread = Thread.currentThread() diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 6e1ebe4ff30..117a04565a0 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -908,6 +908,10 @@ public abstract interface class io/sentry/IContinuousProfiler { public abstract fun stopProfiler (Lio/sentry/ProfileLifecycle;)V } +public abstract interface class io/sentry/IDebugManager { + public abstract fun isDebuggerConnected ()Z +} + public abstract interface class io/sentry/IDistributionApi { public abstract fun checkForUpdate ()Ljava/util/concurrent/Future; public abstract fun checkForUpdateBlocking ()Lio/sentry/UpdateStatus; @@ -3798,6 +3802,7 @@ public class io/sentry/SentryOptions : io/sentry/transport/RateLimiterConfig { public fun getDataCollectionResolver ()Lio/sentry/DataCollectionResolver; public fun getDateProvider ()Lio/sentry/SentryDateProvider; public fun getDeadlineTimeout ()J + public fun getDebugManager ()Lio/sentry/IDebugManager; public fun getDebugMetaLoader ()Lio/sentry/internal/debugmeta/IDebugMetaLoader; public fun getDefaultScopeType ()Lio/sentry/ScopeType; public fun getDiagnosticLevel ()Lio/sentry/SentryLevel; @@ -3953,6 +3958,7 @@ public class io/sentry/SentryOptions : io/sentry/transport/RateLimiterConfig { public fun setDateProvider (Lio/sentry/SentryDateProvider;)V public fun setDeadlineTimeout (J)V public fun setDebug (Z)V + public fun setDebugManager (Lio/sentry/IDebugManager;)V public fun setDebugMetaLoader (Lio/sentry/internal/debugmeta/IDebugMetaLoader;)V public fun setDefaultScopeType (Lio/sentry/ScopeType;)V public fun setDiagnosticLevel (Lio/sentry/SentryLevel;)V diff --git a/sentry/src/main/java/io/sentry/IDebugManager.java b/sentry/src/main/java/io/sentry/IDebugManager.java new file mode 100644 index 00000000000..7d0bee68f9c --- /dev/null +++ b/sentry/src/main/java/io/sentry/IDebugManager.java @@ -0,0 +1,13 @@ +package io.sentry; + +import org.jetbrains.annotations.ApiStatus; + +@ApiStatus.Internal +public interface IDebugManager { + + /** + * Returns whether a debugger is currently attached or the process is waiting for a debugger to + * attach. + */ + boolean isDebuggerConnected(); +} diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index f4a17e3ebe5..4a056c7609c 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -520,6 +520,8 @@ public class SentryOptions implements RateLimiterConfig { private @NotNull IThreadChecker threadChecker = NoOpThreadChecker.getInstance(); + private @Nullable IDebugManager debugManager; + // TODO [MAJOR] this should default to false on the next major /** Whether OPTIONS requests should be traced. */ private boolean traceOptionsRequests = true; @@ -2752,6 +2754,16 @@ public void setThreadChecker(final @NotNull IThreadChecker threadChecker) { this.threadChecker = threadChecker; } + @ApiStatus.Internal + public @Nullable IDebugManager getDebugManager() { + return debugManager; + } + + @ApiStatus.Internal + public void setDebugManager(final @Nullable IDebugManager debugManager) { + this.debugManager = debugManager; + } + /** * Gets the performance collector used to collect performance stats in a time period. *