From d77b127e37599a6606f7ff16cb96a3999a9fe3ab Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Wed, 28 Apr 2021 14:47:54 -0700 Subject: [PATCH] Fixing empty PendingAlarmInfo after alarm store changes The alarm store implementation might change, leaving the pointer passed to MetricsHelper with an empty instance. Also reordered some code to reduce unnecessary locking. Test: Manually, run: adb shell device_config put alarm_manager lazy_batching false Then: ./out/host/linux-x86/bin/statsd_testdrive 10106 should show non-zero values. Bug: 177556103 Change-Id: I55b4318d14632f227adfd79f7a08bb50af98f03a --- .../server/alarm/AlarmManagerService.java | 61 ++++++++++--------- .../android/server/alarm/MetricsHelper.java | 54 +++++++++------- 2 files changed, 62 insertions(+), 53 deletions(-) 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 54e47cf2f13d3..0e362759ef4cc 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -205,12 +205,11 @@ public class AlarmManagerService extends SystemService { final LocalLog mLog = new LocalLog(TAG); AppOpsManager mAppOps; - IAppOpsService mAppOpsService; DeviceIdleInternal mLocalDeviceIdleController; private UsageStatsManagerInternal mUsageStatsManagerInternal; private ActivityManagerInternal mActivityManagerInternal; private PackageManagerInternal mPackageManagerInternal; - private PermissionManagerServiceInternal mLocalPermissionManager; + private volatile PermissionManagerServiceInternal mLocalPermissionManager; final Object mLock = new Object(); @@ -1506,7 +1505,7 @@ public class AlarmManagerService extends SystemService { @Override public void onStart() { mInjector.init(); - mMetricsHelper = new MetricsHelper(getContext()); + mMetricsHelper = new MetricsHelper(getContext(), mLock); mListenerDeathRecipient = new IBinder.DeathRecipient() { @Override @@ -1630,40 +1629,14 @@ public class AlarmManagerService extends SystemService { if (phase == PHASE_SYSTEM_SERVICES_READY) { synchronized (mLock) { mConstants.start(); + mAppOps = (AppOpsManager) getContext().getSystemService(Context.APP_OPS_SERVICE); - mAppOpsService = mInjector.getAppOpsService(); - try { - mAppOpsService.startWatchingMode(AppOpsManager.OP_SCHEDULE_EXACT_ALARM, null, - new IAppOpsCallback.Stub() { - @Override - public void opChanged(int op, int uid, String packageName) - throws RemoteException { - if (op != AppOpsManager.OP_SCHEDULE_EXACT_ALARM) { - return; - } - if (!hasScheduleExactAlarmInternal(packageName, uid)) { - mHandler.obtainMessage(AlarmHandler.REMOVE_EXACT_ALARMS, - uid, 0, packageName).sendToTarget(); - } - } - }); - } catch (RemoteException e) { - } - mMetricsHelper.registerPuller(mAlarmStore); mLocalDeviceIdleController = LocalServices.getService(DeviceIdleInternal.class); mUsageStatsManagerInternal = LocalServices.getService(UsageStatsManagerInternal.class); - mLocalPermissionManager = LocalServices.getService( - PermissionManagerServiceInternal.class); - refreshExactAlarmCandidates(); - - AppStandbyInternal appStandbyInternal = - LocalServices.getService(AppStandbyInternal.class); - appStandbyInternal.addListener(new AppStandbyTracker()); - mAppStateTracker = (AppStateTrackerImpl) LocalServices.getService(AppStateTracker.class); mAppStateTracker.addListener(mForceAppStandbyListener); @@ -1671,6 +1644,34 @@ public class AlarmManagerService extends SystemService { mClockReceiver.scheduleTimeTickEvent(); mClockReceiver.scheduleDateChangedEvent(); } + IAppOpsService iAppOpsService = mInjector.getAppOpsService(); + try { + iAppOpsService.startWatchingMode(AppOpsManager.OP_SCHEDULE_EXACT_ALARM, null, + new IAppOpsCallback.Stub() { + @Override + public void opChanged(int op, int uid, String packageName) + throws RemoteException { + if (op != AppOpsManager.OP_SCHEDULE_EXACT_ALARM) { + return; + } + if (!hasScheduleExactAlarmInternal(packageName, uid)) { + mHandler.obtainMessage(AlarmHandler.REMOVE_EXACT_ALARMS, + uid, 0, packageName).sendToTarget(); + } + } + }); + } catch (RemoteException e) { + } + + mLocalPermissionManager = LocalServices.getService( + PermissionManagerServiceInternal.class); + refreshExactAlarmCandidates(); + + AppStandbyInternal appStandbyInternal = + LocalServices.getService(AppStandbyInternal.class); + appStandbyInternal.addListener(new AppStandbyTracker()); + + mMetricsHelper.registerPuller(() -> mAlarmStore); } } diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/MetricsHelper.java b/apex/jobscheduler/service/java/com/android/server/alarm/MetricsHelper.java index a8cf7b2ae3f8d..2dc131c0bda9d 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/MetricsHelper.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/MetricsHelper.java @@ -30,17 +30,21 @@ import android.os.SystemClock; import com.android.internal.os.BackgroundThread; import com.android.internal.util.FrameworkStatsLog; +import java.util.function.Supplier; + /** * A helper class to write logs to statsd. */ class MetricsHelper { - private Context mContext; + private final Context mContext; + private final Object mLock; - MetricsHelper(Context context) { + MetricsHelper(Context context, Object lock) { mContext = context; + mLock = lock; } - void registerPuller(AlarmStore alarmStore) { + void registerPuller(Supplier alarmStoreSupplier) { final StatsManager statsManager = mContext.getSystemService(StatsManager.class); statsManager.setPullAtomCallback(FrameworkStatsLog.PENDING_ALARM_INFO, null, BackgroundThread.getExecutor(), (atomTag, data) -> { @@ -48,26 +52,30 @@ class MetricsHelper { throw new UnsupportedOperationException("Unknown tag" + atomTag); } final long now = SystemClock.elapsedRealtime(); - data.add(FrameworkStatsLog.buildStatsEvent(atomTag, - alarmStore.size(), - alarmStore.getCount(a -> a.windowLength == 0), - alarmStore.getCount(a -> a.wakeup), - alarmStore.getCount( - a -> (a.flags & AlarmManager.FLAG_ALLOW_WHILE_IDLE) != 0), - alarmStore.getCount(a -> (a.flags & AlarmManager.FLAG_PRIORITIZE) != 0), - alarmStore.getCount(a -> (a.operation != null - && a.operation.isForegroundService())), - alarmStore.getCount( - a -> (a.operation != null && a.operation.isActivity())), - alarmStore.getCount( - a -> (a.operation != null && a.operation.isService())), - alarmStore.getCount(a -> (a.listener != null)), - alarmStore.getCount( - a -> (a.getRequestedElapsed() > now + INDEFINITE_DELAY)), - alarmStore.getCount(a -> (a.repeatInterval != 0)), - alarmStore.getCount(a -> (a.alarmClock != null)) - )); - return StatsManager.PULL_SUCCESS; + synchronized (mLock) { + final AlarmStore alarmStore = alarmStoreSupplier.get(); + data.add(FrameworkStatsLog.buildStatsEvent(atomTag, + alarmStore.size(), + alarmStore.getCount(a -> a.windowLength == 0), + alarmStore.getCount(a -> a.wakeup), + alarmStore.getCount( + a -> (a.flags & AlarmManager.FLAG_ALLOW_WHILE_IDLE) != 0), + alarmStore.getCount( + a -> (a.flags & AlarmManager.FLAG_PRIORITIZE) != 0), + alarmStore.getCount(a -> (a.operation != null + && a.operation.isForegroundService())), + alarmStore.getCount( + a -> (a.operation != null && a.operation.isActivity())), + alarmStore.getCount( + a -> (a.operation != null && a.operation.isService())), + alarmStore.getCount(a -> (a.listener != null)), + alarmStore.getCount( + a -> (a.getRequestedElapsed() > now + INDEFINITE_DELAY)), + alarmStore.getCount(a -> (a.repeatInterval != 0)), + alarmStore.getCount(a -> (a.alarmClock != null)) + )); + return StatsManager.PULL_SUCCESS; + } }); }