diff options
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 */); + } } |