Merge "Don't call AlarmManager while holding the BatteryStatsImpl lock." into udc-dev

2 files changed, 50 insertions, 85 deletions

diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java
index f236a961fcb6..d09ca5cadfe7 100644
--- a/services/core/java/com/android/server/am/BatteryStatsService.java
+++ b/services/core/java/com/android/server/am/BatteryStatsService.java
@@ -30,7 +30,6 @@ import static android.os.BatteryStats.POWER_DATA_UNAVAILABLE;
 import android.annotation.EnforcePermission;
 import android.annotation.NonNull;
 import android.annotation.RequiresNoPermission;
-import android.app.AlarmManager;
 import android.app.StatsManager;
 import android.app.usage.NetworkStatsManager;
 import android.bluetooth.BluetoothActivityEnergyInfo;
@@ -414,18 +413,6 @@ public final class BatteryStatsService extends IBatteryStats.Stub
             Slog.e(TAG, "Could not register INetworkManagement event observer " + e);
         }
 
-        final AlarmManager am = mContext.getSystemService(AlarmManager.class);
-        mHandler.post(() -> {
-            synchronized (mStats) {
-                mStats.setLongPlugInAlarmInterface(new AlarmInterface(am, () -> {
-                    synchronized (mStats) {
-                        if (mStats.isOnBattery()) return;
-                        mStats.maybeResetWhilePluggedInLocked();
-                    }
-                }));
-            }
-        });
-
         synchronized (mPowerStatsLock) {
             mPowerStatsInternal = LocalServices.getService(PowerStatsInternal.class);
             if (mPowerStatsInternal != null) {
@@ -2529,32 +2516,6 @@ public final class BatteryStatsService extends IBatteryStats.Stub
         }
     }
 
