diff --git a/services/core/java/com/android/server/am/AnrHelper.java b/services/core/java/com/android/server/am/AnrHelper.java index 34d9a60d78aae..1e1024e863142 100644 --- a/services/core/java/com/android/server/am/AnrHelper.java +++ b/services/core/java/com/android/server/am/AnrHelper.java @@ -60,6 +60,10 @@ class AnrHelper { */ private long mLastAnrTimeMs = 0L; + /** The pid which is running appNotResponding(). */ + @GuardedBy("mAnrRecords") + private int mProcessingPid = -1; + AnrHelper(final ActivityManagerService service) { mService = service; } @@ -73,7 +77,18 @@ class AnrHelper { void appNotResponding(ProcessRecord anrProcess, String activityShortComponentName, ApplicationInfo aInfo, String parentShortComponentName, WindowProcessController parentProcess, boolean aboveSystem, String annotation) { + final int incomingPid = anrProcess.mPid; synchronized (mAnrRecords) { + if (mProcessingPid == incomingPid) { + Slog.i(TAG, "Skip duplicated ANR, pid=" + incomingPid + " " + annotation); + return; + } + for (int i = mAnrRecords.size() - 1; i >= 0; i--) { + if (mAnrRecords.get(i).mPid == incomingPid) { + Slog.i(TAG, "Skip queued ANR, pid=" + incomingPid + " " + annotation); + return; + } + } mAnrRecords.add(new AnrRecord(anrProcess, activityShortComponentName, aInfo, parentShortComponentName, parentProcess, aboveSystem, annotation)); } @@ -87,8 +102,8 @@ class AnrHelper { } /** - * The thread to execute {@link ProcessRecord#appNotResponding}. It will terminate if all - * records are handled. + * The thread to execute {@link ProcessErrorStateRecord#appNotResponding}. It will terminate if + * all records are handled. */ private class AnrConsumerThread extends Thread { AnrConsumerThread() { @@ -97,7 +112,12 @@ class AnrHelper { private AnrRecord next() { synchronized (mAnrRecords) { - return mAnrRecords.isEmpty() ? null : mAnrRecords.remove(0); + if (mAnrRecords.isEmpty()) { + return null; + } + final AnrRecord record = mAnrRecords.remove(0); + mProcessingPid = record.mPid; + return record; } } @@ -106,6 +126,13 @@ class AnrHelper { AnrRecord r; while ((r = next()) != null) { scheduleBinderHeavyHitterAutoSamplerIfNecessary(); + final int currentPid = r.mApp.mPid; + if (currentPid != r.mPid) { + // The process may have restarted or died. + Slog.i(TAG, "Skip ANR with mismatched pid=" + r.mPid + ", current pid=" + + currentPid); + continue; + } final long startTime = SystemClock.uptimeMillis(); // If there are many ANR at the same time, the latency may be larger. If the latency // is too large, the stack trace might not be meaningful. @@ -120,6 +147,7 @@ class AnrHelper { mRunning.set(false); synchronized (mAnrRecords) { + mProcessingPid = -1; // The race should be unlikely to happen. Just to make sure we don't miss. if (!mAnrRecords.isEmpty()) { startAnrConsumerIfNeeded(); @@ -139,6 +167,7 @@ class AnrHelper { private static class AnrRecord { final ProcessRecord mApp; + final int mPid; final String mActivityShortComponentName; final String mParentShortComponentName; final String mAnnotation; @@ -151,6 +180,7 @@ class AnrHelper { ApplicationInfo aInfo, String parentShortComponentName, WindowProcessController parentProcess, boolean aboveSystem, String annotation) { mApp = anrProcess; + mPid = anrProcess.mPid; mActivityShortComponentName = activityShortComponentName; mParentShortComponentName = parentShortComponentName; mAnnotation = annotation; diff --git a/services/tests/servicestests/src/com/android/server/am/AnrHelperTest.java b/services/tests/servicestests/src/com/android/server/am/AnrHelperTest.java index 877538cdda6f8..782d519418092 100644 --- a/services/tests/servicestests/src/com/android/server/am/AnrHelperTest.java +++ b/services/tests/servicestests/src/com/android/server/am/AnrHelperTest.java @@ -20,7 +20,12 @@ import static android.testing.DexmakerShareClassLoaderRule.runWithDexmakerShareC import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.verify; @@ -42,6 +47,7 @@ import org.junit.Test; import java.io.File; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; /** @@ -51,6 +57,7 @@ import java.util.concurrent.TimeUnit; @SmallTest @Presubmit public class AnrHelperTest { + private static final long TIMEOUT_MS = TimeUnit.SECONDS.toMillis(5); private AnrHelper mAnrHelper; private ProcessRecord mAnrApp; @@ -106,8 +113,42 @@ public class AnrHelperTest { mAnrHelper.appNotResponding(mAnrApp, activityShortComponentName, appInfo, parentShortComponentName, parentProcess, aboveSystem, annotation); - verify(mAnrApp.mErrorState, timeout(TimeUnit.SECONDS.toMillis(5))).appNotResponding( + verify(mAnrApp.mErrorState, timeout(TIMEOUT_MS)).appNotResponding( eq(activityShortComponentName), eq(appInfo), eq(parentShortComponentName), eq(parentProcess), eq(aboveSystem), eq(annotation), eq(false) /* onlyDumpSelf */); } + + @Test + public void testSkipDuplicatedAnr() { + final CountDownLatch consumerLatch = new CountDownLatch(1); + final CountDownLatch processingLatch = new CountDownLatch(1); + doAnswer(invocation -> { + consumerLatch.countDown(); + // Simulate that it is dumping to block the consumer thread. + processingLatch.await(); + return null; + }).when(mAnrApp.mErrorState).appNotResponding(anyString(), any(), any(), any(), + anyBoolean(), anyString(), anyBoolean()); + final ApplicationInfo appInfo = new ApplicationInfo(); + mAnrApp.mPid = 12345; + final Runnable reportAnr = () -> mAnrHelper.appNotResponding(mAnrApp, + "activityShortComponentName", appInfo, "parentShortComponentName", + null /* parentProcess */, false /* aboveSystem */, "annotation"); + reportAnr.run(); + // This should be skipped because the pid is pending in queue. + reportAnr.run(); + // The first reported ANR must be processed. + try { + assertTrue(consumerLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); + } catch (InterruptedException ignored) { + } + // This should be skipped because the pid is under processing. + reportAnr.run(); + + // Assume that the first one finishes after all incoming ANRs. + processingLatch.countDown(); + // There is only one ANR reported. + verify(mAnrApp.mErrorState, timeout(TIMEOUT_MS).only()).appNotResponding( + anyString(), any(), any(), any(), anyBoolean(), anyString(), anyBoolean()); + } }