Skip to content
Open
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 @@ -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

Expand Down
1 change: 1 addition & 0 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,7 @@ public class io/sentry/android/core/anr/AnrProfilingIntegration : io/sentry/Inte
public fun close ()V
protected fun getProfileManager ()Lio/sentry/android/core/anr/AnrProfileManager;
protected fun getState ()Lio/sentry/android/core/anr/AnrProfilingIntegration$MainThreadState;
protected fun isDebuggerConnected ()Z
public fun onBackground ()V
public fun onForeground ()V
public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion;

import android.os.Debug;
import android.os.Handler;
import android.os.Looper;
import android.os.SystemClock;
Expand Down Expand Up @@ -211,6 +212,15 @@ public void run() {

@ApiStatus.Internal
protected void checkMainThread(final @NotNull Thread mainThread) throws IOException {
if (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;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: Before returning, should we also call clearStacks() if we've collected any?

}

final long now = SystemClock.uptimeMillis();
final long diff = now - lastMainThreadExecutionTime;

Expand Down Expand Up @@ -268,6 +278,11 @@ protected void checkMainThread(final @NotNull Thread mainThread) throws IOExcept
}
}

@ApiStatus.Internal
protected boolean isDebuggerConnected() {
return Debug.isDebuggerConnected();
Comment thread
markushi marked this conversation as resolved.
Resolved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

separately, it seems to me this is protected only so that we can override this for testing. if that's the case, can we instead wrap Debug.isDebuggerConnected() and pass this object in to our constructor to make the tests more sane?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah exactly, my clanker came up with this, I guess in order to avoid any breaking API changes in the ctor. First I thought it's elegant, as it saves the extra interface definitions and constructor overloads. But the more I think about it the more disadvantages I see, let me change that!

}

@TestOnly
@NotNull
protected MainThreadState getState() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,37 @@ 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)

val androidOptions =
SentryAndroidOptions().apply {
cacheDirPath = tmpDir.root.absolutePath
setLogger(mockLogger)
anrProfilingSampleRate = 1.0
}

var debuggerConnected = true
val integration =
object : AnrProfilingIntegration() {
override fun isDebuggerConnected(): Boolean = debuggerConnected
}
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()
Expand Down
Loading