-    final class AlarmInterface implements BatteryStatsImpl.AlarmInterface,
-            AlarmManager.OnAlarmListener {
-        private AlarmManager mAm;
-        private Runnable mOnAlarm;
-
-        AlarmInterface(AlarmManager am, Runnable onAlarm) {
-            mAm = am;
-            mOnAlarm = onAlarm;
-        }
-
-        @Override
-        public void schedule(long rtcTimeMs, long windowLengthMs) {
-            mAm.setWindow(AlarmManager.RTC, rtcTimeMs, windowLengthMs, TAG, this, mHandler);
-        }
-
-        @Override
-        public void cancel() {
-            mAm.cancel(this);
-        }
-
-        @Override
-        public void onAlarm() {
-            mOnAlarm.run();
-        }
-    }
-
     private static native int nativeWaitWakeup(ByteBuffer outBuffer);
 
     private void dumpHelp(PrintWriter pw) {
diff --git a/services/core/java/com/android/server/power/stats/BatteryStatsImpl.java b/services/core/java/com/android/server/power/stats/BatteryStatsImpl.java
index bc23020e8dbe..661715c0eb12 100644
--- a/services/core/java/com/android/server/power/stats/BatteryStatsImpl.java
+++ b/services/core/java/com/android/server/power/stats/BatteryStatsImpl.java
@@ -26,6 +26,7 @@ import android.annotation.IntDef;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.app.ActivityManager;
+import android.app.AlarmManager;
 import android.app.usage.NetworkStatsManager;
 import android.bluetooth.BluetoothActivityEnergyInfo;
 import android.bluetooth.UidTraffic;
@@ -500,14 +501,6 @@ public class BatteryStatsImpl extends BatteryStats {
 
     }
 
-    /** Handles calls to AlarmManager */
-    public interface AlarmInterface {
-        /** Schedule an RTC alarm */
-        void schedule(long rtcTimeMs, long windowLengthMs);
-        /** Cancel the previously scheduled alarm */
-        void cancel();
-    }
-
     private final PlatformIdleStateCallback mPlatformIdleStateCallback;
 
     private final Runnable mDeferSetCharging = new Runnable() {
@@ -1569,8 +1562,15 @@ public class BatteryStatsImpl extends BatteryStats {
     @GuardedBy("this")
     protected BatteryStatsConfig mBatteryStatsConfig = new BatteryStatsConfig.Builder().build();
 
-    @VisibleForTesting
-    protected AlarmInterface mLongPlugInAlarmInterface = null;
+    @GuardedBy("this")
+    private AlarmManager mAlarmManager = null;
+
+    private final AlarmManager.OnAlarmListener mLongPlugInAlarmHandler = () ->
+            mHandler.post(() -> {
+                synchronized (BatteryStatsImpl.this) {
+                    maybeResetWhilePluggedInLocked();
+                }
+            });
 
     /*
      * Holds a SamplingTimer associated with each Resource Power Manager state and voter,
@@ -11061,18 +11061,6 @@ public class BatteryStatsImpl extends BatteryStats {
     }
 
     /**
-     * Injects an AlarmInterface for the long plug in alarm.
-     */
-    public void setLongPlugInAlarmInterface(AlarmInterface longPlugInAlarmInterface) {
-        synchronized (this) {
-            mLongPlugInAlarmInterface = longPlugInAlarmInterface;
-            if (mBatteryPluggedIn) {
-                scheduleNextResetWhilePluggedInCheck();
-            }
-        }
-    }
-
-    /**
      * Starts tracking CPU time-in-state for threads of the system server process,
      * keeping a separate account of threads receiving incoming binder calls.
      */
@@ -14173,6 +14161,7 @@ public class BatteryStatsImpl extends BatteryStats {
     /**
      * Might reset battery stats if conditions are met. Assumed the device is currently plugged in.
      */
+    @VisibleForTesting
     @GuardedBy("this")
     public void maybeResetWhilePluggedInLocked() {
         final long elapsedRealtimeMs = mClock.elapsedRealtime();
@@ -14189,28 +14178,31 @@ public class BatteryStatsImpl extends BatteryStats {
 
     @GuardedBy("this")
     private void scheduleNextResetWhilePluggedInCheck() {
-        if (mLongPlugInAlarmInterface != null) {
-            final long timeoutMs = mClock.currentTimeMillis()
-                    + mConstants.RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS
-                    * DateUtils.HOUR_IN_MILLIS;
-            Calendar nextAlarm = Calendar.getInstance();
-            nextAlarm.setTimeInMillis(timeoutMs);
-
-            // Find the 2 AM the same day as the end of the minimum duration.
-            // This logic does not handle a Daylight Savings transition, or a timezone change
-            // while the alarm has been set. The need to reset after a long period while plugged
-            // in is not strict enough to warrant a well architected out solution.
-            nextAlarm.set(Calendar.MILLISECOND, 0);
-            nextAlarm.set(Calendar.SECOND, 0);
-            nextAlarm.set(Calendar.MINUTE, 0);
-            nextAlarm.set(Calendar.HOUR_OF_DAY, 2);
-            long nextTimeMs = nextAlarm.getTimeInMillis();
-            if (nextTimeMs < timeoutMs) {
-                // The 2AM on the day of the timeout, move on the next day.
-                nextTimeMs += DateUtils.DAY_IN_MILLIS;
-            }
-            mLongPlugInAlarmInterface.schedule(nextTimeMs, DateUtils.HOUR_IN_MILLIS);
-        }
+        if (mAlarmManager == null) return;
+        final long timeoutMs = mClock.currentTimeMillis()
+                + mConstants.RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS
+                * DateUtils.HOUR_IN_MILLIS;
+        Calendar nextAlarm = Calendar.getInstance();
+        nextAlarm.setTimeInMillis(timeoutMs);
+
+        // Find the 2 AM the same day as the end of the minimum duration.
+        // This logic does not handle a Daylight Savings transition, or a timezone change
+        // while the alarm has been set. The need to reset after a long period while plugged
+        // in is not strict enough to warrant a well architected out solution.
+        nextAlarm.set(Calendar.MILLISECOND, 0);
+        nextAlarm.set(Calendar.SECOND, 0);
+        nextAlarm.set(Calendar.MINUTE, 0);
+        nextAlarm.set(Calendar.HOUR_OF_DAY, 2);
+        long possibleNextTimeMs = nextAlarm.getTimeInMillis();
+        if (possibleNextTimeMs < timeoutMs) {
+            // The 2AM on the day of the timeout, move on the next day.
+            possibleNextTimeMs += DateUtils.DAY_IN_MILLIS;
+        }
+        final long nextTimeMs = possibleNextTimeMs;
+        final AlarmManager am = mAlarmManager;
+        mHandler.post(() -> am.setWindow(AlarmManager.RTC, nextTimeMs,
+                DateUtils.HOUR_IN_MILLIS,
+                TAG, mLongPlugInAlarmHandler, mHandler));
     }
 
 
@@ -14339,8 +14331,12 @@ public class BatteryStatsImpl extends BatteryStats {
                 initActiveHistoryEventsLocked(mSecRealtime, mSecUptime);
             }
             mBatteryPluggedIn = false;
-            if (mLongPlugInAlarmInterface != null) {
-                mLongPlugInAlarmInterface.cancel();
+            if (mAlarmManager != null) {
+                final AlarmManager am = mAlarmManager;
+                mHandler.post(() -> {
+                    // No longer plugged in. Cancel the long plug in alarm.
+                    am.cancel(mLongPlugInAlarmHandler);
+                });
             }
             mHistory.recordBatteryState(mSecRealtime, mSecUptime, level, mBatteryPluggedIn);
             mDischargeCurrentLevel = mDischargeUnplugLevel = level;
@@ -15178,6 +15174,14 @@ public class BatteryStatsImpl extends BatteryStats {
     public void systemServicesReady(Context context) {
         mConstants.startObserving(context.getContentResolver());
         registerUsbStateReceiver(context);
+
+        synchronized (this) {
+            mAlarmManager = context.getSystemService(AlarmManager.class);
+            if (mBatteryPluggedIn) {
+                // Already plugged in. Schedule the long plug in alarm.
+                scheduleNextResetWhilePluggedInCheck();
+            }
+        }
     }
 
     /**