Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d953c53d authored by Dianne Hackborn's avatar Dianne Hackborn
Browse files

Work on issue #16629489: Google (Play?) Services eating through battery

There is a bug in how we deal with name overflows combined with resetting
the battery stats data.  If we do a reset while a wakelock is being
actively held that has been put into the overflow bucket, then we can
end up reducing the number of known wake locks in the list so when after
that it is released we try to release it under its real name rather than
the overflow name.

This means we need to keep track of which wake locks have been placed
in the overflow bucket while they are actively being used, so we can be
sure to properly handle it as part of that bucket until it is eventually
released.

This makes things...  somewhat more complicated.  So now we have a class
to take care of all these details, and also use it for other places where
we have the same overflow semantics sync and job stats.

Also fix potential deadlock -- BatteryStatsHelper needs to call on to
ConnectivityManager to find out of there is telepohny, however we use
that class when doing a dump while the battery stats lock is held.  To
fix this, we check the connectivity state up in the battery stats service
before acquiring the lock and propagate that information through to the
dump code.

Change-Id: Ib452206af5c36f4b0f03cc94d2845d36613d1ba5
parent 82d6d337
Loading
Loading
Loading
Loading
+30 −9
Original line number Diff line number Diff line
@@ -1844,12 +1844,20 @@ public abstract class BatteryStats implements Parcelable {
        pw.println();
    }

    /**
     * Temporary for settings.
     */
    public final void dumpCheckinLocked(Context context, PrintWriter pw, int which, int reqUid) {
        dumpCheckinLocked(context, pw, which, reqUid, BatteryStatsHelper.checkWifiOnly(context));
    }

    /**
     * Checkin server version of dump to produce more compact, computer-readable log.
     * 
     * NOTE: all times are expressed in 'ms'.
     */
    public final void dumpCheckinLocked(Context context, PrintWriter pw, int which, int reqUid) {
    public final void dumpCheckinLocked(Context context, PrintWriter pw, int which, int reqUid,
            boolean wifiOnly) {
        final long rawUptime = SystemClock.uptimeMillis() * 1000;
        final long rawRealtime = SystemClock.elapsedRealtime() * 1000;
        final long batteryUptime = getBatteryUptime(rawUptime);
@@ -2046,7 +2054,7 @@ public abstract class BatteryStats implements Parcelable {
            }
        }
        
        BatteryStatsHelper helper = new BatteryStatsHelper(context, false);
        BatteryStatsHelper helper = new BatteryStatsHelper(context, false, wifiOnly);
        helper.create(this);
        helper.refreshStats(which, UserHandle.USER_ALL);
        List<BatterySipper> sippers = helper.getUsageList();
@@ -2315,9 +2323,17 @@ public abstract class BatteryStats implements Parcelable {
        printer.print(BatteryStatsHelper.makemAh(power));
    }

    /**
     * Temporary for settings.
     */
    public final void dumpLocked(Context context, PrintWriter pw, String prefix, int which,
            int reqUid) {
        dumpLocked(context, pw, prefix, which, reqUid, BatteryStatsHelper.checkWifiOnly(context));
    }

    @SuppressWarnings("unused")
    public final void dumpLocked(Context context, PrintWriter pw, String prefix, final int which,
            int reqUid) {
            int reqUid, boolean wifiOnly) {
        final long rawUptime = SystemClock.uptimeMillis() * 1000;
        final long rawRealtime = SystemClock.elapsedRealtime() * 1000;
        final long batteryUptime = getBatteryUptime(rawUptime);
@@ -2746,7 +2762,7 @@ public abstract class BatteryStats implements Parcelable {
            pw.println();
        }

        BatteryStatsHelper helper = new BatteryStatsHelper(context, false);
        BatteryStatsHelper helper = new BatteryStatsHelper(context, false, wifiOnly);
        helper.create(this);
        helper.refreshStats(which, UserHandle.USER_ALL);
        List<BatterySipper> sippers = helper.getUsageList();
@@ -3723,6 +3739,7 @@ public abstract class BatteryStats implements Parcelable {
    public static final int DUMP_HISTORY_ONLY = 1<<2;
    public static final int DUMP_INCLUDE_HISTORY = 1<<3;
    public static final int DUMP_VERBOSE = 1<<4;
    public static final int DUMP_DEVICE_WIFI_ONLY = 1<<5;

    private void dumpHistoryLocked(PrintWriter pw, int flags, long histStart, boolean checkin) {
        final HistoryPrinter hprinter = new HistoryPrinter();
@@ -3918,12 +3935,14 @@ public abstract class BatteryStats implements Parcelable {
            pw.println("Statistics since last charge:");
            pw.println("  System starts: " + getStartCount()
                    + ", currently on battery: " + getIsOnBattery());
            dumpLocked(context, pw, "", STATS_SINCE_CHARGED, reqUid);
            dumpLocked(context, pw, "", STATS_SINCE_CHARGED, reqUid,
                    (flags&DUMP_DEVICE_WIFI_ONLY) != 0);
            pw.println();
        }
        if (!filtering || (flags&DUMP_UNPLUGGED_ONLY) != 0) {
            pw.println("Statistics since last unplugged:");
            dumpLocked(context, pw, "", STATS_SINCE_UNPLUGGED, reqUid);
            dumpLocked(context, pw, "", STATS_SINCE_UNPLUGGED, reqUid,
                    (flags&DUMP_DEVICE_WIFI_ONLY) != 0);
        }
    }
    
@@ -4013,10 +4032,12 @@ public abstract class BatteryStats implements Parcelable {
                dumpLine(pw, 0 /* uid */, "i" /* category */, CHARGE_TIME_REMAIN_DATA,
                        (Object[])lineArgs);
            }
            dumpCheckinLocked(context, pw, STATS_SINCE_CHARGED, -1);
            dumpCheckinLocked(context, pw, STATS_SINCE_CHARGED, -1,
                    (flags&DUMP_DEVICE_WIFI_ONLY) != 0);
        }
        if (!filtering || (flags&DUMP_UNPLUGGED_ONLY) != 0) {
            dumpCheckinLocked(context, pw, STATS_SINCE_UNPLUGGED, -1);
            dumpCheckinLocked(context, pw, STATS_SINCE_UNPLUGGED, -1,
                    (flags&DUMP_DEVICE_WIFI_ONLY) != 0);
        }
    }
}
+15 −3
Original line number Diff line number Diff line
@@ -74,6 +74,7 @@ public final class BatteryStatsHelper {

    final private Context mContext;
    final private boolean mCollectBatteryBroadcast;
    final private boolean mWifiOnly;

    private IBatteryStats mBatteryInfo;
    private BatteryStats mStats;
@@ -123,6 +124,19 @@ public final class BatteryStatsHelper {
    public BatteryStatsHelper(Context context, boolean collectBatteryBroadcast) {
        mContext = context;
        mCollectBatteryBroadcast = collectBatteryBroadcast;
        mWifiOnly = checkWifiOnly(context);
    }

    public BatteryStatsHelper(Context context, boolean collectBatteryBroadcast, boolean wifiOnly) {
        mContext = context;
        mCollectBatteryBroadcast = collectBatteryBroadcast;
        mWifiOnly = wifiOnly;
    }

    public static boolean checkWifiOnly(Context context) {
        ConnectivityManager cm = (ConnectivityManager)context.getSystemService(
                Context.CONNECTIVITY_SERVICE);
        return !cm.isNetworkSupported(ConnectivityManager.TYPE_MOBILE);
    }

    public void storeStatsHistoryInFile(String fname) {
@@ -870,9 +884,7 @@ public final class BatteryStatsHelper {
        addBluetoothUsage();
        addIdleUsage(); // Not including cellular idle power
        // Don't compute radio usage if it's a wifi-only device
        ConnectivityManager cm = (ConnectivityManager)mContext.getSystemService(
                Context.CONNECTIVITY_SERVICE);
        if (cm.isNetworkSupported(ConnectivityManager.TYPE_MOBILE)) {
        if (!mWifiOnly) {
            addRadioUsage();
        }
    }
+256 −130

File changed.

Preview size limit exceeded, changes collapsed.

+4 −0
Original line number Diff line number Diff line
@@ -39,6 +39,7 @@ import android.telephony.TelephonyManager;
import android.util.Slog;

import com.android.internal.app.IBatteryStats;
import com.android.internal.os.BatteryStatsHelper;
import com.android.internal.os.BatteryStatsImpl;
import com.android.internal.os.PowerProfile;
import com.android.server.LocalServices;
@@ -868,6 +869,9 @@ public final class BatteryStatsService extends IBatteryStats.Stub
        if (noOutput) {
            return;
        }
        if (BatteryStatsHelper.checkWifiOnly(mContext)) {
            flags |= BatteryStats.DUMP_DEVICE_WIFI_ONLY;
        }
        if (useCheckinFormat) {
            List<ApplicationInfo> apps = mContext.getPackageManager().getInstalledApplications(0);
            if (isRealCheckin) {
+6 −3
Original line number Diff line number Diff line
@@ -865,7 +865,8 @@ public final class PowerManagerService extends com.android.server.SystemService
        }
    }

    private void updateWakeLockWorkSourceInternal(IBinder lock, WorkSource ws, String historyTag) {
    private void updateWakeLockWorkSourceInternal(IBinder lock, WorkSource ws, String historyTag,
            int callingUid) {
        synchronized (mLock) {
            int index = findWakeLockIndexLocked(lock);
            if (index < 0) {
@@ -873,7 +874,8 @@ public final class PowerManagerService extends com.android.server.SystemService
                    Slog.d(TAG, "updateWakeLockWorkSourceInternal: lock=" + Objects.hashCode(lock)
                            + " [not found], ws=" + ws);
                }
                throw new IllegalArgumentException("Wake lock not active");
                throw new IllegalArgumentException("Wake lock not active: " + lock
                        + " from uid " + callingUid);
            }

            WakeLock wakeLock = mWakeLocks.get(index);
@@ -2802,9 +2804,10 @@ public final class PowerManagerService extends com.android.server.SystemService
                ws = null;
            }

            final int callingUid = Binder.getCallingUid();
            final long ident = Binder.clearCallingIdentity();
            try {
                updateWakeLockWorkSourceInternal(lock, ws, historyTag);
                updateWakeLockWorkSourceInternal(lock, ws, historyTag, callingUid);
            } finally {
                Binder.restoreCallingIdentity(ident);
            }