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

Commit 9f9f08f4 authored by Yisroel Forta's avatar Yisroel Forta
Browse files

Address memory leak in AppStartInfoTracker 

Records were cleared for all error cases, but for success they were only cleared if it reached fully drawn.
This is an issue for 2 reasons:
- fully drawn is not required, so there are success cases where the in prog record is never removed.
- it's just not very robust

To fix this, add logic to ensure that the in progress list doesn't exceed a max size, and that removals are
done of oldest records first.

Also refactors some namings and test details, and adds a new test.

Bug: 342282017
Test: run new test, add logging and confirm correct records are cleared
Flag: EXEMPT - day-to-day bugfix
Change-Id: I2e542e44080cbc9910703a9c3227e344ae4f005f
parent 715e4fd9

services/core/java/com/android/server/am/AppStartInfoTracker.java

+91 −24
Original line number Diff line number Diff line
@@ -89,6 +89,14 @@ public final class AppStartInfoTracker { 


    @VisibleForTesting static final int APP_START_INFO_HISTORY_LIST_SIZE = 16;



    /**

     * The max number of records that can be present in {@link mInProgressRecords}.

     *

     * The magic number of 5 records is expected to be enough because this covers in progress

     * activity starts only, of which more than a 1-2 at a time is very uncommon/unlikely.

     */

    @VisibleForTesting static final int MAX_IN_PROGRESS_RECORDS = 5;



    private static final int APP_START_INFO_MONITORING_MODE_LIST_SIZE = 100;



    @VisibleForTesting static final String APP_START_STORE_DIR = "procstartstore";
@@ -147,7 +155,6 @@ public final class AppStartInfoTracker { 
    /** The path to the historical proc start info file, persisted in the storage. */

    @VisibleForTesting File mProcStartInfoFile;





    /**

     * Temporary list of records that have not been completed.

     *
@@ -155,7 +162,12 @@ public final class AppStartInfoTracker { 
     */

    @GuardedBy("mLock")

    @VisibleForTesting

    final ArrayMap<Long, ApplicationStartInfo> mInProgRecords = new ArrayMap<>();

    final ArrayMap<Long, ApplicationStartInfo> mInProgressRecords = new ArrayMap<>();



    /** Temporary list of keys present in {@link mInProgressRecords} for sorting. */

    @GuardedBy("mLock")

    @VisibleForTesting

    final ArrayList<Integer> mTemporaryInProgressIndexes = new ArrayList<>();



    AppStartInfoTracker() {

        mCallbacks = new SparseArray<>();
@@ -193,6 +205,60 @@ public final class AppStartInfoTracker { 
        });

    }



    /**

     * Trim in progress records structure to acceptable size. To be called after each time a new

     * record is added.

     *

     * This is necessary both for robustness, as well as because the call to

     * {@link onReportFullyDrawn} which triggers the removal in the success case is not guaranteed.

     *

     * <p class="note"> Note: this is the expected path for removal of in progress records for

     * successful activity triggered starts that don't report fully drawn. It is *not* only an edge

     * case.</p>

     */

    @GuardedBy("mLock")

    private void maybeTrimInProgressRecordsLocked() {

        if (mInProgressRecords.size() <= MAX_IN_PROGRESS_RECORDS) {

            // Size is acceptable, do nothing.

            return;

        }



        // Make sure the temporary list is empty.

        mTemporaryInProgressIndexes.clear();



        // Populate the list with indexes for size of {@link mInProgressRecords}.

        for (int i = 0; i < mInProgressRecords.size(); i++) {

            mTemporaryInProgressIndexes.add(i, i);

        }



        // Sort the index collection by value of the corresponding key in {@link mInProgressRecords}

        // from smallest to largest.

        Collections.sort(mTemporaryInProgressIndexes, (a, b) -> Long.compare(

                mInProgressRecords.keyAt(a), mInProgressRecords.keyAt(b)));



        if (mTemporaryInProgressIndexes.size() == MAX_IN_PROGRESS_RECORDS + 1) {

            // Only removing a single record so don't bother sorting again as we don't have to worry

            // about indexes changing.

            mInProgressRecords.removeAt(mTemporaryInProgressIndexes.get(0));

        } else {

            // Removing more than 1 record, remove the records we want to keep from the list and

            // then sort again so we can remove in reverse order of indexes.

            mTemporaryInProgressIndexes.subList(

                    mTemporaryInProgressIndexes.size() - MAX_IN_PROGRESS_RECORDS,

                    mTemporaryInProgressIndexes.size()).clear();

            Collections.sort(mTemporaryInProgressIndexes);



            // Remove all remaining record indexes in reverse order to avoid changing the already

            // calculated indexes.

            for (int i = mTemporaryInProgressIndexes.size() - 1; i >= 0; i--) {

                mInProgressRecords.removeAt(mTemporaryInProgressIndexes.get(i));

            }

        }



        // Clear the temorary list.

        mTemporaryInProgressIndexes.clear();

    }



    void onIntentStarted(@NonNull Intent intent, long timestampNanos) {

        synchronized (mLock) {

            if (!mEnabled) {
@@ -211,7 +277,8 @@ public final class AppStartInfoTracker { 
            } else {

                start.setReason(ApplicationStartInfo.START_REASON_START_ACTIVITY);

            }

            mInProgRecords.put(timestampNanos, start);

            mInProgressRecords.put(timestampNanos, start);

            maybeTrimInProgressRecordsLocked();

        }

    }
@@ -220,17 +287,17 @@ public final class AppStartInfoTracker { 
            if (!mEnabled) {

                return;

            }

            int index = mInProgRecords.indexOfKey(id);

            int index = mInProgressRecords.indexOfKey(id);

            if (index < 0) {

                return;

            }

            ApplicationStartInfo info = mInProgRecords.valueAt(index);

            ApplicationStartInfo info = mInProgressRecords.valueAt(index);

            if (info == null) {

                mInProgRecords.removeAt(index);

                mInProgressRecords.removeAt(index);

                return;

            }

            info.setStartupState(ApplicationStartInfo.STARTUP_STATE_ERROR);

            mInProgRecords.removeAt(index);

            mInProgressRecords.removeAt(index);

        }

    }
@@ -239,13 +306,13 @@ public final class AppStartInfoTracker { 
            if (!mEnabled) {

                return;

            }

            int index = mInProgRecords.indexOfKey(id);

            int index = mInProgressRecords.indexOfKey(id);

            if (index < 0) {

                return;

            }

            ApplicationStartInfo info = mInProgRecords.valueAt(index);

            ApplicationStartInfo info = mInProgressRecords.valueAt(index);

            if (info == null || app == null) {

                mInProgRecords.removeAt(index);

                mInProgressRecords.removeAt(index);

                return;

            }

            info.setStartType((int) temperature);
@@ -254,9 +321,9 @@ public final class AppStartInfoTracker { 
            if (newInfo == null) {

                // newInfo can be null if records are added before load from storage is

                // complete. In this case the newly added record will be lost.

                mInProgRecords.removeAt(index);

                mInProgressRecords.removeAt(index);

            } else {

                mInProgRecords.setValueAt(index, newInfo);

                mInProgressRecords.setValueAt(index, newInfo);

            }

        }

    }
@@ -266,17 +333,17 @@ public final class AppStartInfoTracker { 
            if (!mEnabled) {

                return;

            }

            int index = mInProgRecords.indexOfKey(id);

            int index = mInProgressRecords.indexOfKey(id);

            if (index < 0) {

                return;

            }

            ApplicationStartInfo info = mInProgRecords.valueAt(index);

            ApplicationStartInfo info = mInProgressRecords.valueAt(index);

            if (info == null) {

                mInProgRecords.removeAt(index);

                mInProgressRecords.removeAt(index);

                return;

            }

            info.setStartupState(ApplicationStartInfo.STARTUP_STATE_ERROR);

            mInProgRecords.removeAt(index);

            mInProgressRecords.removeAt(index);

        }

    }
