From eaaebf60de73ee9da592748d96a53c57fe51c1ff Mon Sep 17 00:00:00 2001 From: Tetiana Meronyk Date: Mon, 29 Apr 2024 10:47:34 +0000 Subject: Optimise UserWakeupStorePerformance Before the change, mStartingUsers list was used to store users that were being started. This caused spikiness and user boot time regression. After the change, this intermediate mStartingUsers list was removed and the performance was returned to previous state. This step is not necessary as it lasts only a few ms which leaves a very narrow window when user ID is not in the list. Bug: 337788578 Test: manual && perfetto trace testing && atest UserWakeupStoreTest Flag: com.android.server.alarm.start_user_before_scheduled_alarms Change-Id: I83ad157c0b0fa118ce9f2bd3237a44e784641132 --- .../android/server/alarm/AlarmManagerService.java | 9 ---- .../com/android/server/alarm/UserWakeupStore.java | 56 ++-------------------- 2 files changed, 4 insertions(+), 61 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index b982d1253e21..dfa72069c28a 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -4925,7 +4925,6 @@ public class AlarmManagerService extends SystemService { sdFilter.addAction(Intent.ACTION_EXTERNAL_APPLICATIONS_UNAVAILABLE); sdFilter.addAction(Intent.ACTION_USER_STOPPED); if (mStartUserBeforeScheduledAlarms) { - sdFilter.addAction(Intent.ACTION_LOCKED_BOOT_COMPLETED); sdFilter.addAction(Intent.ACTION_USER_REMOVED); } sdFilter.addAction(Intent.ACTION_UID_REMOVED); @@ -4958,14 +4957,6 @@ public class AlarmManagerService extends SystemService { mTemporaryQuotaReserve.removeForUser(userHandle); } return; - case Intent.ACTION_LOCKED_BOOT_COMPLETED: - final int handle = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, -1); - if (handle >= 0) { - if (mStartUserBeforeScheduledAlarms) { - mUserWakeupStore.onUserStarted(handle); - } - } - return; case Intent.ACTION_USER_REMOVED: final int user = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, -1); if (user >= 0) { diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java b/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java index 7fc630c964e5..6840877369f5 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java @@ -98,12 +98,7 @@ public class UserWakeupStore { */ @GuardedBy("mUserWakeupLock") private final SparseLongArray mUserStarts = new SparseLongArray(); - /** - * A list of users that are in a phase after they have been started but before alarms were - * initialized. - */ - @GuardedBy("mUserWakeupLock") - private final SparseLongArray mStartingUsers = new SparseLongArray(); + private Executor mBackgroundExecutor; private static final File USER_WAKEUP_DIR = new File(Environment.getDataSystemDirectory(), ROOT_DIR_NAME); @@ -124,9 +119,6 @@ public class UserWakeupStore { */ public void addUserWakeup(int userId, long alarmTime) { synchronized (mUserWakeupLock) { - // This should not be needed, but if an app in the user is scheduling an alarm clock, we - // consider the user start complete. There is a dedicated removal when user is started. - mStartingUsers.delete(userId); mUserStarts.put(userId, alarmTime - BUFFER_TIME_MS + getUserWakeupOffset()); } updateUserListFile(); @@ -192,23 +184,10 @@ public class UserWakeupStore { } /** - * Move user from wakeup list to starting user list. + * Remove scheduled user wakeup from the list when it is started. */ public void onUserStarting(int userId) { - synchronized (mUserWakeupLock) { - final long wakeup = getWakeupTimeForUser(userId); - if (wakeup >= 0) { - mStartingUsers.put(userId, wakeup); - mUserStarts.delete(userId); - } - } - } - - /** - * Remove userId from starting user list once start is complete. - */ - public void onUserStarted(int userId) { - if (deleteWakeupFromStartingUsers(userId)) { + if (deleteWakeupFromUserStarts(userId)) { updateUserListFile(); } } @@ -217,7 +196,7 @@ public class UserWakeupStore { * Remove userId from the store when the user is removed. */ public void onUserRemoved(int userId) { - if (deleteWakeupFromUserStarts(userId) || deleteWakeupFromStartingUsers(userId)) { + if (deleteWakeupFromUserStarts(userId)) { updateUserListFile(); } } @@ -237,21 +216,6 @@ public class UserWakeupStore { return index >= 0; } - /** - * Remove wakeup for a given userId from mStartingUsers. - * @return true if an entry is found and the list of wakeups changes. - */ - private boolean deleteWakeupFromStartingUsers(int userId) { - int index; - synchronized (mUserWakeupLock) { - index = mStartingUsers.indexOfKey(userId); - if (index >= 0) { - mStartingUsers.removeAt(index); - } - } - return index >= 0; - } - /** * Get the soonest wakeup time in the store. */ @@ -299,9 +263,6 @@ public class UserWakeupStore { for (int i = 0; i < mUserStarts.size(); i++) { listOfUsers.add(new Pair<>(mUserStarts.keyAt(i), mUserStarts.valueAt(i))); } - for (int i = 0; i < mStartingUsers.size(); i++) { - listOfUsers.add(new Pair<>(mStartingUsers.keyAt(i), mStartingUsers.valueAt(i))); - } } Collections.sort(listOfUsers, Comparator.comparingLong(pair -> pair.second)); for (int i = 0; i < listOfUsers.size(); i++) { @@ -329,7 +290,6 @@ public class UserWakeupStore { } synchronized (mUserWakeupLock) { mUserStarts.clear(); - mStartingUsers.clear(); } try (FileInputStream fis = userWakeupFile.openRead()) { final TypedXmlPullParser parser = Xml.resolvePullParser(fis); @@ -396,14 +356,6 @@ public class UserWakeupStore { TimeUtils.formatDuration(mUserStarts.valueAt(i), nowELAPSED, pw); pw.println(); } - pw.println(mStartingUsers.size() + " starting users: "); - for (int i = 0; i < mStartingUsers.size(); i++) { - pw.print("UserId: "); - pw.print(mStartingUsers.keyAt(i)); - pw.print(", userStartTime: "); - TimeUtils.formatDuration(mStartingUsers.valueAt(i), nowELAPSED, pw); - pw.println(); - } pw.decreaseIndent(); } } -- cgit v1.2.3-59-g8ed1b From 74a895c5d0f3b07ec9edfac514a116b0d3d6e9bc Mon Sep 17 00:00:00 2001 From: Tetiana Meronyk Date: Fri, 21 Jun 2024 07:28:21 +0000 Subject: Refactor deleteWakeupFromUserStarts in UserWakeupStore Bug: 314907186 Test: manual Flag: EXEMPT trivial refactoring Change-Id: I3485e90b7b2e6bd5afb752839da936c948dc0f5e --- .../service/java/com/android/server/alarm/UserWakeupStore.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java b/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java index 7fc630c964e5..f0b30f23d9b8 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java @@ -227,14 +227,14 @@ public class UserWakeupStore { * @return true if an entry is found and the list of wakeups changes. */ private boolean deleteWakeupFromUserStarts(int userId) { - int index; synchronized (mUserWakeupLock) { - index = mUserStarts.indexOfKey(userId); + final int index = mUserStarts.indexOfKey(userId); if (index >= 0) { mUserStarts.removeAt(index); + return true; } + return false; } - return index >= 0; } /** -- cgit v1.2.3-59-g8ed1b From 04e5b90e5293643be3108390ece5f64a7a5eec3c Mon Sep 17 00:00:00 2001 From: Tetiana Meronyk Date: Mon, 15 Jul 2024 15:53:34 +0000 Subject: Enable user of UserWakeupStore only on devices that support multiuser. Since devices that do not support multiuser do not have secondary users, there is no need to store wakeups on them. Bug: 352696527 Test: atest AlarmManagerServiceTest Flag: com.android.server.alarm.start_user_before_scheduled_alarms Change-Id: Id1b7d4ae514de42c21e1fdee670d36a88dbeae3d --- .../service/java/com/android/server/alarm/AlarmManagerService.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index dfa72069c28a..ee03e4b2ccd1 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -120,6 +120,7 @@ import android.os.SystemProperties; import android.os.ThreadLocalWorkSource; import android.os.Trace; import android.os.UserHandle; +import android.os.UserManager; import android.os.WorkSource; import android.provider.DeviceConfig; import android.provider.Settings; @@ -1794,7 +1795,8 @@ public class AlarmManagerService extends SystemService { mActivityManagerInternal = LocalServices.getService(ActivityManagerInternal.class); mUseFrozenStateToDropListenerAlarms = Flags.useFrozenStateToDropListenerAlarms(); - mStartUserBeforeScheduledAlarms = Flags.startUserBeforeScheduledAlarms(); + mStartUserBeforeScheduledAlarms = Flags.startUserBeforeScheduledAlarms() + && UserManager.supportsMultipleUsers(); if (mStartUserBeforeScheduledAlarms) { mUserWakeupStore = new UserWakeupStore(); mUserWakeupStore.init(); @@ -3015,7 +3017,7 @@ public class AlarmManagerService extends SystemService { mUseFrozenStateToDropListenerAlarms); pw.println(); pw.print(Flags.FLAG_START_USER_BEFORE_SCHEDULED_ALARMS, - mStartUserBeforeScheduledAlarms); + Flags.startUserBeforeScheduledAlarms()); pw.decreaseIndent(); pw.println(); pw.println(); -- cgit v1.2.3-59-g8ed1b From d86de3022a0e97c86213dd2f67ee3550b6d20ab9 Mon Sep 17 00:00:00 2001 From: Tetiana Meronyk Date: Thu, 18 Jul 2024 11:17:53 +0000 Subject: Do not schedule alarm wakeups for SYSTEM users. SYSTEM users can never be stopped, so waking them up is not necessary. On non-HSUM devices, this also means Main user. On HSUM SYSTEM doesn't schedule user facing alarms so no need to schedule wake Bug: 352696527 Test: atest UserWakeupStore Flag: com.android.server.alarm.start_user_before_scheduled_alarms Change-Id: Ib5709ff2c55ed756478e4e74a49a4681bd332d8d --- .../java/com/android/server/alarm/UserWakeupStore.java | 12 ++++++++---- .../src/com/android/server/alarm/UserWakeupStoreTest.java | 10 ++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java b/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java index dc5e3414a819..93904a773ed5 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java @@ -20,6 +20,7 @@ package com.android.server.alarm; import android.annotation.Nullable; import android.os.Environment; import android.os.SystemClock; +import android.os.UserHandle; import android.util.AtomicFile; import android.util.IndentingPrintWriter; import android.util.Pair; @@ -113,15 +114,18 @@ public class UserWakeupStore { } /** - * Add user wakeup for the alarm. + * Add user wakeup for the alarm if needed. * @param userId Id of the user that scheduled alarm. * @param alarmTime time when alarm is expected to trigger. */ public void addUserWakeup(int userId, long alarmTime) { - synchronized (mUserWakeupLock) { - mUserStarts.put(userId, alarmTime - BUFFER_TIME_MS + getUserWakeupOffset()); + // SYSTEM user is always running, so no need to schedule wakeup for it. + if (userId != UserHandle.USER_SYSTEM) { + synchronized (mUserWakeupLock) { + mUserStarts.put(userId, alarmTime - BUFFER_TIME_MS + getUserWakeupOffset()); + } + updateUserListFile(); } - updateUserListFile(); } /** diff --git a/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java b/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java index 72883e269a65..5bd919f28e6a 100644 --- a/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java @@ -23,6 +23,7 @@ import static com.android.server.alarm.UserWakeupStore.USER_START_TIME_DEVIATION import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.testng.AssertJUnit.assertFalse; import android.os.Environment; import android.os.FileUtils; @@ -51,6 +52,7 @@ public class UserWakeupStoreTest { private static final int USER_ID_1 = 10; private static final int USER_ID_2 = 11; private static final int USER_ID_3 = 12; + private static final int USER_ID_SYSTEM = 0; private static final long TEST_TIMESTAMP = 150_000; private static final File TEST_SYSTEM_DIR = new File(InstrumentationRegistry .getInstrumentation().getContext().getDataDir(), "alarmsTestDir"); @@ -109,6 +111,14 @@ public class UserWakeupStoreTest { assertTrue(file.exists()); } + @Test + public void testAddWakeupForSystemUser_shouldDoNothing() { + mUserWakeupStore.addUserWakeup(USER_ID_SYSTEM, TEST_TIMESTAMP - 19_000); + assertEquals(0, mUserWakeupStore.getUserIdsToWakeup(TEST_TIMESTAMP).length); + final File file = new File(ROOT_DIR , "usersWithAlarmClocks.xml"); + assertFalse(file.exists()); + } + @Test public void testAddMultipleWakeupsForUser_ensureOnlyLastWakeupRemains() { final long finalAlarmTime = TEST_TIMESTAMP - 13_000; -- cgit v1.2.3-59-g8ed1b From 7fe2d011062be5ff098c9b855efb5911d64354ab Mon Sep 17 00:00:00 2001 From: Jorge De la Torre Date: Wed, 24 Jul 2024 18:30:36 +0000 Subject: Revert "AlarmManager: Store the next dst transition in system properties" This reverts commit 2dd606d7ad5ae7588259ad8faf549e8a389397c1. Reason for revert: Updating how sys props are stored b/340482112 Change-Id: I7ec8ba1c6535aaecaeed35a913ed8af6f00464fe --- .../android/server/alarm/AlarmManagerService.java | 23 +--------------------- 1 file changed, 1 insertion(+), 22 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index 8258a6f424a7..39de0af3c8d0 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -179,9 +179,6 @@ import java.io.PrintWriter; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.text.SimpleDateFormat; -import java.time.Instant; -import java.time.zone.ZoneOffsetTransition; -import java.time.zone.ZoneRules; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; @@ -193,7 +190,6 @@ import java.util.Set; import java.util.TimeZone; import java.util.TreeSet; import java.util.concurrent.ThreadLocalRandom; -import java.util.concurrent.TimeUnit; import java.util.function.Predicate; /** @@ -234,12 +230,8 @@ public class AlarmManagerService extends SystemService { private static final long TEMPORARY_QUOTA_DURATION = INTERVAL_DAY; - // System properties read on some device configurations to initialize time properly and - // perform DST transitions at the bootloader level. + // System property read on some device configurations to initialize time properly. private static final String TIMEOFFSET_PROPERTY = "persist.sys.time.offset"; - private static final String DST_TRANSITION_PROPERTY = "persist.sys.time.dst_transition"; - private static final String DST_OFFSET_PROPERTY = "persist.sys.time.dst_offset"; - private final Intent mBackgroundIntent = new Intent().addFlags(Intent.FLAG_FROM_BACKGROUND); @@ -2157,19 +2149,6 @@ public class AlarmManagerService extends SystemService { final int gmtOffset = newZone.getOffset(mInjector.getCurrentTimeMillis()); SystemProperties.set(TIMEOFFSET_PROPERTY, String.valueOf(gmtOffset)); - - final ZoneRules rules = newZone.toZoneId().getRules(); - final ZoneOffsetTransition transition = rules.nextTransition(Instant.now()); - if (null != transition) { - // Get the offset between the time after the DST transition and before. - final long transitionOffset = TimeUnit.SECONDS.toMillis(( - transition.getOffsetAfter().getTotalSeconds() - - transition.getOffsetBefore().getTotalSeconds())); - // Time when the next DST transition is programmed. - final long nextTransition = TimeUnit.SECONDS.toMillis(transition.toEpochSecond()); - SystemProperties.set(DST_TRANSITION_PROPERTY, String.valueOf(nextTransition)); - SystemProperties.set(DST_OFFSET_PROPERTY, String.valueOf(transitionOffset)); - } } // Clear the default time zone in the system server process. This forces the next call -- cgit v1.2.3-59-g8ed1b From 56b0953a33c0eb582a1aa8ebba418e9186a365f2 Mon Sep 17 00:00:00 2001 From: Jorge De la Torre Date: Wed, 24 Jul 2024 18:32:54 +0000 Subject: Revert "AlarmManager: Store time offset system property" This reverts commit 45f7d8701f75128951a7678729c2ba95a07bdf5b. Reason for revert: Updating how sys props are stored b/340482112 Change-Id: Ib846d42fac638e71874a97840b32159132eb9046 --- .../service/java/com/android/server/alarm/AlarmManagerService.java | 7 ------- 1 file changed, 7 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index 39de0af3c8d0..00f6e5c352d2 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -115,7 +115,6 @@ import android.os.ServiceManager; import android.os.ShellCallback; import android.os.ShellCommand; import android.os.SystemClock; -import android.os.SystemProperties; import android.os.ThreadLocalWorkSource; import android.os.Trace; import android.os.UserHandle; @@ -230,9 +229,6 @@ public class AlarmManagerService extends SystemService { private static final long TEMPORARY_QUOTA_DURATION = INTERVAL_DAY; - // System property read on some device configurations to initialize time properly. - private static final String TIMEOFFSET_PROPERTY = "persist.sys.time.offset"; - private final Intent mBackgroundIntent = new Intent().addFlags(Intent.FLAG_FROM_BACKGROUND); @@ -2146,9 +2142,6 @@ public class AlarmManagerService extends SystemService { // "GMT" if the ID is unrecognized). The parameter ID is used here rather than // newZone.getId(). It will be rejected if it is invalid. timeZoneWasChanged = SystemTimeZone.setTimeZoneId(tzId, confidence, logInfo); - - final int gmtOffset = newZone.getOffset(mInjector.getCurrentTimeMillis()); - SystemProperties.set(TIMEOFFSET_PROPERTY, String.valueOf(gmtOffset)); } // Clear the default time zone in the system server process. This forces the next call -- cgit v1.2.3-59-g8ed1b From 698bbdfea7e2bc4a10145d1ade9a0da3dbddc46e Mon Sep 17 00:00:00 2001 From: Xin Guan Date: Thu, 25 Jul 2024 15:06:23 +0000 Subject: Fix CTS breakage for CtsDevicePolicyTestCases. Ensure the idle check will be performed after bootup so that the standby bucket could be refreshed immediately for the protected packages. Bug: 354599086 Test: atest CtsDevicePolicyTestCases:android.devicepolicy.cts.UserControlDisabledPackagesTest#setUserControlDisabledPackages_exemptFromStandbyBuckets Change-Id: I6fb54a3bf035e06b8d3e4b2a7ec160cd830d4adc --- .../service/java/com/android/server/usage/AppStandbyController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/usage/AppStandbyController.java b/apex/jobscheduler/service/java/com/android/server/usage/AppStandbyController.java index c3fe0314636e..d92351de3aa1 100644 --- a/apex/jobscheduler/service/java/com/android/server/usage/AppStandbyController.java +++ b/apex/jobscheduler/service/java/com/android/server/usage/AppStandbyController.java @@ -1990,7 +1990,7 @@ public class AppStandbyController } } if (android.app.admin.flags.Flags.disallowUserControlBgUsageFix()) { - if (!Flags.avoidIdleCheck()) { + if (!Flags.avoidIdleCheck() || mInjector.getBootPhase() >= PHASE_BOOT_COMPLETED) { postCheckIdleStates(userId); } } -- cgit v1.2.3-59-g8ed1b From 49807788f0478a74b0ef084cb2f2626e087974c3 Mon Sep 17 00:00:00 2001 From: Tetiana Meronyk Date: Thu, 25 Jul 2024 00:34:57 +0000 Subject: Only schedule wakeups when alarm is set outside of user wakeup offset Before this change, a repeat wakeup was scheduled for the same alarm when the user was started. This caused a lot of unnecessary kernel reschedules and user start calls. After this change, such repeat wakeups are not scheduled. Bug: 314907186 Test: UserWakeupStoreTest Flag: com.android.server.alarm.start_user_before_scheduled_alarms Change-Id: I054d1814eece95ff2d79b4460c37434f4ff9a3df --- .../service/java/com/android/server/alarm/AlarmManagerService.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index ee03e4b2ccd1..87d157e63788 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -4523,8 +4523,9 @@ public class AlarmManagerService extends SystemService { final int[] userIds = mUserWakeupStore.getUserIdsToWakeup(nowELAPSED); for (int i = 0; i < userIds.length; i++) { - if (!mActivityManagerInternal.startUserInBackground( - userIds[i])) { + if (mActivityManagerInternal.isUserRunning(userIds[i], 0) + || !mActivityManagerInternal.startUserInBackground( + userIds[i])) { mUserWakeupStore.removeUserWakeup(userIds[i]); } } -- cgit v1.2.3-59-g8ed1b From 89ed5491a549efc5fe2f76f3e94a2901ff884a99 Mon Sep 17 00:00:00 2001 From: Tetiana Meronyk Date: Thu, 1 Aug 2024 11:10:57 +0000 Subject: Check user type before adding user wakeup Not all user types should have their wakeups persisted. Guest users should not run in the background. And since they are meant to be used temporarily, their activity like alarms should not disrupt other users. Therefore their alarms do not get persisted and thet are not started prior to their alarms. System users are always running on the device, so there is no need to start them. Profiles are not full users and have special applications. For private profile or work profile, users might want to keep them stopped so starting them in the background can be unexpected to users. So this feature at this time should only be applied to full users. Bug: 353736798 Test: atest UserWakeupStoreTest && atest AlarmManagerServiceTest Flag: com.android.server.alarm.start_user_before_scheduled_alarms Change-Id: I8657342fb48b177455352ad8ec207afa5a7e0dd0 --- .../android/server/alarm/AlarmManagerService.java | 24 +++++++++++++++++-- .../com/android/server/alarm/UserWakeupStore.java | 10 +++----- .../server/alarm/AlarmManagerServiceTest.java | 27 ++++++++++++++++++++++ .../android/server/alarm/UserWakeupStoreTest.java | 10 -------- 4 files changed, 52 insertions(+), 19 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index 9a178e573b53..18ee6f2c7992 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -165,6 +165,7 @@ import com.android.server.SystemService; import com.android.server.SystemServiceManager; import com.android.server.SystemTimeZone; import com.android.server.SystemTimeZone.TimeZoneConfidence; +import com.android.server.pm.UserManagerInternal; import com.android.server.pm.permission.PermissionManagerService; import com.android.server.pm.permission.PermissionManagerServiceInternal; import com.android.server.pm.pkg.AndroidPackage; @@ -3763,8 +3764,10 @@ public class AlarmManagerService extends SystemService { } mNextAlarmClockForUser.put(userId, alarmClock); if (mStartUserBeforeScheduledAlarms) { - mUserWakeupStore.addUserWakeup(userId, convertToElapsed( - mNextAlarmClockForUser.get(userId).getTriggerTime(), RTC)); + if (shouldAddWakeupForUser(userId)) { + mUserWakeupStore.addUserWakeup(userId, convertToElapsed( + mNextAlarmClockForUser.get(userId).getTriggerTime(), RTC)); + } } } else { if (DEBUG_ALARM_CLOCK) { @@ -3783,6 +3786,23 @@ public class AlarmManagerService extends SystemService { mHandler.sendEmptyMessage(AlarmHandler.SEND_NEXT_ALARM_CLOCK_CHANGED); } + /** + * Checks whether the user is of type that needs to be started before the alarm. + */ + @VisibleForTesting + boolean shouldAddWakeupForUser(@UserIdInt int userId) { + final UserManagerInternal umInternal = LocalServices.getService(UserManagerInternal.class); + if (umInternal.getUserInfo(userId) == null || umInternal.getUserInfo(userId).isGuest()) { + // Guest user should not be started in the background. + return false; + } else { + // SYSTEM user is always running, so no need to schedule wakeup for it. + // Profiles are excluded from the wakeup list because users can explicitly stop them and + // so starting them in the background would go against the user's intent. + return userId != UserHandle.USER_SYSTEM && umInternal.getUserInfo(userId).isFull(); + } + } + /** * Updates NEXT_ALARM_FORMATTED and sends NEXT_ALARM_CLOCK_CHANGED_INTENT for all users * for which alarm clocks have changed since the last call to this. diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java b/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java index 93904a773ed5..9fe197d69ce5 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java @@ -20,7 +20,6 @@ package com.android.server.alarm; import android.annotation.Nullable; import android.os.Environment; import android.os.SystemClock; -import android.os.UserHandle; import android.util.AtomicFile; import android.util.IndentingPrintWriter; import android.util.Pair; @@ -119,13 +118,10 @@ public class UserWakeupStore { * @param alarmTime time when alarm is expected to trigger. */ public void addUserWakeup(int userId, long alarmTime) { - // SYSTEM user is always running, so no need to schedule wakeup for it. - if (userId != UserHandle.USER_SYSTEM) { - synchronized (mUserWakeupLock) { - mUserStarts.put(userId, alarmTime - BUFFER_TIME_MS + getUserWakeupOffset()); - } - updateUserListFile(); + synchronized (mUserWakeupLock) { + mUserStarts.put(userId, alarmTime - BUFFER_TIME_MS + getUserWakeupOffset()); } + updateUserListFile(); } /** diff --git a/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java index cb15d6f84403..b980ca05b609 100644 --- a/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java @@ -132,6 +132,7 @@ import android.content.Context; import android.content.Intent; import android.content.PermissionChecker; import android.content.pm.PackageManagerInternal; +import android.content.pm.UserInfo; import android.net.Uri; import android.os.BatteryManager; import android.os.Bundle; @@ -149,6 +150,7 @@ import android.os.RemoteException; import android.os.ServiceManager; import android.os.SystemProperties; import android.os.UserHandle; +import android.os.UserManager; import android.platform.test.annotations.DisableFlags; import android.platform.test.annotations.EnableFlags; import android.platform.test.annotations.Presubmit; @@ -176,6 +178,7 @@ import com.android.server.DeviceIdleInternal; import com.android.server.LocalServices; import com.android.server.SystemClockTime.TimeConfidence; import com.android.server.SystemService; +import com.android.server.pm.UserManagerInternal; import com.android.server.pm.permission.PermissionManagerService; import com.android.server.pm.permission.PermissionManagerServiceInternal; import com.android.server.pm.pkg.AndroidPackage; @@ -250,6 +253,8 @@ public final class AlarmManagerServiceTest { @Mock private ActivityManagerInternal mActivityManagerInternal; @Mock + private UserManagerInternal mUserManagerInternal; + @Mock private ActivityManager mActivityManager; @Mock private PackageManagerInternal mPackageManagerInternal; @@ -447,6 +452,8 @@ public final class AlarmManagerServiceTest { () -> LocalServices.getService(PermissionManagerServiceInternal.class)); doReturn(mActivityManagerInternal).when( () -> LocalServices.getService(ActivityManagerInternal.class)); + doReturn(mUserManagerInternal).when( + () -> LocalServices.getService(UserManagerInternal.class)); doReturn(mPackageManagerInternal).when( () -> LocalServices.getService(PackageManagerInternal.class)); doReturn(mAppStateTracker).when(() -> LocalServices.getService(AppStateTracker.class)); @@ -1251,6 +1258,26 @@ public final class AlarmManagerServiceTest { assertEquals(0, mService.mAlarmsPerUid.get(TEST_CALLING_UID)); } + @Test + public void wakeupShouldBeScheduledForFullUsers_skipsGuestSystemAndProfiles() { + final int systemUserId = 0; + final int fullUserId = 10; + final int privateProfileId = 12; + final int guestUserId = 13; + when(mUserManagerInternal.getUserInfo(fullUserId)).thenReturn(new UserInfo(fullUserId, + "TestUser2", UserInfo.FLAG_FULL)); + when(mUserManagerInternal.getUserInfo(privateProfileId)).thenReturn(new UserInfo( + privateProfileId, "TestUser3", UserInfo.FLAG_PROFILE)); + when(mUserManagerInternal.getUserInfo(guestUserId)).thenReturn(new UserInfo( + guestUserId, "TestUserGuest", null, 0, UserManager.USER_TYPE_FULL_GUEST)); + when(mUserManagerInternal.getUserInfo(systemUserId)).thenReturn(new UserInfo( + systemUserId, "TestUserSystem", null, 0, UserManager.USER_TYPE_FULL_SYSTEM)); + assertTrue(mService.shouldAddWakeupForUser(fullUserId)); + assertFalse(mService.shouldAddWakeupForUser(systemUserId)); + assertFalse(mService.shouldAddWakeupForUser(privateProfileId)); + assertFalse(mService.shouldAddWakeupForUser(guestUserId)); + } + @Test public void sendsTimeTickOnInteractive() { final ArgumentCaptor runnableCaptor = ArgumentCaptor.forClass(Runnable.class); diff --git a/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java b/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java index 5bd919f28e6a..72883e269a65 100644 --- a/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java @@ -23,7 +23,6 @@ import static com.android.server.alarm.UserWakeupStore.USER_START_TIME_DEVIATION import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import static org.testng.AssertJUnit.assertFalse; import android.os.Environment; import android.os.FileUtils; @@ -52,7 +51,6 @@ public class UserWakeupStoreTest { private static final int USER_ID_1 = 10; private static final int USER_ID_2 = 11; private static final int USER_ID_3 = 12; - private static final int USER_ID_SYSTEM = 0; private static final long TEST_TIMESTAMP = 150_000; private static final File TEST_SYSTEM_DIR = new File(InstrumentationRegistry .getInstrumentation().getContext().getDataDir(), "alarmsTestDir"); @@ -111,14 +109,6 @@ public class UserWakeupStoreTest { assertTrue(file.exists()); } - @Test - public void testAddWakeupForSystemUser_shouldDoNothing() { - mUserWakeupStore.addUserWakeup(USER_ID_SYSTEM, TEST_TIMESTAMP - 19_000); - assertEquals(0, mUserWakeupStore.getUserIdsToWakeup(TEST_TIMESTAMP).length); - final File file = new File(ROOT_DIR , "usersWithAlarmClocks.xml"); - assertFalse(file.exists()); - } - @Test public void testAddMultipleWakeupsForUser_ensureOnlyLastWakeupRemains() { final long finalAlarmTime = TEST_TIMESTAMP - 13_000; -- cgit v1.2.3-59-g8ed1b From 699111cd90ebe58824157aa93925b725abbec8de Mon Sep 17 00:00:00 2001 From: Sudheer Shanka Date: Sun, 4 Aug 2024 19:48:17 +0000 Subject: Create the workchain by default when acquiring the wakelock. The APIs to create and use workchains have been available for a while. So, it should be safe to create a workchain by default. Since the System acquires the wakelock on behalf of the apps when scheduling jobs, the worksource should include the System uid to correctly indicate the wakelock acquirer and will allow us to differentiate between system-held and app-held wakelocks. Bug: 352676818 Test: atest services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java Test: statsd_testdrive 10 Flag: com.android.server.job.create_work_chain_by_default Change-Id: Ib135729df1a1bcaace2d01ce1a94ec16094b264a --- apex/jobscheduler/service/aconfig/job.aconfig | 10 +++++++ .../android/server/job/JobSchedulerService.java | 5 ++-- .../server/job/JobSchedulerServiceTest.java | 34 ++++++++++++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/aconfig/job.aconfig b/apex/jobscheduler/service/aconfig/job.aconfig index e489c1ad891a..e8568651eeaa 100644 --- a/apex/jobscheduler/service/aconfig/job.aconfig +++ b/apex/jobscheduler/service/aconfig/job.aconfig @@ -48,3 +48,13 @@ flag { purpose: PURPOSE_BUGFIX } } + +flag { + name: "create_work_chain_by_default" + namespace: "backstage_power" + description: "Create a workchain by default when acquiring a wakelock" + bug: "352676818" + metadata { + purpose: PURPOSE_BUGFIX + } +} diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java index ff73a4922977..5f2b01a7304a 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -1617,10 +1617,11 @@ public class JobSchedulerService extends com.android.server.SystemService @NonNull public WorkSource deriveWorkSource(int sourceUid, @Nullable String sourcePackageName) { - if (WorkSource.isChainedBatteryAttributionEnabled(getContext())) { + if (Flags.createWorkChainByDefault() + || WorkSource.isChainedBatteryAttributionEnabled(getContext())) { WorkSource ws = new WorkSource(); ws.createWorkChain() - .addNode(sourceUid, sourcePackageName) + .addNode(sourceUid, null) .addNode(Process.SYSTEM_UID, "JobScheduler"); return ws; } else { diff --git a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java index d15c24bd68b9..4e1f741b1398 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java @@ -31,8 +31,10 @@ import static com.android.server.job.JobSchedulerService.sElapsedRealtimeClock; import static com.android.server.job.JobSchedulerService.sUptimeMillisClock; import static com.android.server.job.Flags.FLAG_BATCH_ACTIVE_BUCKET_JOBS; import static com.android.server.job.Flags.FLAG_BATCH_CONNECTIVITY_JOBS_PER_NETWORK; +import static com.android.server.job.Flags.FLAG_CREATE_WORK_CHAIN_BY_DEFAULT; import static com.android.server.job.Flags.FLAG_THERMAL_RESTRICTIONS_TO_FGS_JOBS; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; @@ -58,7 +60,9 @@ import android.app.job.JobScheduler; import android.app.job.JobWorkItem; import android.app.usage.UsageStatsManagerInternal; import android.content.ComponentName; +import android.content.ContentResolver; import android.content.Context; +import android.content.IContentProvider; import android.content.Intent; import android.content.PermissionChecker; import android.content.pm.PackageManager; @@ -72,10 +76,14 @@ import android.os.BatteryManager; import android.os.BatteryManagerInternal; import android.os.BatteryManagerInternal.ChargingPolicyChangeListener; import android.os.Looper; +import android.os.Process; import android.os.RemoteException; import android.os.ServiceManager; import android.os.SystemClock; +import android.os.WorkSource; +import android.os.WorkSource.WorkChain; import android.platform.test.annotations.RequiresFlagsDisabled; +import android.platform.test.annotations.RequiresFlagsEnabled; import android.platform.test.flag.junit.CheckFlagsRule; import android.platform.test.flag.junit.DeviceFlagsValueProvider; import android.platform.test.flag.junit.SetFlagsRule; @@ -2496,6 +2504,32 @@ public class JobSchedulerServiceTest { } } + @RequiresFlagsEnabled(FLAG_CREATE_WORK_CHAIN_BY_DEFAULT) + @Test + public void testDeriveWorkSource_flagCreateWorkChainByDefaultEnabled() { + final WorkSource workSource = mService.deriveWorkSource(TEST_UID, "com.test.pkg"); + assertEquals(TEST_UID, workSource.getAttributionUid()); + + assertEquals(1, workSource.getWorkChains().size()); + final WorkChain workChain = workSource.getWorkChains().get(0); + final int[] expectedUids = {TEST_UID, Process.SYSTEM_UID}; + assertArrayEquals(expectedUids, workChain.getUids()); + } + + @RequiresFlagsDisabled(FLAG_CREATE_WORK_CHAIN_BY_DEFAULT) + @Test + public void testDeriveWorkSource_flagCreateWorkChainByDefaultDisabled() { + final ContentResolver contentResolver = mock(ContentResolver.class); + doReturn(contentResolver).when(mContext).getContentResolver(); + final IContentProvider iContentProvider = mock(IContentProvider.class); + doReturn(iContentProvider).when(contentResolver).acquireProvider(anyString()); + + final WorkSource workSource = mService.deriveWorkSource(TEST_UID, "com.test.pkg"); + assertEquals(TEST_UID, workSource.getAttributionUid()); + + assertNull(workSource.getWorkChains()); + } + private void setBatteryLevel(int level) { doReturn(level).when(mBatteryManagerInternal).getBatteryLevel(); mService.mBatteryStateTracker -- cgit v1.2.3-59-g8ed1b From 8ef22c403ed30aba5ec69d6be9bf0a23c240028d Mon Sep 17 00:00:00 2001 From: Shintaro Kawamura Date: Fri, 2 Aug 2024 17:05:03 +0900 Subject: Initialize timestamp in AlarmManagerService with actual timestamps AlarmManagerService sends android.intent.action.TIME_SET broadcast if real time clock and boot time clock proceed differently which means the real time is changed (e.g. by timezone change). Kernel notifies the time change by returning ECANCELED on timerfd. AlarmManagerService filteres the notifications with less than 1 second difference out because kernel can notifies small adjustment. With initializing timestamps (mLastTimeChangeClockTime, mLastTimeChangeRealtime) as 0, AlarmManagerService always sends android.intent.action.TIME_SET for the first time change notification from the kernel even if the time adjustment was small. While stale TIME_SET broadcast does not break any logic technically because applications usually just reset their timers, waking up BroadcastReceiver of applications simultaneously has a performance impact on devices on which the processes for the applications had not been launched and the system end up launching application processes. ARCVM on ChromeOS can easily reproduce the stale TIME_SET broadcast by suspending and resuming the device for the first time. Initializing the timestamps as 0 was introduced from BatteryStatsImpl logic by this commit: https://gerrit.aospa.co/plugins/gitiles/AOSPA/android_frameworks_base/+/c352722e8af0a5510144b5f32ea87561db553f42%5E%21/ Before that BatteryStatsImpl had just monitored time change notifications for a stats purpose and a stale notification did not have much cost. However sending a broadcast to applications costs more on AlarmManagerService. The cost of initializing the 2 timestamps on the AlarmManager startup is small enough to avoid the stale broadcast. Bug: 356564926 Test: manually check that no TIME_SET broadcast is sent on the first susupend/resume on ARCVM Test: atest FrameworksMockingServicesTests:com.android.server.alarm Flag: EXEMPT bugfix Change-Id: Ibad10ad6a7c2812dc085f80c1a94a3026f9fb96f --- .../android/server/alarm/AlarmManagerService.java | 10 ++-- .../server/alarm/AlarmManagerServiceTest.java | 58 ++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index 18ee6f2c7992..ba66ff72bfdd 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -4441,6 +4441,11 @@ public class AlarmManagerService extends SystemService { public void run() { ArrayList triggerList = new ArrayList(); + synchronized (mLock) { + mLastTimeChangeClockTime = mInjector.getCurrentTimeMillis(); + mLastTimeChangeRealtime = mInjector.getElapsedRealtimeMillis(); + } + while (true) { int result = mInjector.waitForAlarm(); final long nowRTC = mInjector.getCurrentTimeMillis(); @@ -4464,10 +4469,9 @@ public class AlarmManagerService extends SystemService { expectedClockTime = lastTimeChangeClockTime + (nowELAPSED - mLastTimeChangeRealtime); } - if (lastTimeChangeClockTime == 0 || nowRTC < (expectedClockTime - 1000) + if (nowRTC < (expectedClockTime - 1000) || nowRTC > (expectedClockTime + 1000)) { - // The change is by at least +/- 1000 ms (or this is the first change), - // let's do it! + // The change is by at least +/- 1000 ms, let's do it! if (DEBUG_BATCH) { Slog.v(TAG, "Time changed notification from kernel; rebatching"); } diff --git a/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java index b980ca05b609..30de0e8c7981 100644 --- a/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java @@ -512,6 +512,9 @@ public final class AlarmManagerServiceTest { when(mPermissionManagerInternal.getAppOpPermissionPackages( SCHEDULE_EXACT_ALARM)).thenReturn(EmptyArray.STRING); + // Initialize timestamps with arbitrary values of time + mNowElapsedTest = 12; + mNowRtcTest = 345; mInjector = new Injector(mMockContext); mService = new AlarmManagerService(mMockContext, mInjector); spyOn(mService); @@ -773,6 +776,61 @@ public final class AlarmManagerServiceTest { triggerElapsed1 - timeDelta); } + @Test + public void timeChangeBroadcastForward() throws Exception { + final long timeDelta = 12345; + // AlarmManagerService sends the broadcast if real time clock proceeds 1000ms more than boot + // time clock. + mNowRtcTest += timeDelta + 1001; + mNowElapsedTest += timeDelta; + mTestTimer.expire(TIME_CHANGED_MASK); + + verify(mMockContext) + .sendBroadcastAsUser( + argThat((intent) -> intent.getAction() == Intent.ACTION_TIME_CHANGED), + eq(UserHandle.ALL), + isNull(), + any()); + } + + @Test + public void timeChangeBroadcastBackward() throws Exception { + final long timeDelta = 12345; + // AlarmManagerService sends the broadcast if real time clock proceeds 1000ms less than boot + // time clock. + mNowRtcTest += timeDelta - 1001; + mNowElapsedTest += timeDelta; + mTestTimer.expire(TIME_CHANGED_MASK); + + verify(mMockContext) + .sendBroadcastAsUser( + argThat((intent) -> intent.getAction() == Intent.ACTION_TIME_CHANGED), + eq(UserHandle.ALL), + isNull(), + any()); + } + + @Test + public void timeChangeFilterMinorAdjustment() throws Exception { + final long timeDelta = 12345; + // AlarmManagerService does not send the broadcast if real time clock proceeds within 1000ms + // than boot time clock. + mNowRtcTest += timeDelta + 1000; + mNowElapsedTest += timeDelta; + mTestTimer.expire(TIME_CHANGED_MASK); + + mNowRtcTest += timeDelta - 1000; + mNowElapsedTest += timeDelta; + mTestTimer.expire(TIME_CHANGED_MASK); + + verify(mMockContext, never()) + .sendBroadcastAsUser( + argThat((intent) -> intent.getAction() == Intent.ACTION_TIME_CHANGED), + any(), + any(), + any()); + } + @Test public void testSingleAlarmExpiration() throws Exception { final long triggerTime = mNowElapsedTest + 5000; -- cgit v1.2.3-59-g8ed1b From ce09fdef598f11bc50b26ea50e800309ea8ce247 Mon Sep 17 00:00:00 2001 From: Adam Bookatz Date: Fri, 2 Aug 2024 15:58:31 -0700 Subject: Don't stop scheduled background user near alarm If a user's alarm is going to go off in the next X minutes, don't automatically stop a background user. There's no point automatically stopping a user that we're just going to (or want to) restart soon due to its alarm. Bug: 353734966 Bug: 330351042 Flag: android.multiuser.schedule_stop_of_background_user Test: atest FrameworksServicesTests:UserControllerTest#testScheduleStopOfBackgroundUser_rescheduleIfAlarm Change-Id: I4813804f2646279871717be723d33e6c66d10033 --- .../android/server/alarm/AlarmManagerService.java | 8 +++++ .../com/android/server/AlarmManagerInternal.java | 11 ++++++ .../java/com/android/server/am/UserController.java | 29 +++++++++++++++ .../com/android/server/am/UserControllerTest.java | 42 ++++++++++++++++++++++ 4 files changed, 90 insertions(+) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index 18ee6f2c7992..e0d1d529ccac 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -357,6 +357,7 @@ public class AlarmManagerService extends SystemService { } // TODO(b/172085676): Move inside alarm store. + @GuardedBy("mLock") private final SparseArray mNextAlarmClockForUser = new SparseArray<>(); private final SparseArray mTmpSparseAlarmClockArray = @@ -2616,6 +2617,13 @@ public class AlarmManagerService extends SystemService { mInFlightListeners.add(callback); } } + + /** @see AlarmManagerInternal#getNextAlarmTriggerTimeForUser(int) */ + @Override + public long getNextAlarmTriggerTimeForUser(@UserIdInt int userId) { + final AlarmManager.AlarmClockInfo nextAlarm = getNextAlarmClockImpl(userId); + return nextAlarm != null ? nextAlarm.getTriggerTime() : 0; + } } boolean hasUseExactAlarmInternal(String packageName, int uid) { diff --git a/services/core/java/com/android/server/AlarmManagerInternal.java b/services/core/java/com/android/server/AlarmManagerInternal.java index b7f2b8d4ffe6..191137ade3a3 100644 --- a/services/core/java/com/android/server/AlarmManagerInternal.java +++ b/services/core/java/com/android/server/AlarmManagerInternal.java @@ -17,6 +17,7 @@ package com.android.server; import android.annotation.CurrentTimeMillisLong; +import android.annotation.UserIdInt; import android.app.PendingIntent; import com.android.server.SystemClockTime.TimeConfidence; @@ -36,6 +37,16 @@ public interface AlarmManagerInternal { /** Returns true if AlarmManager is delaying alarms due to device idle. */ boolean isIdling(); + /** + * Returns the time at which the next alarm for the given user is going to trigger, or 0 if + * there is none. + * + *

This value is UTC wall clock time in milliseconds, as returned by + * {@link System#currentTimeMillis()} for example. + * @see android.app.AlarmManager.AlarmClockInfo#getTriggerTime() + */ + long getNextAlarmTriggerTimeForUser(@UserIdInt int userId); + public void removeAlarmsForUid(int uid); public void registerInFlightListener(InFlightListener callback); diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java index 30efa3e87fc6..ff64230051b6 100644 --- a/services/core/java/com/android/server/am/UserController.java +++ b/services/core/java/com/android/server/am/UserController.java @@ -131,6 +131,7 @@ import com.android.internal.util.FrameworkStatsLog; import com.android.internal.util.ObjectUtils; import com.android.internal.util.Preconditions; import com.android.internal.widget.LockPatternUtils; +import com.android.server.AlarmManagerInternal; import com.android.server.FactoryResetter; import com.android.server.FgThread; import com.android.server.LocalServices; @@ -246,6 +247,12 @@ class UserController implements Handler.Callback { // TODO(b/197344658): Increase to 10s or 15s once we have a switch-UX-is-done invocation too. private static final int USER_COMPLETED_EVENT_DELAY_MS = 5 * 1000; + /** + * If a user has an alarm in the next this many milliseconds, avoid stopping it due to + * scheduled background stopping. + */ + private static final long TIME_BEFORE_USERS_ALARM_TO_AVOID_STOPPING_MS = 60 * 60_000; // 60 mins + /** * Maximum number of users we allow to be running at a time, including system user. * @@ -2371,6 +2378,12 @@ class UserController implements Handler.Callback { void processScheduledStopOfBackgroundUser(Integer userIdInteger) { final int userId = userIdInteger; Slogf.d(TAG, "Considering stopping background user %d due to inactivity", userId); + + if (avoidStoppingUserDueToUpcomingAlarm(userId)) { + // We want this user running soon for alarm-purposes, so don't stop it now. Reschedule. + scheduleStopOfBackgroundUser(userId); + return; + } synchronized (mLock) { if (getCurrentOrTargetUserIdLU() == userId) { return; @@ -2390,6 +2403,18 @@ class UserController implements Handler.Callback { } } + /** + * Returns whether we should avoid stopping the user now due to it having an alarm set to fire + * soon. + */ + private boolean avoidStoppingUserDueToUpcomingAlarm(@UserIdInt int userId) { + final long alarmWallclockMs + = mInjector.getAlarmManagerInternal().getNextAlarmTriggerTimeForUser(userId); + return System.currentTimeMillis() < alarmWallclockMs + && (alarmWallclockMs + < System.currentTimeMillis() + TIME_BEFORE_USERS_ALARM_TO_AVOID_STOPPING_MS); + } + private void timeoutUserSwitch(UserState uss, int oldUserId, int newUserId) { TimingsTraceAndSlog t = new TimingsTraceAndSlog(TAG); t.traceBegin("timeoutUserSwitch-" + oldUserId + "-to-" + newUserId); @@ -3860,6 +3885,10 @@ class UserController implements Handler.Callback { return mPowerManagerInternal; } + AlarmManagerInternal getAlarmManagerInternal() { + return LocalServices.getService(AlarmManagerInternal.class); + } + KeyguardManager getKeyguardManager() { return mService.mContext.getSystemService(KeyguardManager.class); } diff --git a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java index dbab54b76a2e..6a15cefee32f 100644 --- a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java @@ -103,6 +103,7 @@ import android.view.Display; import androidx.test.filters.SmallTest; import com.android.internal.widget.LockPatternUtils; +import com.android.server.AlarmManagerInternal; import com.android.server.FgThread; import com.android.server.SystemService; import com.android.server.am.UserState.KeyEvictedCallback; @@ -122,6 +123,7 @@ import org.junit.Rule; import org.junit.Test; import org.mockito.ArgumentCaptor; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -727,6 +729,39 @@ public class UserControllerTest { mUserController.getRunningUsersLU()); } + /** Test scheduling stopping of background users - reschedule if user with a scheduled alarm. */ + @Test + public void testScheduleStopOfBackgroundUser_rescheduleIfAlarm() throws Exception { + mSetFlagsRule.enableFlags(android.multiuser.Flags.FLAG_SCHEDULE_STOP_OF_BACKGROUND_USER); + + mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true, + /* maxRunningUsers= */ 10, /* delayUserDataLocking= */ false, + /* backgroundUserScheduledStopTimeSecs= */ 2); + + setUpAndStartUserInBackground(TEST_USER_ID); + assertEquals(newHashSet(SYSTEM_USER_ID, TEST_USER_ID), + new HashSet<>(mUserController.getRunningUsersLU())); + + // Initially, the background user has an alarm that will fire soon. So don't stop the user. + when(mInjector.mAlarmManagerInternal.getNextAlarmTriggerTimeForUser(eq(TEST_USER_ID))) + .thenReturn(System.currentTimeMillis() + Duration.ofMinutes(2).toMillis()); + assertAndProcessScheduledStopBackgroundUser(true, TEST_USER_ID); + assertEquals(newHashSet(SYSTEM_USER_ID, TEST_USER_ID), + new HashSet<>(mUserController.getRunningUsersLU())); + + // Now, that alarm is gone and the next alarm isn't for a long time. Do stop the user. + when(mInjector.mAlarmManagerInternal.getNextAlarmTriggerTimeForUser(eq(TEST_USER_ID))) + .thenReturn(System.currentTimeMillis() + Duration.ofDays(1).toMillis()); + assertAndProcessScheduledStopBackgroundUser(true, TEST_USER_ID); + assertEquals(newHashSet(SYSTEM_USER_ID), + new HashSet<>(mUserController.getRunningUsersLU())); + + // No-one is scheduled to stop anymore. + assertAndProcessScheduledStopBackgroundUser(false, null); + verify(mInjector.mAlarmManagerInternal, never()) + .getNextAlarmTriggerTimeForUser(eq(SYSTEM_USER_ID)); + } + /** * Process queued SCHEDULED_STOP_BACKGROUND_USER_MSG message, if expected. * @param userId the user we are checking to see whether it is scheduled. @@ -1689,6 +1724,7 @@ public class UserControllerTest { private final WindowManagerService mWindowManagerMock; private final ActivityTaskManagerInternal mActivityTaskManagerInternal; private final PowerManagerInternal mPowerManagerInternal; + private final AlarmManagerInternal mAlarmManagerInternal; private final KeyguardManager mKeyguardManagerMock; private final LockPatternUtils mLockPatternUtilsMock; @@ -1711,6 +1747,7 @@ public class UserControllerTest { mActivityTaskManagerInternal = mock(ActivityTaskManagerInternal.class); mStorageManagerMock = mock(IStorageManager.class); mPowerManagerInternal = mock(PowerManagerInternal.class); + mAlarmManagerInternal = mock(AlarmManagerInternal.class); mKeyguardManagerMock = mock(KeyguardManager.class); when(mKeyguardManagerMock.isDeviceSecure(anyInt())).thenReturn(true); mLockPatternUtilsMock = mock(LockPatternUtils.class); @@ -1780,6 +1817,11 @@ public class UserControllerTest { return mPowerManagerInternal; } + @Override + AlarmManagerInternal getAlarmManagerInternal() { + return mAlarmManagerInternal; + } + @Override KeyguardManager getKeyguardManager() { return mKeyguardManagerMock; -- cgit v1.2.3-59-g8ed1b From 6434dd18826af65cf231afc73b29546e9e9ac5b7 Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Mon, 12 Aug 2024 16:07:57 -0700 Subject: Update TIME_TICK removal warning When kernel sends a time change event, we reset the TIME_TICK alarm to go at the next minute change. This requires removing and rescheduling the TIME_TICK alarm and results in spurious warning notifications from the AlarmStore. Moving the warning to removal history code, which automatically skips code paths not relevant for this warning. These are: - calling app is instant - rescheduling - user removed The warning is in place to detect erroneous removal of TIME_TICK. Flag: EXEMPT bugfix Test: Manual. Change the time and look at logcat. Bug: 356549385 Change-Id: I96b52ba509994a7bef4488fb474aa7ed93b4d141 --- .../service/java/com/android/server/alarm/AlarmManagerService.java | 5 ++++- .../service/java/com/android/server/alarm/LazyAlarmStore.java | 7 ------- 2 files changed, 4 insertions(+), 8 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index ba66ff72bfdd..dfe740ac6fbb 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -319,7 +319,7 @@ public class AlarmManagerService extends SystemService { */ int mSystemUiUid; - static boolean isTimeTickAlarm(Alarm a) { + private static boolean isTimeTickAlarm(Alarm a) { return a.uid == Process.SYSTEM_UID && TIME_TICK_TAG.equals(a.listenerTag); } @@ -3947,6 +3947,9 @@ public class AlarmManagerService extends SystemService { if (!RemovedAlarm.isLoggable(reason)) { continue; } + if (isTimeTickAlarm(removed)) { + Slog.wtf(TAG, "Removed TIME_TICK alarm"); + } RingBuffer bufferForUid = mRemovalHistory.get(removed.uid); if (bufferForUid == null) { bufferForUid = new RingBuffer<>(RemovedAlarm.class, REMOVAL_HISTORY_SIZE_PER_UID); diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/LazyAlarmStore.java b/apex/jobscheduler/service/java/com/android/server/alarm/LazyAlarmStore.java index 0073335a1332..020b510269f8 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/LazyAlarmStore.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/LazyAlarmStore.java @@ -17,11 +17,9 @@ package com.android.server.alarm; import static com.android.server.alarm.AlarmManagerService.dumpAlarmList; -import static com.android.server.alarm.AlarmManagerService.isTimeTickAlarm; import android.app.AlarmManager; import android.util.IndentingPrintWriter; -import android.util.Slog; import android.util.proto.ProtoOutputStream; import com.android.internal.annotations.VisibleForTesting; @@ -88,11 +86,6 @@ public class LazyAlarmStore implements AlarmStore { if (removed.alarmClock != null && mOnAlarmClockRemoved != null) { mOnAlarmClockRemoved.run(); } - if (isTimeTickAlarm(removed)) { - // This code path is not invoked when delivering alarms, only when removing - // alarms due to the caller cancelling it or getting uninstalled, etc. - Slog.wtf(TAG, "Removed TIME_TICK alarm"); - } removedAlarms.add(removed); } } -- cgit v1.2.3-59-g8ed1b From 00f0fd9d93957cce014590e6e5f132fd85a5b948 Mon Sep 17 00:00:00 2001 From: Jahdiel Alvarez Date: Wed, 8 May 2024 15:54:41 -0700 Subject: Refactor JobScheduler to handle user switch via onUserSwitching method JobSchedulerService will handle SystemService's onUserSwitching method in order to remove the previous user from its internal started users array in the case the previous user is stopped during user switch. With respect to the stopping previous user apps feature, this will prevent JobSchedulerService from restarting packages that were early killed during the user switch system event. Test: Manually run user switch on device Test: atest frameworks/base/services/tests/mockingservicestests/src/com/android/server/job Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/job Test: atest CtsJobSchedulerTestCases Bug: 323200731 Flag: com.android.server.job.remove_user_during_user_switch Change-Id: Ie2813482db061c8b2f21ce0731bbf5cf23897158 --- apex/jobscheduler/service/aconfig/job.aconfig | 7 +++++++ .../java/com/android/server/job/JobSchedulerService.java | 14 ++++++++++++++ 2 files changed, 21 insertions(+) (limited to 'apex') diff --git a/apex/jobscheduler/service/aconfig/job.aconfig b/apex/jobscheduler/service/aconfig/job.aconfig index e8568651eeaa..613b7842ae3a 100644 --- a/apex/jobscheduler/service/aconfig/job.aconfig +++ b/apex/jobscheduler/service/aconfig/job.aconfig @@ -58,3 +58,10 @@ flag { purpose: PURPOSE_BUGFIX } } + +flag { + name: "remove_user_during_user_switch" + namespace: "backstage_power" + description: "Remove started user if user will be stopped due to user switch" + bug: "321598070" +} diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java index 5f2b01a7304a..3d25ed5aa2b7 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -1668,6 +1668,20 @@ public class JobSchedulerService extends com.android.server.SystemService } } + @Override + public void onUserSwitching(@Nullable TargetUser from, @NonNull TargetUser to) { + if (!Flags.removeUserDuringUserSwitch() + || from == null + || !mActivityManagerInternal.isEarlyPackageKillEnabledForUserSwitch( + from.getUserIdentifier(), + to.getUserIdentifier())) { + return; + } + synchronized (mLock) { + mStartedUsers = ArrayUtils.removeInt(mStartedUsers, from.getUserIdentifier()); + } + } + @Override public void onUserStopping(@NonNull TargetUser user) { synchronized (mLock) { -- cgit v1.2.3-59-g8ed1b From d60a73d94933203929bc952fbd44f5edfa9514be Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Fri, 16 Aug 2024 15:21:23 +0100 Subject: Clean up fully rolled out DisallowUserControlBgUsageFix Flag was rolled out in Android V. Bug: 335663055 Test: TH Flag: EXEMPT flag cleanup Change-Id: I8699ac50ae7538a2de98e0e892236eef5b0bdd89 --- .../java/com/android/server/usage/AppStandbyController.java | 6 ++---- core/java/android/app/admin/flags/flags.aconfig | 10 ---------- .../android/settingslib/fuelgauge/PowerAllowlistBackend.java | 10 ++++------ .../android/server/devicepolicy/PolicyEnforcerCallbacks.java | 4 +--- 4 files changed, 7 insertions(+), 23 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/usage/AppStandbyController.java b/apex/jobscheduler/service/java/com/android/server/usage/AppStandbyController.java index d92351de3aa1..c9d340757c6b 100644 --- a/apex/jobscheduler/service/java/com/android/server/usage/AppStandbyController.java +++ b/apex/jobscheduler/service/java/com/android/server/usage/AppStandbyController.java @@ -1989,10 +1989,8 @@ public class AppStandbyController mAdminProtectedPackages.put(userId, packageNames); } } - if (android.app.admin.flags.Flags.disallowUserControlBgUsageFix()) { - if (!Flags.avoidIdleCheck() || mInjector.getBootPhase() >= PHASE_BOOT_COMPLETED) { - postCheckIdleStates(userId); - } + if (!Flags.avoidIdleCheck() || mInjector.getBootPhase() >= PHASE_BOOT_COMPLETED) { + postCheckIdleStates(userId); } } diff --git a/core/java/android/app/admin/flags/flags.aconfig b/core/java/android/app/admin/flags/flags.aconfig index 56f47922b078..37cfec04cbb4 100644 --- a/core/java/android/app/admin/flags/flags.aconfig +++ b/core/java/android/app/admin/flags/flags.aconfig @@ -216,16 +216,6 @@ flag { } } -flag { - name: "disallow_user_control_bg_usage_fix" - namespace: "enterprise" - description: "Make DPM.setUserControlDisabledPackages() ensure background usage is allowed" - bug: "326031059" - metadata { - purpose: PURPOSE_BUGFIX - } -} - flag { name: "disallow_user_control_stopped_state_fix" namespace: "enterprise" diff --git a/packages/SettingsLib/src/com/android/settingslib/fuelgauge/PowerAllowlistBackend.java b/packages/SettingsLib/src/com/android/settingslib/fuelgauge/PowerAllowlistBackend.java index 4f2329bb75c2..47a08eb9a72b 100644 --- a/packages/SettingsLib/src/com/android/settingslib/fuelgauge/PowerAllowlistBackend.java +++ b/packages/SettingsLib/src/com/android/settingslib/fuelgauge/PowerAllowlistBackend.java @@ -136,12 +136,10 @@ public class PowerAllowlistBackend { return true; } - if (android.app.admin.flags.Flags.disallowUserControlBgUsageFix()) { - // App is subject to DevicePolicyManager.setUserControlDisabledPackages() policy. - final int userId = UserHandle.getUserId(uid); - if (mAppContext.getPackageManager().isPackageStateProtected(pkg, userId)) { - return true; - } + // App is subject to DevicePolicyManager.setUserControlDisabledPackages() policy. + final int userId = UserHandle.getUserId(uid); + if (mAppContext.getPackageManager().isPackageStateProtected(pkg, userId)) { + return true; } return false; diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyEnforcerCallbacks.java b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyEnforcerCallbacks.java index e1cb37dbeef5..8068d46d6a9d 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyEnforcerCallbacks.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyEnforcerCallbacks.java @@ -238,9 +238,7 @@ final class PolicyEnforcerCallbacks { } for (int user : resolveUsers(userId)) { - if (Flags.disallowUserControlBgUsageFix()) { - setBgUsageAppOp(packages, pmi, user, appOpsManager); - } + setBgUsageAppOp(packages, pmi, user, appOpsManager); if (Flags.disallowUserControlStoppedStateFix()) { for (String packageName : packages) { pmi.setPackageStoppedState(packageName, false, user); -- cgit v1.2.3-59-g8ed1b From 70f043152984a78b90038ab4c651b9a9c48bdc91 Mon Sep 17 00:00:00 2001 From: Xin Guan Date: Thu, 29 Aug 2024 00:13:08 +0000 Subject: Fix the wrong process state for statsd logging. The process state haven't been updated yet, which causes the wrong attribution of the job metrics per process state. Bug: 361308212 Flag: com.android.server.job.use_correct_process_state_for_logging Test: trace/logging. Change-Id: If1a135f16f277688f59f254809e0ad57b1ba8c12 --- apex/jobscheduler/service/aconfig/job.aconfig | 10 ++++++++++ .../com/android/server/job/JobServiceContext.java | 19 +++++++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/aconfig/job.aconfig b/apex/jobscheduler/service/aconfig/job.aconfig index 613b7842ae3a..e5389b4f96fb 100644 --- a/apex/jobscheduler/service/aconfig/job.aconfig +++ b/apex/jobscheduler/service/aconfig/job.aconfig @@ -65,3 +65,13 @@ flag { description: "Remove started user if user will be stopped due to user switch" bug: "321598070" } + +flag { + name: "use_correct_process_state_for_logging" + namespace: "backstage_power" + description: "Use correct process state for statsd logging" + bug: "361308212" + metadata { + purpose: PURPOSE_BUGFIX + } +} diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java index d65a66c83fcc..be8e304a8101 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java @@ -473,6 +473,14 @@ public final class JobServiceContext implements ServiceConnection { mInitialDownloadedBytesFromCalling = TrafficStats.getUidRxBytes(job.getUid()); mInitialUploadedBytesFromCalling = TrafficStats.getUidTxBytes(job.getUid()); + int procState = mService.getUidProcState(job.getUid()); + if (Flags.useCorrectProcessStateForLogging() + && procState > ActivityManager.PROCESS_STATE_TRANSIENT_BACKGROUND) { + // Try to get the latest proc state from AMS, there might be some delay + // for the proc states worse than TRANSIENT_BACKGROUND. + procState = mActivityManagerInternal.getUidProcessState(job.getUid()); + } + FrameworkStatsLog.write(FrameworkStatsLog.SCHEDULED_JOB_STATE_CHANGED, job.isProxyJob() ? new int[]{sourceUid, job.getUid()} : new int[]{sourceUid}, // Given that the source tag is set by the calling app, it should be connected @@ -517,7 +525,7 @@ public final class JobServiceContext implements ServiceConnection { job.getEstimatedNetworkDownloadBytes(), job.getEstimatedNetworkUploadBytes(), job.getWorkCount(), - ActivityManager.processStateAmToProto(mService.getUidProcState(job.getUid())), + ActivityManager.processStateAmToProto(procState), job.getNamespaceHash(), /* system_measured_source_download_bytes */ 0, /* system_measured_source_upload_bytes */ 0, @@ -1528,6 +1536,13 @@ public final class JobServiceContext implements ServiceConnection { mJobPackageTracker.noteInactive(completedJob, loggingInternalStopReason, loggingDebugReason); final int sourceUid = completedJob.getSourceUid(); + int procState = mService.getUidProcState(completedJob.getUid()); + if (Flags.useCorrectProcessStateForLogging() + && procState > ActivityManager.PROCESS_STATE_TRANSIENT_BACKGROUND) { + // Try to get the latest proc state from AMS, there might be some delay + // for the proc states worse than TRANSIENT_BACKGROUND. + procState = mActivityManagerInternal.getUidProcessState(completedJob.getUid()); + } FrameworkStatsLog.write(FrameworkStatsLog.SCHEDULED_JOB_STATE_CHANGED, completedJob.isProxyJob() ? new int[]{sourceUid, completedJob.getUid()} : new int[]{sourceUid}, @@ -1573,7 +1588,7 @@ public final class JobServiceContext implements ServiceConnection { completedJob.getEstimatedNetworkUploadBytes(), completedJob.getWorkCount(), ActivityManager - .processStateAmToProto(mService.getUidProcState(completedJob.getUid())), + .processStateAmToProto(procState), completedJob.getNamespaceHash(), TrafficStats.getUidRxBytes(completedJob.getSourceUid()) - mInitialDownloadedBytesFromSource, -- cgit v1.2.3-59-g8ed1b From 084121d17616ee3e1875ef0c38fd3c688ab0cf0a Mon Sep 17 00:00:00 2001 From: nongyujun Date: Mon, 2 Sep 2024 03:08:54 +0000 Subject: Add --proto option in jobscheduler dumpHelp. Flag: EXEMPT log only update Test: manual Bug: 363897064 Change-Id: I2970bc89fb3e12b328b06cd254308ffe431b8249 --- .../service/java/com/android/server/job/JobSchedulerService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'apex') diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java index 3d25ed5aa2b7..97c6e25eeb75 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -5808,9 +5808,10 @@ public class JobSchedulerService extends com.android.server.SystemService static void dumpHelp(PrintWriter pw) { pw.println("Job Scheduler (jobscheduler) dump options:"); - pw.println(" [-h] [package] ..."); + pw.println(" [-h] [package] [--proto] ..."); pw.println(" -h: print this help"); pw.println(" [package] is an optional package name to limit the output to."); + pw.println(" --proto: output dump in protocol buffer format."); } /** Sort jobs by caller UID, then by Job ID. */ -- cgit v1.2.3-59-g8ed1b From aa9151e38631694ea04bf73ad6077df84d56e82e Mon Sep 17 00:00:00 2001 From: Sanath Kumar Date: Thu, 29 Aug 2024 23:16:03 -0500 Subject: JobScheduler: Add evaluation patch for detecting and logging empty jobs This patch implements a low-risk evaluation to detect and log empty jobs via tracing within the job scheduler service. It uses a new hidden API in IJobCallback.aidl. The atrace trace, combined with ScheduledJobStateChanged, will help analyze empty job characteristics. Bug: 349688611 Flag: android.app.job.cleanup_empty_jobs Test: Existing CTS and unit tests pass. Ran a test JobScheduler app and checked the perfetto trace for empty jobs Change-Id: I3261bd14be7f62be664a31a4dfb9baf044c33957 --- apex/jobscheduler/framework/aconfig/job.aconfig | 7 + .../java/android/app/job/IJobCallback.aidl | 8 ++ .../java/android/app/job/JobParameters.java | 123 +++++++++++++++++ .../java/android/app/job/JobServiceEngine.java | 9 ++ .../com/android/server/job/JobServiceContext.java | 36 +++++ .../com/android/server/job/JobParametersTest.java | 153 +++++++++++++++++++++ 6 files changed, 336 insertions(+) create mode 100644 services/tests/mockingservicestests/src/com/android/server/job/JobParametersTest.java (limited to 'apex') diff --git a/apex/jobscheduler/framework/aconfig/job.aconfig b/apex/jobscheduler/framework/aconfig/job.aconfig index 80db264d0f44..5f5507587f72 100644 --- a/apex/jobscheduler/framework/aconfig/job.aconfig +++ b/apex/jobscheduler/framework/aconfig/job.aconfig @@ -23,3 +23,10 @@ flag { description: "Introduce a new RUN_BACKUP_JOBS permission and exemption logic allowing for longer running jobs for apps whose primary purpose is to backup or sync content." bug: "318731461" } + +flag { + name: "cleanup_empty_jobs" + namespace: "backstage_power" + description: "Enables automatic cancellation of jobs due to leaked JobParameters, reducing unnecessary battery drain and improving system efficiency. This includes logging and traces for better issue diagnosis." + bug: "349688611" +} diff --git a/apex/jobscheduler/framework/java/android/app/job/IJobCallback.aidl b/apex/jobscheduler/framework/java/android/app/job/IJobCallback.aidl index 96494ec28204..11d17ca749b7 100644 --- a/apex/jobscheduler/framework/java/android/app/job/IJobCallback.aidl +++ b/apex/jobscheduler/framework/java/android/app/job/IJobCallback.aidl @@ -85,6 +85,14 @@ interface IJobCallback { */ @UnsupportedAppUsage void jobFinished(int jobId, boolean reschedule); + + /* + * Inform JobScheduler to force finish this job because the client has lost + * the job handle. jobFinished can no longer be called from the client. + * @param jobId Unique integer used to identify this job + */ + void forceJobFinished(int jobId); + /* * Inform JobScheduler of a change in the estimated transfer payload. * diff --git a/apex/jobscheduler/framework/java/android/app/job/JobParameters.java b/apex/jobscheduler/framework/java/android/app/job/JobParameters.java index e833bb95a302..52a761f8d486 100644 --- a/apex/jobscheduler/framework/java/android/app/job/JobParameters.java +++ b/apex/jobscheduler/framework/java/android/app/job/JobParameters.java @@ -34,15 +34,21 @@ import android.os.Parcel; import android.os.Parcelable; import android.os.PersistableBundle; import android.os.RemoteException; +import android.system.SystemCleaner; +import android.util.Log; + +import com.android.internal.annotations.VisibleForTesting; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.lang.ref.Cleaner; /** * Contains the parameters used to configure/identify your job. You do not create this object * yourself, instead it is handed in to your application by the System. */ public class JobParameters implements Parcelable { + private static final String TAG = "JobParameters"; /** @hide */ public static final int INTERNAL_STOP_REASON_UNKNOWN = -1; @@ -306,6 +312,10 @@ public class JobParameters implements Parcelable { private int mStopReason = STOP_REASON_UNDEFINED; private int mInternalStopReason = INTERNAL_STOP_REASON_UNKNOWN; private String debugStopReason; // Human readable stop reason for debugging. + @Nullable + private JobCleanupCallback mJobCleanupCallback; + @Nullable + private Cleaner.Cleanable mCleanable; /** @hide */ public JobParameters(IBinder callback, String namespace, int jobId, PersistableBundle extras, @@ -326,6 +336,8 @@ public class JobParameters implements Parcelable { this.mTriggeredContentAuthorities = triggeredContentAuthorities; this.mNetwork = network; this.mJobNamespace = namespace; + this.mJobCleanupCallback = null; + this.mCleanable = null; } /** @@ -597,6 +609,8 @@ public class JobParameters implements Parcelable { mStopReason = in.readInt(); mInternalStopReason = in.readInt(); debugStopReason = in.readString(); + mJobCleanupCallback = null; + mCleanable = null; } /** @hide */ @@ -612,6 +626,54 @@ public class JobParameters implements Parcelable { this.debugStopReason = debugStopReason; } + /** @hide */ + public void initCleaner(JobCleanupCallback jobCleanupCallback) { + mJobCleanupCallback = jobCleanupCallback; + mCleanable = SystemCleaner.cleaner().register(this, mJobCleanupCallback); + } + + /** + * Lazy initialize the cleaner and enable it + * + * @hide + */ + public void enableCleaner() { + if (mJobCleanupCallback == null) { + initCleaner(new JobCleanupCallback(IJobCallback.Stub.asInterface(callback), jobId)); + } + mJobCleanupCallback.enableCleaner(); + } + + /** + * Disable the cleaner from running and unregister it + * + * @hide + */ + public void disableCleaner() { + if (mJobCleanupCallback != null) { + mJobCleanupCallback.disableCleaner(); + if (mCleanable != null) { + mCleanable.clean(); + mCleanable = null; + } + mJobCleanupCallback = null; + } + } + + /** @hide */ + @VisibleForTesting + @Nullable + public Cleaner.Cleanable getCleanable() { + return mCleanable; + } + + /** @hide */ + @VisibleForTesting + @Nullable + public JobCleanupCallback getJobCleanupCallback() { + return mJobCleanupCallback; + } + @Override public int describeContents() { return 0; @@ -647,6 +709,67 @@ public class JobParameters implements Parcelable { dest.writeString(debugStopReason); } + /** + * JobCleanupCallback is used track JobParameters leak. If the job is started + * and jobFinish is not called at the time of garbage collection of JobParameters + * instance, it is considered a job leak. Force finish the job. + * + * @hide + */ + public static class JobCleanupCallback implements Runnable { + private final IJobCallback mCallback; + private final int mJobId; + private boolean mIsCleanerEnabled; + + public JobCleanupCallback( + IJobCallback callback, + int jobId) { + mCallback = callback; + mJobId = jobId; + mIsCleanerEnabled = false; + } + + /** + * Check if the cleaner is enabled + * + * @hide + */ + public boolean isCleanerEnabled() { + return mIsCleanerEnabled; + } + + /** + * Enable the cleaner to detect JobParameter leak + * + * @hide + */ + public void enableCleaner() { + mIsCleanerEnabled = true; + } + + /** + * Disable the cleaner from running. + * + * @hide + */ + public void disableCleaner() { + mIsCleanerEnabled = false; + } + + /** @hide */ + @Override + public void run() { + if (!isCleanerEnabled()) { + return; + } + try { + mCallback.forceJobFinished(mJobId); + } catch (Exception e) { + Log.wtf(TAG, "Could not destroy running job", e); + } + } + } + public static final @android.annotation.NonNull Creator CREATOR = new Creator() { @Override public JobParameters createFromParcel(Parcel in) { diff --git a/apex/jobscheduler/framework/java/android/app/job/JobServiceEngine.java b/apex/jobscheduler/framework/java/android/app/job/JobServiceEngine.java index 79d87edff9b2..5f80c52388b4 100644 --- a/apex/jobscheduler/framework/java/android/app/job/JobServiceEngine.java +++ b/apex/jobscheduler/framework/java/android/app/job/JobServiceEngine.java @@ -165,7 +165,13 @@ public abstract class JobServiceEngine { case MSG_EXECUTE_JOB: { final JobParameters params = (JobParameters) msg.obj; try { + if (Flags.cleanupEmptyJobs()) { + params.enableCleaner(); + } boolean workOngoing = JobServiceEngine.this.onStartJob(params); + if (Flags.cleanupEmptyJobs() && !workOngoing) { + params.disableCleaner(); + } ackStartMessage(params, workOngoing); } catch (Exception e) { Log.e(TAG, "Error while executing job: " + params.getJobId()); @@ -190,6 +196,9 @@ public abstract class JobServiceEngine { IJobCallback callback = params.getCallback(); if (callback != null) { try { + if (Flags.cleanupEmptyJobs()) { + params.disableCleaner(); + } callback.jobFinished(params.getJobId(), needsReschedule); } catch (RemoteException e) { Log.e(TAG, "Error reporting job finish to system: binder has gone" + diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java index be8e304a8101..ee246d84997f 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java @@ -129,6 +129,8 @@ public final class JobServiceContext implements ServiceConnection { private static final String[] VERB_STRINGS = { "VERB_BINDING", "VERB_STARTING", "VERB_EXECUTING", "VERB_STOPPING", "VERB_FINISHED" }; + private static final String TRACE_JOB_FORCE_FINISHED_PREFIX = "forceJobFinished:"; + private static final String TRACE_JOB_FORCE_FINISHED_DELIMITER = "#"; // States that a job occupies while interacting with the client. static final int VERB_BINDING = 0; @@ -291,6 +293,11 @@ public final class JobServiceContext implements ServiceConnection { doJobFinished(this, jobId, reschedule); } + @Override + public void forceJobFinished(int jobId) { + doForceJobFinished(this, jobId); + } + @Override public void updateEstimatedNetworkBytes(int jobId, JobWorkItem item, long downloadBytes, long uploadBytes) { @@ -762,6 +769,35 @@ public final class JobServiceContext implements ServiceConnection { } } + /** + * This method just adds traces to evaluate jobs that leak jobparameters at the client. + * It does not stop the job. + */ + void doForceJobFinished(JobCallback cb, int jobId) { + final long ident = Binder.clearCallingIdentity(); + try { + final JobStatus executing; + synchronized (mLock) { + // not the current job, presumably it has finished in some way already + if (!verifyCallerLocked(cb)) { + return; + } + + executing = getRunningJobLocked(); + } + if (executing != null && jobId == executing.getJobId()) { + final StringBuilder stateSuffix = new StringBuilder(); + stateSuffix.append(TRACE_JOB_FORCE_FINISHED_PREFIX); + stateSuffix.append(executing.getBatteryName()); + stateSuffix.append(TRACE_JOB_FORCE_FINISHED_DELIMITER); + stateSuffix.append(executing.getJobId()); + Trace.instant(Trace.TRACE_TAG_POWER, stateSuffix.toString()); + } + } finally { + Binder.restoreCallingIdentity(ident); + } + } + private void doAcknowledgeGetTransferredDownloadBytesMessage(JobCallback cb, int jobId, int workId, @BytesLong long transferredBytes) { // TODO(255393346): Make sure apps call this appropriately and monitor for abuse diff --git a/services/tests/mockingservicestests/src/com/android/server/job/JobParametersTest.java b/services/tests/mockingservicestests/src/com/android/server/job/JobParametersTest.java new file mode 100644 index 000000000000..c8e4f89aaee6 --- /dev/null +++ b/services/tests/mockingservicestests/src/com/android/server/job/JobParametersTest.java @@ -0,0 +1,153 @@ +/* + * Copyright (C) 2024 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.job; + +import static android.app.job.Flags.FLAG_CLEANUP_EMPTY_JOBS; + +import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession; +import static com.android.dx.mockito.inline.extended.ExtendedMockito.when; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.ArgumentMatchers.any; + +import android.app.job.IJobCallback; +import android.app.job.JobParameters; +import android.net.Uri; +import android.os.Parcel; +import android.platform.test.annotations.RequiresFlagsEnabled; +import android.platform.test.flag.junit.CheckFlagsRule; +import android.platform.test.flag.junit.DeviceFlagsValueProvider; +import android.platform.test.flag.junit.SetFlagsRule; + +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoSession; +import org.mockito.quality.Strictness; + +public class JobParametersTest { + private static final String TAG = JobParametersTest.class.getSimpleName(); + private static final int TEST_JOB_ID_1 = 123; + private static final String TEST_NAMESPACE = "TEST_NAMESPACE"; + private static final String TEST_DEBUG_STOP_REASON = "TEST_DEBUG_STOP_REASON"; + @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); + + @Rule + public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule(); + + private MockitoSession mMockingSession; + @Mock private Parcel mMockParcel; + @Mock private IJobCallback.Stub mMockJobCallbackStub; + + @Before + public void setUp() throws Exception { + mMockingSession = + mockitoSession().initMocks(this).strictness(Strictness.LENIENT).startMocking(); + } + + @After + public void tearDown() { + if (mMockingSession != null) { + mMockingSession.finishMocking(); + } + + when(mMockParcel.readInt()) + .thenReturn(TEST_JOB_ID_1) // Job ID + .thenReturn(0) // No clip data + .thenReturn(0) // No deadline expired + .thenReturn(0) // No network + .thenReturn(0) // No stop reason + .thenReturn(0); // Internal stop reason + when(mMockParcel.readString()) + .thenReturn(TEST_NAMESPACE) // Job namespace + .thenReturn(TEST_DEBUG_STOP_REASON); // Debug stop reason + when(mMockParcel.readPersistableBundle()).thenReturn(null); + when(mMockParcel.readBundle()).thenReturn(null); + when(mMockParcel.readStrongBinder()).thenReturn(mMockJobCallbackStub); + when(mMockParcel.readBoolean()) + .thenReturn(false) // expedited + .thenReturn(false); // user initiated + when(mMockParcel.createTypedArray(any())).thenReturn(new Uri[0]); + when(mMockParcel.createStringArray()).thenReturn(new String[0]); + } + + /** + * Test to verify that the JobParameters created using Non-Parcelable constructor has not + * cleaner attached + */ + @Test + public void testJobParametersNonParcelableConstructor_noCleaner() { + JobParameters jobParameters = + new JobParameters( + null, + TEST_NAMESPACE, + TEST_JOB_ID_1, + null, + null, + null, + 0, + false, + false, + false, + null, + null, + null); + + // Verify that cleaner is not registered + assertThat(jobParameters.getCleanable()).isNull(); + assertThat(jobParameters.getJobCleanupCallback()).isNull(); + } + + /** + * Test to verify that the JobParameters created using Parcelable constructor has not cleaner + * attached + */ + @Test + public void testJobParametersParcelableConstructor_noCleaner() { + JobParameters jobParameters = JobParameters.CREATOR.createFromParcel(mMockParcel); + + // Verify that cleaner is not registered + assertThat(jobParameters.getCleanable()).isNull(); + assertThat(jobParameters.getJobCleanupCallback()).isNull(); + } + + /** Test to verify that the JobParameters Cleaner is disabled */ + @RequiresFlagsEnabled(FLAG_CLEANUP_EMPTY_JOBS) + @Test + public void testCleanerWithLeakedJobCleanerDisabled_flagCleanupEmptyJobsEnabled() { + // Inject real JobCallbackCleanup + JobParameters jobParameters = JobParameters.CREATOR.createFromParcel(mMockParcel); + + // Enable the cleaner + jobParameters.enableCleaner(); + + // Verify the cleaner is enabled + assertThat(jobParameters.getCleanable()).isNotNull(); + assertThat(jobParameters.getJobCleanupCallback()).isNotNull(); + assertThat(jobParameters.getJobCleanupCallback().isCleanerEnabled()).isTrue(); + + // Disable the cleaner + jobParameters.disableCleaner(); + + // Verify the cleaner is disabled + assertThat(jobParameters.getCleanable()).isNull(); + assertThat(jobParameters.getJobCleanupCallback()).isNull(); + } +} -- cgit v1.2.3-59-g8ed1b