From 08ab38498984462e2bba4b73b74243fec2fa1c2d Mon Sep 17 00:00:00 2001 From: Hui Yu Date: Tue, 2 Jun 2020 16:22:00 -0700 Subject: [PATCH] Add uid to PendingStartActivityUids when activity resumed. Previously we add uid to PendingStartActivityUids at activity start. Now we also add uid to PendingStartActivityUids at activity resumed because we can not wait for updateOomAdj() to be run by a runnable that WindowManager send to DisplayThread at activity resumed. At ActivityManagerService.getProcessStatesAndOomScoresForPIDs(), if the ProcessRecord is in PendingStartActivityUids, we hard code the ProcessRecord to be PROCESS_STATE_TOP and FOREGROUND_APP_ADJ, ahead of result of updateOomAdj() the comes later. Bug: 155143386, 157180494 Test: Reproduce steps of 155143386, swipe between GCA and Snapchat, camera access is allowed after swipe. Change-Id: Ia11b0e3400e4df851b250beb01dcfda43580668b --- .../android/app/ActivityManagerInternal.java | 12 ++- .../server/am/ActivityManagerService.java | 88 +++++-------------- .../com/android/server/am/OomAdjuster.java | 2 +- .../server/am/PendingStartActivityUids.java | 80 +++++++++++++++++ .../android/server/appop/AppOpsService.java | 19 +--- .../com/android/server/wm/ActivityRecord.java | 3 +- .../com/android/server/wm/ActivityStack.java | 6 +- .../server/wm/WindowProcessController.java | 7 +- 8 files changed, 126 insertions(+), 91 deletions(-) create mode 100644 services/core/java/com/android/server/am/PendingStartActivityUids.java diff --git a/core/java/android/app/ActivityManagerInternal.java b/core/java/android/app/ActivityManagerInternal.java index c491eea7a674a..2ce55219506fd 100644 --- a/core/java/android/app/ActivityManagerInternal.java +++ b/core/java/android/app/ActivityManagerInternal.java @@ -429,11 +429,17 @@ public abstract class ActivityManagerInternal { int userId, int[] appIdWhitelist); /** - * Add or delete uid from the ActivityManagerService PendingStartActivityUids list. + * Add uid to the ActivityManagerService PendingStartActivityUids list. * @param uid uid - * @param pending add to the list if true, delete from list if false. + * @param pid pid of the ProcessRecord that is pending top. */ - public abstract void updatePendingTopUid(int uid, boolean pending); + public abstract void addPendingTopUid(int uid, int pid); + + /** + * Delete uid from the ActivityManagerService PendingStartActivityUids list. + * @param uid uid + */ + public abstract void deletePendingTopUid(int uid); /** * Is the uid in ActivityManagerService PendingStartActivityUids list? diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index caaa8371af53f..933dc99a43f01 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -28,6 +28,7 @@ import static android.app.ActivityManager.INSTR_FLAG_DISABLE_TEST_API_CHECKS; import static android.app.ActivityManager.INSTR_FLAG_MOUNT_EXTERNAL_STORAGE_FULL; import static android.app.ActivityManager.PROCESS_STATE_LAST_ACTIVITY; import static android.app.ActivityManager.PROCESS_STATE_NONEXISTENT; +import static android.app.ActivityManager.PROCESS_STATE_TOP; import static android.app.ActivityManagerInternal.ALLOW_FULL_ONLY; import static android.app.ActivityManagerInternal.ALLOW_NON_FULL; import static android.app.AppOpsManager.OP_NONE; @@ -299,7 +300,6 @@ import android.util.PrintWriterPrinter; import android.util.Slog; import android.util.SparseArray; import android.util.SparseIntArray; -import android.util.SparseLongArray; import android.util.TimeUtils; import android.util.proto.ProtoOutputStream; import android.util.proto.ProtoUtils; @@ -421,7 +421,6 @@ public class ActivityManagerService extends IActivityManager.Stub * Priority we boost main thread and RT of top app to. */ public static final int TOP_APP_PRIORITY_BOOST = -10; - private static final String SYSTEM_PROPERTY_DEVICE_PROVISIONED = "persist.sys.device_provisioned"; @@ -823,45 +822,7 @@ public class ActivityManagerService extends IActivityManager.Stub } } - /** - * While starting activity, WindowManager posts a runnable to DisplayThread to updateOomAdj. - * The latency of the thread switch could cause client app failure when the app is checking - * {@link #isUidActive} before updateOomAdj is done. - * - * Use PendingStartActivityUids to save uid after WindowManager start activity and before - * updateOomAdj is done. - * - *

NOTE: This object is protected by its own lock, NOT the global activity manager lock! - */ - final PendingStartActivityUids mPendingStartActivityUidsLocked = new PendingStartActivityUids(); - final class PendingStartActivityUids { - // Key is uid, value is SystemClock.elapsedRealtime() when the key is added. - private final SparseLongArray mPendingUids = new SparseLongArray(); - - void add(int uid) { - if (mPendingUids.indexOfKey(uid) < 0) { - mPendingUids.put(uid, SystemClock.elapsedRealtime()); - } - } - - void delete(int uid) { - if (mPendingUids.indexOfKey(uid) >= 0) { - long delay = SystemClock.elapsedRealtime() - mPendingUids.get(uid); - if (delay >= 1000) { - Slog.wtf(TAG, - "PendingStartActivityUids startActivity to updateOomAdj delay:" - + delay + "ms," - + " uid:" + uid - + " packageName:" + Settings.getPackageNameForUid(mContext, uid)); - } - mPendingUids.delete(uid); - } - } - - boolean isPendingTopUid(int uid) { - return mPendingUids.indexOfKey(uid) >= 0; - } - } + private final PendingStartActivityUids mPendingStartActivityUids; /** * Puts the process record in the map. @@ -2585,6 +2546,7 @@ public class ActivityManagerService extends IActivityManager.Stub mFactoryTest = FACTORY_TEST_OFF; mUgmInternal = LocalServices.getService(UriGrantsManagerInternal.class); mInternal = new LocalService(); + mPendingStartActivityUids = new PendingStartActivityUids(mContext); } // Note: This method is invoked on the main thread but may need to attach various @@ -2742,6 +2704,7 @@ public class ActivityManagerService extends IActivityManager.Stub } mInternal = new LocalService(); + mPendingStartActivityUids = new PendingStartActivityUids(mContext); } public void setSystemServiceManager(SystemServiceManager mgr) { @@ -6093,9 +6056,18 @@ public class ActivityManagerService extends IActivityManager.Stub synchronized (mPidsSelfLocked) { for (int i = 0; i < pids.length; i++) { ProcessRecord pr = mPidsSelfLocked.get(pids[i]); - states[i] = (pr == null) ? PROCESS_STATE_NONEXISTENT : pr.getCurProcState(); - if (scores != null) { - scores[i] = (pr == null) ? ProcessList.INVALID_ADJ : pr.curAdj; + if (pr != null) { + final boolean isPendingTop = + mPendingStartActivityUids.isPendingTopPid(pr.uid, pids[i]); + states[i] = isPendingTop ? PROCESS_STATE_TOP : pr.getCurProcState(); + if (scores != null) { + scores[i] = isPendingTop ? ProcessList.FOREGROUND_APP_ADJ : pr.curAdj; + } + } else { + states[i] = PROCESS_STATE_NONEXISTENT; + if (scores != null) { + scores[i] = ProcessList.INVALID_ADJ; + } } } } @@ -8839,15 +8811,7 @@ public class ActivityManagerService extends IActivityManager.Stub return true; } } - - if (mInternal.isPendingTopUid(uid)) { - Slog.wtf(TAG, "PendingStartActivityUids isUidActive false but" - + " isPendingTopUid true, uid:" + uid - + " callingPackage:" + callingPackage); - return true; - } else { - return false; - } + return mInternal.isPendingTopUid(uid); } boolean isUidActiveLocked(int uid) { @@ -19731,22 +19695,18 @@ public class ActivityManagerService extends IActivityManager.Stub } @Override - public void updatePendingTopUid(int uid, boolean pending) { - synchronized (mPendingStartActivityUidsLocked) { - if (pending) { - mPendingStartActivityUidsLocked.add(uid); - } else { - mPendingStartActivityUidsLocked.delete(uid); - } - } + public void addPendingTopUid(int uid, int pid) { + mPendingStartActivityUids.add(uid, pid); + } + @Override + public void deletePendingTopUid(int uid) { + mPendingStartActivityUids.delete(uid); } @Override public boolean isPendingTopUid(int uid) { - synchronized (mPendingStartActivityUidsLocked) { - return mPendingStartActivityUidsLocked.isPendingTopUid(uid); - } + return mPendingStartActivityUids.isPendingTopUid(uid); } } diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java index 14c5b2cb12b22..03ff3d0e09781 100644 --- a/services/core/java/com/android/server/am/OomAdjuster.java +++ b/services/core/java/com/android/server/am/OomAdjuster.java @@ -951,7 +951,7 @@ public final class OomAdjuster { mService.mServices.foregroundServiceProcStateChangedLocked(uidRec); } } - mService.mInternal.updatePendingTopUid(uidRec.uid, false); + mService.mInternal.deletePendingTopUid(uidRec.uid); } if (mLocalPowerManager != null) { mLocalPowerManager.finishUidChanges(); diff --git a/services/core/java/com/android/server/am/PendingStartActivityUids.java b/services/core/java/com/android/server/am/PendingStartActivityUids.java new file mode 100644 index 0000000000000..0ed99fedf23d5 --- /dev/null +++ b/services/core/java/com/android/server/am/PendingStartActivityUids.java @@ -0,0 +1,80 @@ +/* + * 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 android.content.Context; +import android.os.SystemClock; +import android.provider.Settings; +import android.util.Pair; +import android.util.Slog; +import android.util.SparseArray; + +/** + * While starting activity, WindowManager posts a runnable to DisplayThread to updateOomAdj. + * The latency of the thread switch could cause client app failure when the app is checking + * {@link ActivityManagerService#isUidActive} before updateOomAdj is done. + * + * Use PendingStartActivityUids to save uid after WindowManager start activity and before + * updateOomAdj is done. + * + *

NOTE: This object is protected by its own lock, NOT the global activity manager lock! + */ +final class PendingStartActivityUids { + static final String TAG = ActivityManagerService.TAG; + + // Key is uid, value is Pair of pid and SystemClock.elapsedRealtime() when the + // uid is added. + private final SparseArray> mPendingUids = new SparseArray(); + private Context mContext; + + PendingStartActivityUids(Context context) { + mContext = context; + } + + synchronized void add(int uid, int pid) { + if (mPendingUids.get(uid) == null) { + mPendingUids.put(uid, new Pair<>(pid, SystemClock.elapsedRealtime())); + } + } + + synchronized void delete(int uid) { + final Pair pendingPid = mPendingUids.get(uid); + if (pendingPid != null) { + final long delay = SystemClock.elapsedRealtime() - pendingPid.second; + if (delay >= 1000 /*ms*/) { + Slog.i(TAG, + "PendingStartActivityUids startActivity to updateOomAdj delay:" + + delay + "ms," + " uid:" + uid + " packageName:" + + Settings.getPackageNameForUid(mContext, uid)); + } + mPendingUids.delete(uid); + } + } + + synchronized boolean isPendingTopPid(int uid, int pid) { + final Pair pendingPid = mPendingUids.get(uid); + if (pendingPid != null) { + return pendingPid.first == pid; + } else { + return false; + } + } + + synchronized boolean isPendingTopUid(int uid) { + return mPendingUids.get(uid) != null; + } +} \ No newline at end of file diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index 37ab1df3fa0c7..1d81245fd5632 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -521,7 +521,6 @@ public class AppOpsService extends IAppOpsService.Stub { public boolean hasForegroundWatchers; public long lastTimeShowDebugToast; - public long lastTimePendingTopUid; public UidState(int uid) { this.uid = uid; @@ -545,7 +544,6 @@ public class AppOpsService extends IAppOpsService.Stub { return MODE_ALLOWED; } else if (mActivityManagerInternal != null && mActivityManagerInternal.isPendingTopUid(uid)) { - maybeLogPendingTopUid(op, mode); return MODE_ALLOWED; } else if (state <= UID_STATE_TOP) { // process is in TOP. @@ -611,7 +609,6 @@ public class AppOpsService extends IAppOpsService.Stub { case OP_CAMERA: if (mActivityManagerInternal != null && mActivityManagerInternal.isPendingTopUid(uid)) { - maybeLogPendingTopUid(op, mode); return MODE_ALLOWED; } else if ((capability & PROCESS_CAPABILITY_FOREGROUND_CAMERA) != 0) { return MODE_ALLOWED; @@ -629,7 +626,6 @@ public class AppOpsService extends IAppOpsService.Stub { case OP_RECORD_AUDIO: if (mActivityManagerInternal != null && mActivityManagerInternal.isPendingTopUid(uid)) { - maybeLogPendingTopUid(op, mode); return MODE_ALLOWED; } else if ((capability & PROCESS_CAPABILITY_FOREGROUND_MICROPHONE) != 0) { return MODE_ALLOWED; @@ -709,7 +705,7 @@ public class AppOpsService extends IAppOpsService.Stub { if (mode == DEBUG_FGS_ALLOW_WHILE_IN_USE && state != UID_STATE_FOREGROUND_SERVICE) { return; } - final long now = System.currentTimeMillis(); + final long now = SystemClock.elapsedRealtime(); if (lastTimeShowDebugToast == 0 || now - lastTimeShowDebugToast > 600000) { lastTimeShowDebugToast = now; mHandler.sendMessage(PooledLambda.obtainMessage( @@ -717,19 +713,6 @@ public class AppOpsService extends IAppOpsService.Stub { mActivityManagerInternal, uid, op, mode)); } } - - - void maybeLogPendingTopUid(int op, int mode) { - final long now = System.currentTimeMillis(); - if (lastTimePendingTopUid == 0 || now - lastTimePendingTopUid > 300000) { - lastTimePendingTopUid = now; - Slog.wtf(TAG, "PendingStartActivityUids evalMode, isPendingTopUid true, uid:" - + uid - + " packageName:" + Settings.getPackageNameForUid(mContext, uid) - + " op:" + op - + " mode:" + mode); - } - } } final static class Ops extends SparseArray { diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index fe2b144bcdd6b..f38a506dd4608 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -2842,7 +2842,8 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A // their client may have activities. // No longer have activities, so update LRU list and oom adj. app.updateProcessInfo(true /* updateServiceConnectionActivities */, - false /* activityChange */, true /* updateOomAdj */); + false /* activityChange */, true /* updateOomAdj */, + false /* addPendingTopUid */); } } diff --git a/services/core/java/com/android/server/wm/ActivityStack.java b/services/core/java/com/android/server/wm/ActivityStack.java index 0d5621dc0e4f8..c99980911cefd 100644 --- a/services/core/java/com/android/server/wm/ActivityStack.java +++ b/services/core/java/com/android/server/wm/ActivityStack.java @@ -1682,7 +1682,8 @@ class ActivityStack extends Task { // happens to be sitting towards the end. if (next.attachedToProcess()) { next.app.updateProcessInfo(false /* updateServiceConnectionActivities */, - true /* activityChange */, false /* updateOomAdj */); + true /* activityChange */, false /* updateOomAdj */, + false /* addPendingTopUid */); } else if (!next.isProcessRunning()) { // Since the start-process is asynchronous, if we already know the process of next // activity isn't running, we can start the process earlier to save the time to wait @@ -1839,7 +1840,8 @@ class ActivityStack extends Task { next.setState(RESUMED, "resumeTopActivityInnerLocked"); next.app.updateProcessInfo(false /* updateServiceConnectionActivities */, - true /* activityChange */, true /* updateOomAdj */); + true /* activityChange */, true /* updateOomAdj */, + true /* addPendingTopUid */); // Have the window manager re-evaluate the orientation of // the screen based on the new activity order. diff --git a/services/core/java/com/android/server/wm/WindowProcessController.java b/services/core/java/com/android/server/wm/WindowProcessController.java index 2ec6df5878ef9..29cf1776df9ce 100644 --- a/services/core/java/com/android/server/wm/WindowProcessController.java +++ b/services/core/java/com/android/server/wm/WindowProcessController.java @@ -1007,8 +1007,11 @@ public class WindowProcessController extends ConfigurationContainer