From 3f5b1521f048e8a396a70cc5510abaa9b6eae561 Mon Sep 17 00:00:00 2001 From: Hui Yu Date: Fri, 15 May 2020 14:16:02 -0700 Subject: [PATCH] Check PendingStartActivityUids list before updateOomAdj is done. 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 ActivityManagerService.isUidActive() before updateOomAdj is done. Use PendingStartActivityUids to save uid after WindowManager start activity and before updateOomAdj is done. PendingStartActivityUids list is checked in ActivityManagerService.isUidActive() and AppOpsService.UidState.evalMode(). A uid in this list is treated same as uid is active. Bug: 157180494 Test: atest cts/tests/app/src/android/app/cts/ActivityManagerCameraLaunchTest.java Change-Id: If0685c3c2fad01e48f3fcf2228057041f4ec9b00 --- .../android/app/ActivityManagerInternal.java | 13 ++++ .../server/am/ActivityManagerService.java | 73 ++++++++++++++++++- .../com/android/server/am/OomAdjuster.java | 1 + .../android/server/appop/AppOpsService.java | 30 +++++++- .../server/wm/WindowProcessController.java | 5 ++ 5 files changed, 119 insertions(+), 3 deletions(-) diff --git a/core/java/android/app/ActivityManagerInternal.java b/core/java/android/app/ActivityManagerInternal.java index f1472dd002fdb..c491eea7a674a 100644 --- a/core/java/android/app/ActivityManagerInternal.java +++ b/core/java/android/app/ActivityManagerInternal.java @@ -428,4 +428,17 @@ public abstract class ActivityManagerInternal { String[] requiredPermissions, boolean serialized, int userId, int[] appIdWhitelist); + /** + * Add or delete uid from the ActivityManagerService PendingStartActivityUids list. + * @param uid uid + * @param pending add to the list if true, delete from list if false. + */ + public abstract void updatePendingTopUid(int uid, boolean pending); + + /** + * Is the uid in ActivityManagerService PendingStartActivityUids list? + * @param uid + * @return true if exists, false otherwise. + */ + public abstract boolean isPendingTopUid(int uid); } diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 37f1ad100eee0..671733b7cb7a6 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -299,6 +299,7 @@ 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; @@ -821,6 +822,46 @@ 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; + } + } + /** * Puts the process record in the map. *

NOTE: Callers should avoid acquiring the mPidsSelfLocked lock before calling this @@ -8791,7 +8832,18 @@ public class ActivityManagerService extends IActivityManager.Stub "isUidActive"); } synchronized (this) { - return isUidActiveLocked(uid); + if (isUidActiveLocked(uid)) { + return true; + } + } + + if (mInternal.isPendingTopUid(uid)) { + Slog.wtf(TAG, "PendingStartActivityUids isUidActive false but" + + " isPendingTopUid true, uid:" + uid + + " callingPackage:" + callingPackage); + return true; + } else { + return false; } } @@ -19682,6 +19734,25 @@ public class ActivityManagerService extends IActivityManager.Stub return uid >= 0 && mDeviceOwnerUid == uid; } } + + @Override + public void updatePendingTopUid(int uid, boolean pending) { + synchronized (mPendingStartActivityUidsLocked) { + if (pending) { + mPendingStartActivityUidsLocked.add(uid); + } else { + mPendingStartActivityUidsLocked.delete(uid); + } + } + + } + + @Override + public boolean isPendingTopUid(int uid) { + synchronized (mPendingStartActivityUidsLocked) { + return mPendingStartActivityUidsLocked.isPendingTopUid(uid); + } + } } long inputDispatchingTimedOut(int pid, final boolean aboveSystem, String reason) { diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java index c13bb5aff9b9d..f7a158a885c64 100644 --- a/services/core/java/com/android/server/am/OomAdjuster.java +++ b/services/core/java/com/android/server/am/OomAdjuster.java @@ -938,6 +938,7 @@ public final class OomAdjuster { mService.mServices.foregroundServiceProcStateChangedLocked(uidRec); } } + mService.mInternal.updatePendingTopUid(uidRec.uid, false); } if (mLocalPowerManager != null) { mLocalPowerManager.finishUidChanges(); diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index ae38e81fb8a37..e70de5727334b 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -521,6 +521,7 @@ public class AppOpsService extends IAppOpsService.Stub { public boolean hasForegroundWatchers; public long lastTimeShowDebugToast; + public long lastTimePendingTopUid; public UidState(int uid) { this.uid = uid; @@ -542,6 +543,10 @@ public class AppOpsService extends IAppOpsService.Stub { if (mode == MODE_FOREGROUND) { if (appWidgetVisible) { 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. return MODE_ALLOWED; @@ -604,7 +609,11 @@ public class AppOpsService extends IAppOpsService.Stub { } else if (mode == MODE_ALLOWED) { switch (op) { case OP_CAMERA: - if ((capability & PROCESS_CAPABILITY_FOREGROUND_CAMERA) != 0) { + if (mActivityManagerInternal != null + && mActivityManagerInternal.isPendingTopUid(uid)) { + maybeLogPendingTopUid(op, mode); + return MODE_ALLOWED; + } else if ((capability & PROCESS_CAPABILITY_FOREGROUND_CAMERA) != 0) { return MODE_ALLOWED; } else if ((capability & DEBUG_PROCESS_CAPABILITY_FOREGROUND_CAMERA_Q) != 0) { @@ -618,7 +627,11 @@ public class AppOpsService extends IAppOpsService.Stub { return MODE_IGNORED; } case OP_RECORD_AUDIO: - if ((capability & PROCESS_CAPABILITY_FOREGROUND_MICROPHONE) != 0) { + if (mActivityManagerInternal != null + && mActivityManagerInternal.isPendingTopUid(uid)) { + maybeLogPendingTopUid(op, mode); + return MODE_ALLOWED; + } else if ((capability & PROCESS_CAPABILITY_FOREGROUND_MICROPHONE) != 0) { return MODE_ALLOWED; } else if ((capability & DEBUG_PROCESS_CAPABILITY_FOREGROUND_MICROPHONE_Q) != 0) { @@ -704,6 +717,19 @@ 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/WindowProcessController.java b/services/core/java/com/android/server/wm/WindowProcessController.java index fe68cd6110f29..cdbcae8d9b302 100644 --- a/services/core/java/com/android/server/wm/WindowProcessController.java +++ b/services/core/java/com/android/server/wm/WindowProcessController.java @@ -44,6 +44,7 @@ import static com.android.server.wm.ActivityTaskManagerService.RELAUNCH_REASON_N import android.Manifest; import android.annotation.NonNull; import android.annotation.Nullable; +import android.app.ActivityManager; import android.app.ActivityThread; import android.app.IApplicationThread; import android.app.ProfilerInfo; @@ -1065,6 +1066,10 @@ public class WindowProcessController extends ConfigurationContainer