From 4d298b5df143b32b30d5d442f8743e292ed2147d Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Mon, 5 Feb 2018 10:54:58 -0800 Subject: [PATCH] Don't take a lock on the UI thread Bug: 72945360 Test: atest CtsAlarmManagerTestCases Test: atest CtsBatterySavingTestCases Test: atest $ANDROID_BUILD_TOP/frameworks/base/services/tests/servicestests/src/com/android/server/ForceAppStandbyTrackerTest.java Change-Id: Ifbc72acadf512d70625bbf4e08e960349eeae4d8 --- .../android/server/AlarmManagerService.java | 23 ++-- .../server/ForceAppStandbyTracker.java | 118 +++++++++++++----- .../server/ForceAppStandbyTrackerTest.java | 13 ++ 3 files changed, 112 insertions(+), 42 deletions(-) diff --git a/services/core/java/com/android/server/AlarmManagerService.java b/services/core/java/com/android/server/AlarmManagerService.java index f49cd6745e792..64bbbb9b15563 100644 --- a/services/core/java/com/android/server/AlarmManagerService.java +++ b/services/core/java/com/android/server/AlarmManagerService.java @@ -3470,10 +3470,15 @@ class AlarmManagerService extends SystemService { public static final int REPORT_ALARMS_ACTIVE = 4; public static final int APP_STANDBY_BUCKET_CHANGED = 5; public static final int APP_STANDBY_PAROLE_CHANGED = 6; + public static final int REMOVE_FOR_STOPPED = 7; public AlarmHandler() { } + public void postRemoveForStopped(int uid) { + obtainMessage(REMOVE_FOR_STOPPED, uid, 0).sendToTarget(); + } + public void handleMessage(Message msg) { switch (msg.what) { case ALARM_EVENT: { @@ -3528,6 +3533,12 @@ class AlarmManagerService extends SystemService { } break; + case REMOVE_FOR_STOPPED: + synchronized (mLock) { + removeForStoppedLocked(msg.arg1); + } + break; + default: // nope, just ignore it break; @@ -3717,10 +3728,8 @@ class AlarmManagerService extends SystemService { } @Override public void onUidGone(int uid, boolean disabled) { - synchronized (mLock) { - if (disabled) { - removeForStoppedLocked(uid); - } + if (disabled) { + mHandler.postRemoveForStopped(uid); } } @@ -3728,10 +3737,8 @@ class AlarmManagerService extends SystemService { } @Override public void onUidIdle(int uid, boolean disabled) { - synchronized (mLock) { - if (disabled) { - removeForStoppedLocked(uid); - } + if (disabled) { + mHandler.postRemoveForStopped(uid); } } diff --git a/services/core/java/com/android/server/ForceAppStandbyTracker.java b/services/core/java/com/android/server/ForceAppStandbyTracker.java index 339101fa79bb6..100680df637da 100644 --- a/services/core/java/com/android/server/ForceAppStandbyTracker.java +++ b/services/core/java/com/android/server/ForceAppStandbyTracker.java @@ -614,48 +614,22 @@ public class ForceAppStandbyTracker { private final class UidObserver extends IUidObserver.Stub { @Override public void onUidStateChanged(int uid, int procState, long procStateSeq) { - synchronized (mLock) { - if (procState > ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND) { - if (removeUidFromArray(mForegroundUids, uid, false)) { - mHandler.notifyUidForegroundStateChanged(uid); - } - } else { - if (addUidToArray(mForegroundUids, uid)) { - mHandler.notifyUidForegroundStateChanged(uid); - } - } - } - } - - @Override - public void onUidGone(int uid, boolean disabled) { - removeUid(uid, true); + mHandler.onUidStateChanged(uid, procState); } @Override public void onUidActive(int uid) { - synchronized (mLock) { - if (addUidToArray(mActiveUids, uid)) { - mHandler.notifyUidActiveStateChanged(uid); - } - } + mHandler.onUidActive(uid); + } + + @Override + public void onUidGone(int uid, boolean disabled) { + mHandler.onUidGone(uid, disabled); } @Override public void onUidIdle(int uid, boolean disabled) { - // Just to avoid excessive memcpy, don't remove from the array in this case. - removeUid(uid, false); - } - - private void removeUid(int uid, boolean remove) { - synchronized (mLock) { - if (removeUidFromArray(mActiveUids, uid, remove)) { - mHandler.notifyUidActiveStateChanged(uid); - } - if (removeUidFromArray(mForegroundUids, uid, remove)) { - mHandler.notifyUidForegroundStateChanged(uid); - } - } + mHandler.onUidIdle(uid, disabled); } @Override @@ -740,6 +714,11 @@ public class ForceAppStandbyTracker { private static final int MSG_FORCE_APP_STANDBY_FEATURE_FLAG_CHANGED = 9; private static final int MSG_EXEMPT_CHANGED = 10; + private static final int MSG_ON_UID_STATE_CHANGED = 11; + private static final int MSG_ON_UID_ACTIVE = 12; + private static final int MSG_ON_UID_GONE = 13; + private static final int MSG_ON_UID_IDLE = 14; + public MyHandler(Looper looper) { super(looper); } @@ -790,6 +769,22 @@ public class ForceAppStandbyTracker { obtainMessage(MSG_USER_REMOVED, userId, 0).sendToTarget(); } + public void onUidStateChanged(int uid, int procState) { + obtainMessage(MSG_ON_UID_STATE_CHANGED, uid, procState).sendToTarget(); + } + + public void onUidActive(int uid) { + obtainMessage(MSG_ON_UID_ACTIVE, uid, 0).sendToTarget(); + } + + public void onUidGone(int uid, boolean disabled) { + obtainMessage(MSG_ON_UID_GONE, uid, disabled ? 1 : 0).sendToTarget(); + } + + public void onUidIdle(int uid, boolean disabled) { + obtainMessage(MSG_ON_UID_IDLE, uid, disabled ? 1 : 0).sendToTarget(); + } + @Override public void handleMessage(Message msg) { switch (msg.what) { @@ -883,6 +878,61 @@ public class ForceAppStandbyTracker { case MSG_USER_REMOVED: handleUserRemoved(msg.arg1); return; + + case MSG_ON_UID_STATE_CHANGED: + handleUidStateChanged(msg.arg1, msg.arg2); + return; + case MSG_ON_UID_ACTIVE: + handleUidActive(msg.arg1); + return; + case MSG_ON_UID_GONE: + handleUidGone(msg.arg1, msg.arg1 != 0); + return; + case MSG_ON_UID_IDLE: + handleUidIdle(msg.arg1, msg.arg1 != 0); + return; + } + } + + public void handleUidStateChanged(int uid, int procState) { + synchronized (mLock) { + if (procState > ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND) { + if (removeUidFromArray(mForegroundUids, uid, false)) { + mHandler.notifyUidForegroundStateChanged(uid); + } + } else { + if (addUidToArray(mForegroundUids, uid)) { + mHandler.notifyUidForegroundStateChanged(uid); + } + } + } + } + + public void handleUidActive(int uid) { + synchronized (mLock) { + if (addUidToArray(mActiveUids, uid)) { + mHandler.notifyUidActiveStateChanged(uid); + } + } + } + + public void handleUidGone(int uid, boolean disabled) { + removeUid(uid, true); + } + + public void handleUidIdle(int uid, boolean disabled) { + // Just to avoid excessive memcpy, don't remove from the array in this case. + removeUid(uid, false); + } + + private void removeUid(int uid, boolean remove) { + synchronized (mLock) { + if (removeUidFromArray(mActiveUids, uid, remove)) { + mHandler.notifyUidActiveStateChanged(uid); + } + if (removeUidFromArray(mForegroundUids, uid, remove)) { + mHandler.notifyUidForegroundStateChanged(uid); + } } } } diff --git a/services/tests/servicestests/src/com/android/server/ForceAppStandbyTrackerTest.java b/services/tests/servicestests/src/com/android/server/ForceAppStandbyTrackerTest.java index 2c50f2272187f..a499472d197f9 100644 --- a/services/tests/servicestests/src/com/android/server/ForceAppStandbyTrackerTest.java +++ b/services/tests/servicestests/src/com/android/server/ForceAppStandbyTrackerTest.java @@ -343,6 +343,7 @@ public class ForceAppStandbyTrackerTest { assertTrue(instance.isUidActive(Process.SYSTEM_UID)); mIUidObserver.onUidActive(UID_1); + waitUntilMainHandlerDrain(); areRestricted(instance, UID_1, PACKAGE_1, NONE); areRestricted(instance, UID_2, PACKAGE_2, JOBS_AND_ALARMS); areRestricted(instance, Process.SYSTEM_UID, PACKAGE_SYSTEM, NONE); @@ -350,6 +351,7 @@ public class ForceAppStandbyTrackerTest { assertFalse(instance.isUidActive(UID_2)); mIUidObserver.onUidGone(UID_1, /*disable=*/ false); + waitUntilMainHandlerDrain(); areRestricted(instance, UID_1, PACKAGE_1, JOBS_AND_ALARMS); areRestricted(instance, UID_2, PACKAGE_2, JOBS_AND_ALARMS); areRestricted(instance, Process.SYSTEM_UID, PACKAGE_SYSTEM, NONE); @@ -357,11 +359,13 @@ public class ForceAppStandbyTrackerTest { assertFalse(instance.isUidActive(UID_2)); mIUidObserver.onUidActive(UID_1); + waitUntilMainHandlerDrain(); areRestricted(instance, UID_1, PACKAGE_1, NONE); areRestricted(instance, UID_2, PACKAGE_2, JOBS_AND_ALARMS); areRestricted(instance, Process.SYSTEM_UID, PACKAGE_SYSTEM, NONE); mIUidObserver.onUidIdle(UID_1, /*disable=*/ false); + waitUntilMainHandlerDrain(); areRestricted(instance, UID_1, PACKAGE_1, JOBS_AND_ALARMS); areRestricted(instance, UID_2, PACKAGE_2, JOBS_AND_ALARMS); areRestricted(instance, Process.SYSTEM_UID, PACKAGE_SYSTEM, NONE); @@ -467,6 +471,7 @@ public class ForceAppStandbyTrackerTest { mIUidObserver.onUidActive(UID_1); + waitUntilMainHandlerDrain(); assertTrue(instance.isUidActive(UID_1)); assertFalse(instance.isUidActive(UID_2)); assertTrue(instance.isUidActive(Process.SYSTEM_UID)); @@ -479,6 +484,7 @@ public class ForceAppStandbyTrackerTest { mIUidObserver.onUidStateChanged(UID_2, ActivityManager.PROCESS_STATE_BOUND_FOREGROUND_SERVICE, 0); + waitUntilMainHandlerDrain(); assertTrue(instance.isUidActive(UID_1)); assertFalse(instance.isUidActive(UID_2)); assertTrue(instance.isUidActive(Process.SYSTEM_UID)); @@ -491,6 +497,7 @@ public class ForceAppStandbyTrackerTest { mIUidObserver.onUidStateChanged(UID_1, ActivityManager.PROCESS_STATE_FOREGROUND_SERVICE, 0); + waitUntilMainHandlerDrain(); assertTrue(instance.isUidActive(UID_1)); assertFalse(instance.isUidActive(UID_2)); assertTrue(instance.isUidActive(Process.SYSTEM_UID)); @@ -501,6 +508,7 @@ public class ForceAppStandbyTrackerTest { mIUidObserver.onUidGone(UID_1, true); + waitUntilMainHandlerDrain(); assertFalse(instance.isUidActive(UID_1)); assertFalse(instance.isUidActive(UID_2)); assertTrue(instance.isUidActive(Process.SYSTEM_UID)); @@ -511,6 +519,7 @@ public class ForceAppStandbyTrackerTest { mIUidObserver.onUidIdle(UID_2, true); + waitUntilMainHandlerDrain(); assertFalse(instance.isUidActive(UID_1)); assertFalse(instance.isUidActive(UID_2)); assertTrue(instance.isUidActive(Process.SYSTEM_UID)); @@ -522,6 +531,7 @@ public class ForceAppStandbyTrackerTest { mIUidObserver.onUidStateChanged(UID_1, ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND, 0); + waitUntilMainHandlerDrain(); assertFalse(instance.isUidActive(UID_1)); assertFalse(instance.isUidActive(UID_2)); assertTrue(instance.isUidActive(Process.SYSTEM_UID)); @@ -533,6 +543,7 @@ public class ForceAppStandbyTrackerTest { mIUidObserver.onUidStateChanged(UID_1, ActivityManager.PROCESS_STATE_TRANSIENT_BACKGROUND, 0); + waitUntilMainHandlerDrain(); assertFalse(instance.isUidActive(UID_1)); assertFalse(instance.isUidActive(UID_2)); assertTrue(instance.isUidActive(Process.SYSTEM_UID)); @@ -1037,6 +1048,8 @@ public class ForceAppStandbyTrackerTest { mIUidObserver.onUidActive(UID_1); mIUidObserver.onUidActive(UID_10_1); + waitUntilMainHandlerDrain(); + setAppOps(UID_2, PACKAGE_2, true); setAppOps(UID_10_2, PACKAGE_2, true);