Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

fix: before raising ANR event, we check ProcessErrorStateInfo if available #412

Merged
merged 6 commits into from
May 16, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@
// Based on the class above. The API unnecessary here was removed.
package io.sentry.android.core;

import static android.app.ActivityManager.ProcessErrorStateInfo.NOT_RESPONDING;

import android.app.ActivityManager;
import android.content.Context;
import android.os.Debug;
import io.sentry.core.ILogger;
import io.sentry.core.SentryLevel;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.TestOnly;
Expand All @@ -19,21 +25,24 @@ final class ANRWatchDog extends Thread {
private final long timeoutIntervalMillis;
private final @NotNull ILogger logger;
private AtomicLong tick = new AtomicLong(0);
private volatile boolean reported = false;
private AtomicBoolean reported = new AtomicBoolean(false);

private final @NotNull Context context;

@SuppressWarnings("UnnecessaryLambda")
private final Runnable ticker =
() -> {
tick = new AtomicLong(0);
reported = false;
reported.set(false);
};

ANRWatchDog(
long timeoutIntervalMillis,
boolean reportInDebug,
@NotNull ANRListener listener,
@NotNull ILogger logger) {
this(timeoutIntervalMillis, reportInDebug, listener, logger, new MainLooperHandler());
@NotNull ILogger logger,
final @NotNull Context context) {
this(timeoutIntervalMillis, reportInDebug, listener, logger, new MainLooperHandler(), context);
}

@TestOnly
Expand All @@ -42,16 +51,17 @@ final class ANRWatchDog extends Thread {
boolean reportInDebug,
@NotNull ANRListener listener,
@NotNull ILogger logger,
@NotNull IHandler uiHandler) {
@NotNull IHandler uiHandler,
final @NotNull Context context) {
super();
this.reportInDebug = reportInDebug;
this.anrListener = listener;
this.timeoutIntervalMillis = timeoutIntervalMillis;
this.logger = logger;
this.uiHandler = uiHandler;
this.context = context;
}

@SuppressWarnings("NonAtomicOperationOnVolatileField")
@Override
public void run() {
setName("|ANR-WatchDog|");
Expand All @@ -73,16 +83,39 @@ public void run() {
}

// If the main thread has not handled ticker, it is blocked. ANR.
if (tick.get() != 0 && !reported) {
//noinspection ConstantConditions
if (tick.get() != 0 && !reported.get()) {
if (!reportInDebug && (Debug.isDebuggerConnected() || Debug.waitingForDebugger())) {
logger.log(
SentryLevel.DEBUG,
"An ANR was detected but ignored because the debugger is connected.");
reported = true;
reported.set(true);
continue;
}

// we only raise an ANR event if the process is in ANR state.
// if ActivityManager is not available, we'll still be able to send ANRs
final ActivityManager am =
(ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE);

if (am != null) {
final List<ActivityManager.ProcessErrorStateInfo> processesInErrorState =
am.getProcessesInErrorState();
// if list is null, there's no process in ANR state.
if (processesInErrorState == null) {
continue;
}
boolean isAnr = false;
for (ActivityManager.ProcessErrorStateInfo item : processesInErrorState) {
if (item.condition == NOT_RESPONDING) {
Copy link
Member

Choose a reason for hiding this comment

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

So this is a best effort since it could give false positive? Meaning there's some process in ANR state but this isn't the current one? Either way would improve a lot already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's only the processes of this app, so that's why it's a list. an App can have multiple processes but its not common at all.

isAnr = true;
break;
}
}
if (!isAnr) {
continue;
}
}

logger.log(SentryLevel.INFO, "Raising ANR");
final String message =
"Application Not Responding for at least " + timeoutIntervalMillis + " ms.";
Expand All @@ -91,7 +124,8 @@ public void run() {
new ApplicationNotResponding(message, uiHandler.getThread());
anrListener.onAppNotResponding(error);
interval = timeoutIntervalMillis;
reported = true;

reported.set(true);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ private static void installDefaultIntegrations(
}
}));

options.addIntegration(new AnrIntegration());
options.addIntegration(new AnrIntegration(context));
options.addIntegration(new AppLifecycleIntegration());

// registerActivityLifecycleCallbacks is only available if Context is an AppContext
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.sentry.android.core;

import android.content.Context;
import io.sentry.core.IHub;
import io.sentry.core.ILogger;
import io.sentry.core.Integration;
Expand All @@ -20,6 +21,12 @@
*/
public final class AnrIntegration implements Integration, Closeable {

private final @NotNull Context context;

public AnrIntegration(final @NotNull Context context) {
this.context = context;
}

/**
* Responsible for watching UI thread. being static to avoid multiple instances running at the
* same time.
Expand Down Expand Up @@ -56,7 +63,8 @@ private void register(final @NotNull IHub hub, final @NotNull SentryAndroidOptio
options.getAnrTimeoutIntervalMillis(),
options.isAnrReportInDebug(),
error -> reportANR(hub, options.getLogger(), error),
options.getLogger());
options.getLogger(),
context);
anrWatchDog.start();

options.getLogger().log(SentryLevel.DEBUG, "AnrIntegration installed.");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
package io.sentry.android.core

import android.app.ActivityManager
import android.app.ActivityManager.ProcessErrorStateInfo.NOT_RESPONDING
import android.app.ActivityManager.ProcessErrorStateInfo.NO_ERROR
import android.content.Context
import com.nhaarman.mockitokotlin2.any
import com.nhaarman.mockitokotlin2.eq
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.whenever
import io.sentry.android.core.ANRWatchDog.ANRListener
Expand All @@ -27,7 +32,7 @@ class ANRWatchDogTest {
whenever(handler.post(any())).then { latch.countDown() }
whenever(handler.thread).thenReturn(thread)
val interval = 1L
val sut = ANRWatchDog(interval, true, ANRListener { a -> anr = a }, mock(), handler)
val sut = ANRWatchDog(interval, true, ANRListener { a -> anr = a }, mock(), handler, mock())
val es = Executors.newSingleThreadExecutor()
try {
es.submit { sut.run() }
Expand Down Expand Up @@ -59,7 +64,7 @@ class ANRWatchDogTest {
}
whenever(handler.thread).thenReturn(thread)
val interval = 1L
val sut = ANRWatchDog(interval, true, ANRListener { a -> anr = a }, mock(), handler)
val sut = ANRWatchDog(interval, true, ANRListener { a -> anr = a }, mock(), handler, mock())
val es = Executors.newSingleThreadExecutor()
try {
es.submit { sut.run() }
Expand All @@ -76,4 +81,83 @@ class ANRWatchDogTest {
es.shutdown()
}
}

@Test
fun `when ANR is detected and ActivityManager has ANR process, callback not invoked`() {
var anr: ApplicationNotResponding? = null
val handler = mock<IHandler>()
val thread = mock<Thread>()
val expectedState = Thread.State.BLOCKED
val stacktrace = StackTraceElement("class", "method", "fileName", 10)
whenever(thread.state).thenReturn(expectedState)
whenever(thread.stackTrace).thenReturn(arrayOf(stacktrace))
val latch = CountDownLatch(1)
whenever(handler.post(any())).then { latch.countDown() }
whenever(handler.thread).thenReturn(thread)
val interval = 1L
val context = mock<Context>()
val am = mock<ActivityManager>()

whenever(context.getSystemService(eq(Context.ACTIVITY_SERVICE))).thenReturn(am)
val stateInfo = ActivityManager.ProcessErrorStateInfo()
stateInfo.condition = NOT_RESPONDING
val anrs = listOf(stateInfo)
whenever(am.processesInErrorState).thenReturn(anrs)
val sut = ANRWatchDog(interval, true, ANRListener { a -> anr = a }, mock(), handler, context)
val es = Executors.newSingleThreadExecutor()
try {
es.submit { sut.run() }

assertTrue(latch.await(10L, TimeUnit.SECONDS)) // Wait until worker posts the job for the "UI thread"
var waitCount = 0
do {
Thread.sleep(100) // Let worker realize this is ANR
} while (anr == null && waitCount++ < 100)

assertNotNull(anr)
assertEquals(expectedState, anr!!.thread.state)
assertEquals(stacktrace.className, anr!!.stackTrace[0].className)
} finally {
sut.interrupt()
es.shutdown()
}
}

@Test
fun `when ANR is detected and ActivityManager has not ANR process, callback is not invoked`() {
var anr: ApplicationNotResponding? = null
val handler = mock<IHandler>()
val thread = mock<Thread>()
val expectedState = Thread.State.BLOCKED
val stacktrace = StackTraceElement("class", "method", "fileName", 10)
whenever(thread.state).thenReturn(expectedState)
whenever(thread.stackTrace).thenReturn(arrayOf(stacktrace))
val latch = CountDownLatch(1)
whenever(handler.post(any())).then { latch.countDown() }
whenever(handler.thread).thenReturn(thread)
val interval = 1L
val context = mock<Context>()
val am = mock<ActivityManager>()

whenever(context.getSystemService(eq(Context.ACTIVITY_SERVICE))).thenReturn(am)
val stateInfo = ActivityManager.ProcessErrorStateInfo()
stateInfo.condition = NO_ERROR
val anrs = listOf(stateInfo)
whenever(am.processesInErrorState).thenReturn(anrs)
val sut = ANRWatchDog(interval, true, ANRListener { a -> anr = a }, mock(), handler, context)
val es = Executors.newSingleThreadExecutor()
try {
es.submit { sut.run() }

assertTrue(latch.await(10L, TimeUnit.SECONDS)) // Wait until worker posts the job for the "UI thread"
var waitCount = 0
do {
Thread.sleep(100) // Let worker realize this is ANR
} while (anr == null && waitCount++ < 100)
assertNull(anr) // callback never ran
} finally {
sut.interrupt()
es.shutdown()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import kotlin.test.assertTrue

class AnrIntegrationTest {

private val integration = AnrIntegration()
private val integration = AnrIntegration(mock())

@BeforeTest
fun `before each test`() {
Expand All @@ -34,15 +34,15 @@ class AnrIntegrationTest {
val options = SentryAndroidOptions()
options.isAnrEnabled = false
val hub = mock<IHub>()
val integration = AnrIntegration()
val integration = AnrIntegration(mock())
integration.register(hub, options)
assertNull(integration.anrWatchDog)
}

@Test
fun `When ANR watch dog is triggered, it should capture exception`() {
val hub = mock<IHub>()
val integration = AnrIntegration()
val integration = AnrIntegration(mock())
integration.reportANR(hub, mock(), mock())
verify(hub).captureException(any())
}
Expand All @@ -51,7 +51,7 @@ class AnrIntegrationTest {
fun `When ANR integration is closed, watch dog should stop`() {
val options = SentryAndroidOptions()
val hub = mock<IHub>()
val integration = AnrIntegration()
val integration = AnrIntegration(mock())
integration.register(hub, options)
assertNotNull(integration.anrWatchDog)
integration.close()
Expand Down