@@ -286,13 +353,13 @@ public final class AppStartInfoTracker { 
            if (!mEnabled) {

                return;

            }

            int index = mInProgRecords.indexOfKey(id);

            int index = mInProgressRecords.indexOfKey(id);

            if (index < 0) {

                return;

            }

            ApplicationStartInfo info = mInProgRecords.valueAt(index);

            ApplicationStartInfo info = mInProgressRecords.valueAt(index);

            if (info == null) {

                mInProgRecords.removeAt(index);

                mInProgressRecords.removeAt(index);

                return;

            }

            info.setLaunchMode(launchMode);
@@ -308,18 +375,18 @@ public final class AppStartInfoTracker { 
            if (!mEnabled) {

                return;

            }

            int index = mInProgRecords.indexOfKey(id);

            int index = mInProgressRecords.indexOfKey(id);

            if (index < 0) {

                return;

            }

            ApplicationStartInfo info = mInProgRecords.valueAt(index);

            ApplicationStartInfo info = mInProgressRecords.valueAt(index);

            if (info == null) {

                mInProgRecords.removeAt(index);

                mInProgressRecords.removeAt(index);

                return;

            }

            info.addStartupTimestamp(ApplicationStartInfo.START_TIMESTAMP_FULLY_DRAWN,

                    timestampNanos);

            mInProgRecords.removeAt(index);

            mInProgressRecords.removeAt(index);

        }

    }
@@ -964,7 +1031,7 @@ public final class AppStartInfoTracker { 
                mProcStartInfoFile.delete();

            }

            mData.getMap().clear();

            mInProgRecords.clear();

            mInProgressRecords.clear();

        }

    }

services/tests/mockingservicestests/src/com/android/server/am/ApplicationStartInfoTest.java

+133 −88

File changed.

Preview size limit exceeded, changes collapsed.