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 --- .../java/com/android/server/alarm/AlarmManagerService.java | 10 +++++++--- 1 file changed, 7 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"); } -- cgit v1.2.3-59-g8ed1b