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

Commit 356fe971 authored by Jacky Wang's avatar Jacky Wang
Browse files

Fix potential OOM caused by DataProcessManager 

The AsyncTasks started in DataProcessManager are not cancelled when
fragment is closed. Introduce helper class LifecycleAwareAsyncTask to
cancel AsyncTask automatically when lifecycle is stopped.

Bug: 384473507
Flag: EXEMPT bug fix
Test: Unit test & open/close battery usage screen 1000 times
Change-Id: I060f559fa85cc5feb9a42cb8dcc0581782a91d09
parent 87ce9dbb

src/com/android/settings/fuelgauge/batteryusage/BatteryUsageDataLoader.java

+1 −0
Original line number Diff line number Diff line
@@ -118,6 +118,7 @@ public final class BatteryUsageDataLoader { 
        final BatteryLevelData batteryLevelData =

                DataProcessManager.getBatteryLevelData(

                        context,

                        null,

                        userIdsSeries,

                        /* isFromPeriodJob= */ true,

                        batteryDiffDataMap -> {

src/com/android/settings/fuelgauge/batteryusage/DataProcessManager.java

+35 −17
Original line number Diff line number Diff line
@@ -18,12 +18,12 @@ package com.android.settings.fuelgauge.batteryusage; 


import android.app.usage.UsageEvents;

import android.content.Context;

import android.os.AsyncTask;

import android.util.ArrayMap;

import android.util.Log;



import androidx.annotation.NonNull;

import androidx.annotation.Nullable;

import androidx.lifecycle.Lifecycle;



import com.android.internal.annotations.VisibleForTesting;

import com.android.settings.fuelgauge.PowerUsageFeatureProvider;
@@ -80,6 +80,7 @@ public class DataProcessManager { 
    private final long mLastFullChargeTimestamp;

    private final boolean mIsFromPeriodJob;

    private final Context mContext;

    private final @Nullable Lifecycle mLifecycle;

    private final UserIdsSeries mUserIdsSeries;

    private final OnBatteryDiffDataMapLoadedListener mCallbackFunction;

    private final List<AppUsageEvent> mAppUsageEventList = new ArrayList<>();
@@ -120,6 +121,7 @@ public class DataProcessManager { 
    /** Constructor when there exists battery level data. */

    DataProcessManager(

            Context context,

            @Nullable Lifecycle lifecycle,

            final UserIdsSeries userIdsSeries,

            final boolean isFromPeriodJob,

            final long rawStartTimestamp,
@@ -128,6 +130,7 @@ public class DataProcessManager { 
            @NonNull final List<BatteryLevelData.PeriodBatteryLevelData> hourlyBatteryLevelsPerDay,

            @NonNull final Map<Long, Map<String, BatteryHistEntry>> batteryHistoryMap) {

        mContext = context.getApplicationContext();

        mLifecycle = lifecycle;

        mUserIdsSeries = userIdsSeries;

        mIsFromPeriodJob = isFromPeriodJob;

        mRawStartTimestamp = rawStartTimestamp;
@@ -140,9 +143,11 @@ public class DataProcessManager { 
    /** Constructor when there is no battery level data. */

    DataProcessManager(

            Context context,

            @Nullable Lifecycle lifecycle,

            final UserIdsSeries userIdsSeries,

            @NonNull final OnBatteryDiffDataMapLoadedListener callbackFunction) {

        mContext = context.getApplicationContext();

        mLifecycle = lifecycle;

        mUserIdsSeries = userIdsSeries;

        mCallbackFunction = callbackFunction;

        mIsFromPeriodJob = false;
@@ -223,7 +228,7 @@ public class DataProcessManager { 
    }



    private void loadCurrentBatteryHistoryMap() {

        new AsyncTask<Void, Void, Map<String, BatteryHistEntry>>() {

        new LifecycleAwareAsyncTask<Map<String, BatteryHistEntry>>(mLifecycle) {

            @Override

            protected Map<String, BatteryHistEntry> doInBackground(Void... voids) {

                final long startTime = System.currentTimeMillis();
@@ -242,6 +247,7 @@ public class DataProcessManager { 
            @Override

            protected void onPostExecute(

                    final Map<String, BatteryHistEntry> currentBatteryHistoryMap) {

                super.onPostExecute(currentBatteryHistoryMap);

                if (mBatteryHistoryMap != null) {

                    // Replaces the placeholder in mBatteryHistoryMap.

                    for (Map.Entry<Long, Map<String, BatteryHistEntry>> mapEntry :
@@ -256,11 +262,11 @@ public class DataProcessManager { 
                mIsCurrentBatteryHistoryLoaded = true;

                tryToGenerateFinalDataAndApplyCallback();

            }

        }.execute();

        }.start();

    }



    private void loadCurrentAppUsageList() {

        new AsyncTask<Void, Void, List<AppUsageEvent>>() {

        new LifecycleAwareAsyncTask<List<AppUsageEvent>>(mLifecycle) {

            @Override

            @Nullable

            protected List<AppUsageEvent> doInBackground(Void... voids) {
@@ -299,6 +305,7 @@ public class DataProcessManager { 


            @Override

            protected void onPostExecute(final List<AppUsageEvent> currentAppUsageList) {

                super.onPostExecute(currentAppUsageList);

                if (currentAppUsageList == null || currentAppUsageList.isEmpty()) {

                    Log.d(TAG, "currentAppUsageList is null or empty");

                } else {
@@ -307,11 +314,11 @@ public class DataProcessManager { 
                mIsCurrentAppUsageLoaded = true;

                tryToProcessAppUsageData();

            }

        }.execute();

        }.start();

    }



    private void loadDatabaseAppUsageList() {

        new AsyncTask<Void, Void, List<AppUsageEvent>>() {

        new LifecycleAwareAsyncTask<List<AppUsageEvent>>(mLifecycle) {

            @Override

            protected List<AppUsageEvent> doInBackground(Void... voids) {

                if (!shouldLoadAppUsageData()) {
@@ -337,6 +344,7 @@ public class DataProcessManager { 


            @Override

            protected void onPostExecute(final List<AppUsageEvent> databaseAppUsageList) {

                super.onPostExecute(databaseAppUsageList);

                if (databaseAppUsageList == null || databaseAppUsageList.isEmpty()) {

                    Log.d(TAG, "databaseAppUsageList is null or empty");

                } else {
@@ -345,11 +353,11 @@ public class DataProcessManager { 
                mIsDatabaseAppUsageLoaded = true;

                tryToProcessAppUsageData();

            }

        }.execute();

        }.start();

    }



    private void loadPowerConnectionBatteryEventList() {

        new AsyncTask<Void, Void, List<BatteryEvent>>() {

        new LifecycleAwareAsyncTask<List<BatteryEvent>>(mLifecycle) {

            @Override

            protected List<BatteryEvent> doInBackground(Void... voids) {

                final long startTime = System.currentTimeMillis();
@@ -370,6 +378,7 @@ public class DataProcessManager { 


            @Override

            protected void onPostExecute(final List<BatteryEvent> batteryEventList) {

                super.onPostExecute(batteryEventList);

                if (batteryEventList == null || batteryEventList.isEmpty()) {

                    Log.d(TAG, "batteryEventList is null or empty");

                } else {
@@ -379,11 +388,11 @@ public class DataProcessManager { 
                mIsBatteryEventLoaded = true;

                tryToProcessAppUsageData();

            }

        }.execute();

        }.start();

    }



    private void loadBatteryUsageSlotList() {

        new AsyncTask<Void, Void, List<BatteryUsageSlot>>() {

        new LifecycleAwareAsyncTask<List<BatteryUsageSlot>>(mLifecycle) {

            @Override

            protected List<BatteryUsageSlot> doInBackground(Void... voids) {

                final long startTime = System.currentTimeMillis();
@@ -402,6 +411,7 @@ public class DataProcessManager { 


            @Override

            protected void onPostExecute(final List<BatteryUsageSlot> batteryUsageSlotList) {

                super.onPostExecute(batteryUsageSlotList);

                if (batteryUsageSlotList == null || batteryUsageSlotList.isEmpty()) {

                    Log.d(TAG, "batteryUsageSlotList is null or empty");

                } else {
@@ -411,11 +421,11 @@ public class DataProcessManager { 
                mIsBatteryUsageSlotLoaded = true;

                tryToGenerateFinalDataAndApplyCallback();

            }

        }.execute();

        }.start();

    }



    private void loadAndApplyBatteryMapFromServiceOnly() {

        new AsyncTask<Void, Void, Map<Long, BatteryDiffData>>() {

        new LifecycleAwareAsyncTask<Map<Long, BatteryDiffData>>(mLifecycle) {

            @Override

            protected Map<Long, BatteryDiffData> doInBackground(Void... voids) {

                final long startTime = System.currentTimeMillis();
@@ -437,11 +447,12 @@ public class DataProcessManager { 


            @Override

            protected void onPostExecute(final Map<Long, BatteryDiffData> batteryDiffDataMap) {

                super.onPostExecute(batteryDiffDataMap);

                if (mCallbackFunction != null) {

                    mCallbackFunction.onBatteryDiffDataMapLoaded(batteryDiffDataMap);

                }

            }

        }.execute();

        }.start();

    }



    private void tryToProcessAppUsageData() {
@@ -481,7 +492,7 @@ public class DataProcessManager { 
    }



    private synchronized void generateFinalDataAndApplyCallback() {

        new AsyncTask<Void, Void, Map<Long, BatteryDiffData>>() {

        new LifecycleAwareAsyncTask<Map<Long, BatteryDiffData>>(mLifecycle) {

            @Override

            protected Map<Long, BatteryDiffData> doInBackground(Void... voids) {

                final long startTime = System.currentTimeMillis();
@@ -523,11 +534,12 @@ public class DataProcessManager { 


            @Override

            protected void onPostExecute(final Map<Long, BatteryDiffData> batteryDiffDataMap) {

                super.onPostExecute(batteryDiffDataMap);

                if (mCallbackFunction != null) {

                    mCallbackFunction.onBatteryDiffDataMapLoaded(batteryDiffDataMap);

                }

            }

        }.execute();

        }.start();

    }



    // Whether we should load app usage data from service or database.
@@ -566,6 +578,7 @@ public class DataProcessManager { 
    @Nullable

    public static BatteryLevelData getBatteryLevelData(

            Context context,

            @Nullable Lifecycle lifecycle,

            final UserIdsSeries userIdsSeries,

            final boolean isFromPeriodJob,

            final OnBatteryDiffDataMapLoadedListener onBatteryUsageMapLoadedListener) {
@@ -585,6 +598,7 @@ public class DataProcessManager { 
        final BatteryLevelData batteryLevelData =

                getPeriodBatteryLevelData(

                        context,

                        lifecycle,

                        userIdsSeries,

                        startTimestamp,

                        lastFullChargeTime,
@@ -604,6 +618,7 @@ public class DataProcessManager { 


    private static BatteryLevelData getPeriodBatteryLevelData(

            Context context,

            @Nullable Lifecycle lifecycle,

            final UserIdsSeries userIdsSeries,

            final long startTimestamp,

            final long lastFullChargeTime,
@@ -631,7 +646,8 @@ public class DataProcessManager { 
                                lastFullChargeTime);

        if (batteryHistoryMap == null || batteryHistoryMap.isEmpty()) {

            Log.d(TAG, "batteryHistoryMap is null in getPeriodBatteryLevelData()");

            new DataProcessManager(context, userIdsSeries, onBatteryDiffDataMapLoadedListener)

            new DataProcessManager(context, lifecycle, userIdsSeries,

                    onBatteryDiffDataMapLoadedListener)

                    .start();

            return null;

        }
@@ -660,7 +676,8 @@ public class DataProcessManager { 
                DataProcessor.getLevelDataThroughProcessedHistoryMap(

                        context, processedBatteryHistoryMap);

        if (batteryLevelData == null) {

            new DataProcessManager(context, userIdsSeries, onBatteryDiffDataMapLoadedListener)

            new DataProcessManager(context, lifecycle, userIdsSeries,

                    onBatteryDiffDataMapLoadedListener)

                    .start();

            Log.d(TAG, "getBatteryLevelData() returns null");

            return null;
@@ -669,6 +686,7 @@ public class DataProcessManager { 
        // Start the async task to compute diff usage data and load labels and icons.

        new DataProcessManager(

                        context,

                        lifecycle,

                        userIdsSeries,

                        isFromPeriodJob,

                        startTimestamp,

src/com/android/settings/fuelgauge/batteryusage/LifecycleAwareAsyncTask.kt

0 → 100644
+61 −0
Original line number Diff line number Diff line 
/*

 * 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.settings.fuelgauge.batteryusage



import android.os.AsyncTask

import androidx.annotation.CallSuper

import androidx.lifecycle.DefaultLifecycleObserver

import androidx.lifecycle.Lifecycle

import androidx.lifecycle.LifecycleOwner

import com.android.settingslib.datastore.HandlerExecutor.Companion.main as mainExecutor



/**

 * Lifecycle aware [AsyncTask] to cancel task automatically when [lifecycle] is stopped.

 *

 * Must call [start] instead of [execute] to run the task.

 */

abstract class LifecycleAwareAsyncTask<Result>(private val lifecycle: Lifecycle?) :

    AsyncTask<Void, Void, Result>(), DefaultLifecycleObserver {



    @CallSuper

    override fun onPostExecute(result: Result) {

        lifecycle?.removeObserver(this)

    }



    override fun onStop(owner: LifecycleOwner) {

        cancel(false)

        lifecycle?.removeObserver(this)

    }



    /**

     * Starts the task, which invokes [execute] (cannot override [execute] as it is final).

     *

     * This method is expected to be invoked from main thread but current usage might call from

     * background thread.

     */

    fun start() {

        execute() // expects main thread

        val lifecycle = lifecycle ?: return

        mainExecutor.execute {

            // Status is updated to FINISHED if onPoseExecute happened before. And task is cancelled

            // if lifecycle is stopped.

            if (status == Status.RUNNING && !isCancelled) {

                lifecycle.addObserver(this) // requires main thread

            }

        }

    }

}

src/com/android/settings/fuelgauge/batteryusage/PowerUsageAdvanced.java

+4 −2
Original line number Diff line number Diff line
@@ -503,9 +503,11 @@ public class PowerUsageAdvanced extends PowerUsageBase { 


                @Override

                public BatteryLevelData loadInBackground() {

                    Context context = getContext();

                    return DataProcessManager.getBatteryLevelData(

                            getContext(),

                            new UserIdsSeries(getContext(), /* isNonUIRequest= */ false),

                            context,

                            getLifecycle(),

                            new UserIdsSeries(context, /* isNonUIRequest= */ false),

                            /* isFromPeriodJob= */ false,

                            PowerUsageAdvanced.this::onBatteryDiffDataMapUpdate);

                }

tests/robotests/src/com/android/settings/fuelgauge/batteryusage/DataProcessManagerTest.java

+7 −0
Original line number Diff line number Diff line
@@ -110,6 +110,7 @@ public final class DataProcessManagerTest { 
        mDataProcessManager =

                new DataProcessManager(

                        mContext,

                        null,

                        mUserIdsSeries,

                        /* isFromPeriodJob= */ false,

                        /* rawStartTimestamp= */ 0L,
@@ -130,6 +131,7 @@ public final class DataProcessManagerTest { 
        final DataProcessManager dataProcessManager =

                new DataProcessManager(

                        mContext,

                        null,

                        mUserIdsSeries,

                        /* callbackFunction= */ null);

        assertThat(dataProcessManager.getShowScreenOnTime()).isFalse();
@@ -255,6 +257,7 @@ public final class DataProcessManagerTest { 
        final DataProcessManager dataProcessManager =

                new DataProcessManager(

                        mContext,

                        null,

                        mUserIdsSeries,

                        /* isFromPeriodJob= */ false,

                        /* rawStartTimestamp= */ 2L,
@@ -346,6 +349,7 @@ public final class DataProcessManagerTest { 
        assertThat(

                        DataProcessManager.getBatteryLevelData(

                                mContext,

                                null,

                                mUserIdsSeries,

                                /* isFromPeriodJob= */ false,

                                /* asyncResponseDelegate= */ null))
@@ -353,6 +357,7 @@ public final class DataProcessManagerTest { 
        assertThat(

                        DataProcessManager.getBatteryLevelData(

                                mContext,

                                null,

                                mUserIdsSeries,

                                /* isFromPeriodJob= */ true,

                                /* asyncResponseDelegate= */ null))
@@ -374,6 +379,7 @@ public final class DataProcessManagerTest { 
        final BatteryLevelData resultData =

                DataProcessManager.getBatteryLevelData(

                        mContext,

                        null,

                        mUserIdsSeries,

                        /* isFromPeriodJob= */ false,

                        /* asyncResponseDelegate= */ null);
@@ -402,6 +408,7 @@ public final class DataProcessManagerTest { 
        final BatteryLevelData resultData =

                DataProcessManager.getBatteryLevelData(

                        mContext,

                        null,

                        mUserIdsSeries,

                        /* isFromPeriodJob= */ false,

                        /* asyncResponseDelegate= */ null);