diff options
| author | 2020-03-26 20:22:22 +0800 | |
|---|---|---|
| committer | 2020-03-31 12:08:45 +0800 | |
| commit | f9a2c04f2ba965815a4a9fe8bfec6f19bd8ab2e7 (patch) | |
| tree | 21bae443e0c3ca5f86664974650bf12b6e263a77 | |
| parent | 8927986f3bcbf3c64e1f3cdbc6032e90814daa75 (diff) | |
Handle ANR in a separate thread
So the ActivityManager and InputDispatcher thread won't be delayed,
e.g. process next broadcast, input event.
The thread handles ANR sequentially and is only alive if there is ANR.
If system is very slow to handle ANR which has delayed over 1 minute,
only the traces of no response process will be dumped to reduce load.
Bug: 143573504
Test: atest FrameworksServicesTests:AnrHelperTest
FrameworksServicesTests:ProcessRecordTests
CtsAppTestCases:ActivityManagerTest#testAppNotResponding
Change-Id: I892ea60665f072bf7673f7af96f5f1a734aa540c
8 files changed, 268 insertions, 52 deletions
diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index 714aae1b70ca..7523710dde9f 100644 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -4213,7 +4213,7 @@ public final class ActiveServices { } if (anrMessage != null) { - proc.appNotResponding(null, null, null, null, false, anrMessage); + mAm.mAnrHelper.appNotResponding(proc, anrMessage); } } @@ -4238,7 +4238,7 @@ public final class ActiveServices { } if (app != null) { - app.appNotResponding(null, null, null, null, false, + mAm.mAnrHelper.appNotResponding(app, "Context.startForegroundService() did not then call Service.startForeground(): " + r); } diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 73d6fff37ac1..1f65f1b1571e 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -1522,6 +1522,8 @@ public class ActivityManagerService extends IActivityManager.Stub void onOomAdjMessage(String msg); } + final AnrHelper mAnrHelper = new AnrHelper(); + /** * Runtime CPU use collection thread. This object's lock is used to * perform synchronization with the thread (notifying it to run). @@ -7797,13 +7799,7 @@ public class ActivityManagerService extends IActivityManager.Stub return; } - mHandler.post(new Runnable() { - @Override - public void run() { - host.appNotResponding( - null, null, null, null, false, "ContentProvider not responding"); - } - }); + mAnrHelper.appNotResponding(host, "ContentProvider not responding"); } @Override @@ -7816,13 +7812,8 @@ public class ActivityManagerService extends IActivityManager.Stub throw new SecurityException("Unknown process: " + callingPid); } - mHandler.post(new Runnable() { - @Override - public void run() { - app.appNotResponding( - null, app.info, null, null, false, "App requested: " + reason); - } - }); + mAnrHelper.appNotResponding(app, null, app.info, null, null, false, + "App requested: " + reason); } } @@ -19267,10 +19258,7 @@ public class ActivityManagerService extends IActivityManager.Stub @Override public long inputDispatchingTimedOut(int pid, boolean aboveSystem, String reason) { - synchronized (ActivityManagerService.this) { - return ActivityManagerService.this.inputDispatchingTimedOut( - pid, aboveSystem, reason); - } + return ActivityManagerService.this.inputDispatchingTimedOut(pid, aboveSystem, reason); } @Override @@ -19561,7 +19549,7 @@ public class ActivityManagerService extends IActivityManager.Stub return true; } } - proc.appNotResponding(activityShortComponentName, aInfo, + mAnrHelper.appNotResponding(proc, activityShortComponentName, aInfo, parentShortComponentName, parentProcess, aboveSystem, annotation); } diff --git a/services/core/java/com/android/server/am/AnrHelper.java b/services/core/java/com/android/server/am/AnrHelper.java new file mode 100644 index 000000000000..bc455338fc52 --- /dev/null +++ b/services/core/java/com/android/server/am/AnrHelper.java @@ -0,0 +1,142 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.am; + +import static com.android.server.am.ActivityManagerDebugConfig.TAG_AM; +import static com.android.server.am.ActivityManagerDebugConfig.TAG_WITH_CLASS_NAME; + +import android.content.pm.ApplicationInfo; +import android.os.SystemClock; +import android.util.Slog; + +import com.android.internal.annotations.GuardedBy; +import com.android.server.wm.WindowProcessController; + +import java.util.ArrayList; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * The helper class to handle no response process. An independent thread will be created on demand + * so the caller can report the ANR without worrying about taking long time. + */ +class AnrHelper { + private static final String TAG = TAG_WITH_CLASS_NAME ? "AnrHelper" : TAG_AM; + + /** + * If the system is extremely slow somehow that the ANR has been pending too long for more than + * this time, the information might be outdated. So we only the dump the unresponsive process + * instead of including other processes to avoid making the system more busy. + */ + private static final long EXPIRED_REPORT_TIME_MS = TimeUnit.MINUTES.toMillis(1); + + @GuardedBy("mAnrRecords") + private final ArrayList<AnrRecord> mAnrRecords = new ArrayList<>(); + private final AtomicBoolean mRunning = new AtomicBoolean(false); + + void appNotResponding(ProcessRecord anrProcess, String annotation) { + appNotResponding(anrProcess, null /* activityShortComponentName */, null /* aInfo */, + null /* parentShortComponentName */, null /* parentProcess */, + false /* aboveSystem */, annotation); + } + + void appNotResponding(ProcessRecord anrProcess, String activityShortComponentName, + ApplicationInfo aInfo, String parentShortComponentName, + WindowProcessController parentProcess, boolean aboveSystem, String annotation) { + synchronized (mAnrRecords) { + mAnrRecords.add(new AnrRecord(anrProcess, activityShortComponentName, aInfo, + parentShortComponentName, parentProcess, aboveSystem, annotation)); + } + startAnrConsumerIfNeeded(); + } + + private void startAnrConsumerIfNeeded() { + if (mRunning.compareAndSet(false, true)) { + new AnrConsumerThread().start(); + } + } + + /** + * The thread to execute {@link ProcessRecord#appNotResponding}. It will terminate if all + * records are handled. + */ + private class AnrConsumerThread extends Thread { + AnrConsumerThread() { + super("AnrConsumer"); + } + + private AnrRecord next() { + synchronized (mAnrRecords) { + return mAnrRecords.isEmpty() ? null : mAnrRecords.remove(0); + } + } + + @Override + public void run() { + AnrRecord r; + while ((r = next()) != null) { + 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. + final long reportLatency = startTime - r.mTimestamp; + final boolean onlyDumpSelf = reportLatency > EXPIRED_REPORT_TIME_MS; + r.appNotResponding(onlyDumpSelf); + final long endTime = SystemClock.uptimeMillis(); + Slog.d(TAG, "Completed ANR of " + r.mApp.processName + " in " + + (endTime - startTime) + "ms, latency " + reportLatency + + (onlyDumpSelf ? "ms" : "ms (expired, only dump ANR app)")); + } + + mRunning.set(false); + synchronized (mAnrRecords) { + // The race should be unlikely to happen. Just to make sure we don't miss. + if (!mAnrRecords.isEmpty()) { + startAnrConsumerIfNeeded(); + } + } + } + } + + private static class AnrRecord { + final ProcessRecord mApp; + final String mActivityShortComponentName; + final String mParentShortComponentName; + final String mAnnotation; + final ApplicationInfo mAppInfo; + final WindowProcessController mParentProcess; + final boolean mAboveSystem; + final long mTimestamp = SystemClock.uptimeMillis(); + + AnrRecord(ProcessRecord anrProcess, String activityShortComponentName, + ApplicationInfo aInfo, String parentShortComponentName, + WindowProcessController parentProcess, boolean aboveSystem, String annotation) { + mApp = anrProcess; + mActivityShortComponentName = activityShortComponentName; + mParentShortComponentName = parentShortComponentName; + mAnnotation = annotation; + mAppInfo = aInfo; + mParentProcess = parentProcess; + mAboveSystem = aboveSystem; + } + + void appNotResponding(boolean onlyDumpSelf) { + mApp.appNotResponding(mActivityShortComponentName, mAppInfo, + mParentShortComponentName, mParentProcess, mAboveSystem, mAnnotation, + onlyDumpSelf); + } + } +} diff --git a/services/core/java/com/android/server/am/BroadcastQueue.java b/services/core/java/com/android/server/am/BroadcastQueue.java index 3aec53a44058..1cc41b22838e 100644 --- a/services/core/java/com/android/server/am/BroadcastQueue.java +++ b/services/core/java/com/android/server/am/BroadcastQueue.java @@ -198,21 +198,6 @@ public final class BroadcastQueue { } } - private final class AppNotResponding implements Runnable { - private final ProcessRecord mApp; - private final String mAnnotation; - - public AppNotResponding(ProcessRecord app, String annotation) { - mApp = app; - mAnnotation = annotation; - } - - @Override - public void run() { - mApp.appNotResponding(null, null, null, null, false, mAnnotation); - } - } - BroadcastQueue(ActivityManagerService service, Handler handler, String name, BroadcastConstants constants, boolean allowDelayBehindServices) { mService = service; @@ -1808,9 +1793,7 @@ public final class BroadcastQueue { scheduleBroadcastsLocked(); if (!debugging && anrMessage != null) { - // Post the ANR to the handler since we do not want to process ANRs while - // potentially holding our lock. - mHandler.post(new AppNotResponding(app, anrMessage)); + mService.mAnrHelper.appNotResponding(app, anrMessage); } } diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java index 68bb8f5763e7..61ebc361b6af 100644 --- a/services/core/java/com/android/server/am/ProcessRecord.java +++ b/services/core/java/com/android/server/am/ProcessRecord.java @@ -1502,7 +1502,7 @@ class ProcessRecord implements WindowProcessListener { void appNotResponding(String activityShortComponentName, ApplicationInfo aInfo, String parentShortComponentName, WindowProcessController parentProcess, - boolean aboveSystem, String annotation) { + boolean aboveSystem, String annotation, boolean onlyDumpSelf) { ArrayList<Integer> firstPids = new ArrayList<>(5); SparseArray<Boolean> lastPids = new SparseArray<>(20); @@ -1514,6 +1514,7 @@ class ProcessRecord implements WindowProcessListener { mService.updateCpuStatsNow(); } + final boolean isSilentAnr; synchronized (mService) { // PowerManager.reboot() can block for a long time, so ignore ANRs while shutting down. if (mService.mAtmInternal.isShuttingDown()) { @@ -1544,8 +1545,9 @@ class ProcessRecord implements WindowProcessListener { // Dump thread traces as quickly as we can, starting with "interesting" processes. firstPids.add(pid); - // Don't dump other PIDs if it's a background ANR - if (!isSilentAnr()) { + // Don't dump other PIDs if it's a background ANR or is requested to only dump self. + isSilentAnr = isSilentAnr(); + if (!isSilentAnr && !onlyDumpSelf) { int parentPid = pid; if (parentProcess != null && parentProcess.getPid() > 0) { parentPid = parentProcess.getPid(); @@ -1598,7 +1600,7 @@ class ProcessRecord implements WindowProcessListener { // don't dump native PIDs for background ANRs unless it is the process of interest String[] nativeProcs = null; - if (isSilentAnr()) { + if (isSilentAnr || onlyDumpSelf) { for (int i = 0; i < NATIVE_STACKS_OF_INTEREST.length; i++) { if (NATIVE_STACKS_OF_INTEREST[i].equals(processName)) { nativeProcs = new String[] { processName }; @@ -1625,7 +1627,7 @@ class ProcessRecord implements WindowProcessListener { // To hold the start and end offset to the ANR trace file respectively. final long[] offsets = new long[2]; File tracesFile = ActivityManagerService.dumpStackTraces(firstPids, - (isSilentAnr()) ? null : processCpuTracker, (isSilentAnr()) ? null : lastPids, + isSilentAnr ? null : processCpuTracker, isSilentAnr ? null : lastPids, nativePids, tracesFileException, offsets); if (isMonitorCpuUsage()) { diff --git a/services/core/java/com/android/server/wm/InputManagerCallback.java b/services/core/java/com/android/server/wm/InputManagerCallback.java index deaec0ac75ae..9c4ac890fed8 100644 --- a/services/core/java/com/android/server/wm/InputManagerCallback.java +++ b/services/core/java/com/android/server/wm/InputManagerCallback.java @@ -172,8 +172,20 @@ final class InputManagerCallback implements InputManagerService.WindowManagerCal * Called by the InputManager. */ @Override - public long notifyANR(InputApplicationHandle inputApplicationHandle, - IBinder token, String reason) { + public long notifyANR(InputApplicationHandle inputApplicationHandle, IBinder token, + String reason) { + final long startTime = SystemClock.uptimeMillis(); + try { + return notifyANRInner(inputApplicationHandle, token, reason); + } finally { + // Log the time because the method is called from InputDispatcher thread. It shouldn't + // take too long that may affect input response time. + Slog.d(TAG_WM, "notifyANR took " + (SystemClock.uptimeMillis() - startTime) + "ms"); + } + } + + private long notifyANRInner(InputApplicationHandle inputApplicationHandle, IBinder token, + String reason) { ActivityRecord activity = null; WindowState windowState = null; boolean aboveSystem = false; diff --git a/services/tests/servicestests/src/com/android/server/am/AnrHelperTest.java b/services/tests/servicestests/src/com/android/server/am/AnrHelperTest.java new file mode 100644 index 000000000000..13f63b3a52bf --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/am/AnrHelperTest.java @@ -0,0 +1,69 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.am; + +import static android.testing.DexmakerShareClassLoaderRule.runWithDexmakerShareClassLoader; + +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; + +import android.content.pm.ApplicationInfo; +import android.platform.test.annotations.Presubmit; + +import androidx.test.filters.SmallTest; + +import com.android.server.wm.WindowProcessController; + +import org.junit.Before; +import org.junit.Test; + +import java.util.concurrent.TimeUnit; + +/** + * Build/Install/Run: + * atest FrameworksServicesTests:AnrHelperTest + */ +@SmallTest +@Presubmit +public class AnrHelperTest { + private final AnrHelper mAnrHelper = new AnrHelper(); + + private ProcessRecord mAnrApp; + + @Before + public void setUp() { + runWithDexmakerShareClassLoader(() -> mAnrApp = mock(ProcessRecord.class)); + } + + @Test + public void testHandleAppNotResponding() { + final String activityShortComponentName = "pkg.test/.A"; + final String parentShortComponentName = "pkg.test/.P"; + final ApplicationInfo appInfo = new ApplicationInfo(); + final WindowProcessController parentProcess = mock(WindowProcessController.class); + final boolean aboveSystem = false; + final String annotation = "test"; + mAnrHelper.appNotResponding(mAnrApp, activityShortComponentName, appInfo, + parentShortComponentName, parentProcess, aboveSystem, annotation); + + verify(mAnrApp, timeout(TimeUnit.SECONDS.toMillis(5))).appNotResponding( + eq(activityShortComponentName), eq(appInfo), eq(parentShortComponentName), + eq(parentProcess), eq(aboveSystem), eq(annotation), eq(false) /* onlyDumpSelf */); + } +} diff --git a/services/tests/servicestests/src/com/android/server/am/ProcessRecordTests.java b/services/tests/servicestests/src/com/android/server/am/ProcessRecordTests.java index 5b85980d3f1c..ded14b89d01c 100644 --- a/services/tests/servicestests/src/com/android/server/am/ProcessRecordTests.java +++ b/services/tests/servicestests/src/com/android/server/am/ProcessRecordTests.java @@ -27,13 +27,17 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; +import android.content.ComponentName; import android.content.Context; +import android.content.pm.PackageManagerInternal; import android.platform.test.annotations.Presubmit; import androidx.test.filters.FlakyTest; +import com.android.server.LocalServices; import com.android.server.wm.ActivityTaskManagerService; +import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -64,6 +68,17 @@ public class ProcessRecordTests { sService.mActivityTaskManager.initialize(null, null, sContext.getMainLooper()); sService.mAtmInternal = sService.mActivityTaskManager.getAtmInternal(); }); + + // Avoid NPE when initializing {@link ProcessRecord#mWindowProcessController}. + final PackageManagerInternal packageManagerInternal = mock(PackageManagerInternal.class); + LocalServices.addService(PackageManagerInternal.class, packageManagerInternal); + final ComponentName sysUiName = new ComponentName(sContext.getPackageName(), "test"); + doReturn(sysUiName).when(packageManagerInternal).getSystemUiServiceComponent(); + } + + @AfterClass + public static void tearDownOnce() { + LocalServices.removeServiceForTest(PackageManagerInternal.class); } @Before @@ -99,7 +114,7 @@ public class ProcessRecordTests { public void testAnrWhenCrash() { mProcessRecord.setCrashing(true); assertTrue(mProcessRecord.isCrashing()); - mProcessRecord.appNotResponding(null, null, null, null, false, "Test ANR when crash"); + appNotResponding(mProcessRecord, "Test ANR when crash"); assertFalse(mProcessRecord.isNotResponding()); assertFalse(mProcessRecord.killedByAm); assertFalse(mProcessRecord.killed); @@ -111,8 +126,7 @@ public class ProcessRecordTests { @Test public void testAnrWhenKilledByAm() { mProcessRecord.killedByAm = true; - mProcessRecord.appNotResponding(null, null, null, null, false, - "Test ANR when killed by AM"); + appNotResponding(mProcessRecord, "Test ANR when killed by AM"); assertFalse(mProcessRecord.isNotResponding()); assertFalse(mProcessRecord.isCrashing()); assertFalse(mProcessRecord.killed); @@ -124,7 +138,7 @@ public class ProcessRecordTests { @Test public void testAnrWhenKilled() { mProcessRecord.killed = true; - mProcessRecord.appNotResponding(null, null, null, null, false, "Test ANR when killed"); + appNotResponding(mProcessRecord, "Test ANR when killed"); assertFalse(mProcessRecord.isNotResponding()); assertFalse(mProcessRecord.isCrashing()); assertFalse(mProcessRecord.killedByAm); @@ -136,7 +150,7 @@ public class ProcessRecordTests { */ @Test public void testNonSilentAnr() { - mProcessRecord.appNotResponding(null, null, null, null, false, "Test non-silent ANR"); + appNotResponding(mProcessRecord, "Test non-silent ANR"); assertTrue(mProcessRecord.isNotResponding()); assertFalse(mProcessRecord.isCrashing()); assertFalse(mProcessRecord.killedByAm); @@ -151,10 +165,16 @@ public class ProcessRecordTests { public void testSilentAnr() { // Silent Anr will run through even without a parent process, and directly killed by AM. doReturn(true).when(mProcessRecord).isSilentAnr(); - mProcessRecord.appNotResponding(null, null, null, null, false, "Test silent ANR"); + appNotResponding(mProcessRecord, "Test silent ANR"); assertTrue(mProcessRecord.isNotResponding()); assertFalse(mProcessRecord.isCrashing()); assertTrue(mProcessRecord.killedByAm); assertTrue(mProcessRecord.killed); } + + private static void appNotResponding(ProcessRecord processRecord, String annotation) { + processRecord.appNotResponding(null /* activityShortComponentName */, null /* aInfo */, + null /* parentShortComponentName */, null /* parentProcess */, + false /* aboveSystem */, annotation, false /* onlyDumpSelf */); + } } |