Merge "Skip duplicated ANR before enqueuing" (dae6f626) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/am/AnrHelper.java

+33 −3

Original line number	Diff line number	Diff line
		@@ -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;

services/tests/servicestests/src/com/android/server/am/AnrHelperTest.java

+42 −1

Original line number	Diff line number	Diff line
		@@ -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());
		}
		}