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

Commit eae7d291 authored by Ahan Wu's avatar Ahan Wu
Browse files

Fix memory leak due to not removing ended or cancelled tasks 

While the tracker session ends abnormaly, we should also remove the task
which is kept by InteractionJankMonitor, or a memory leak happens.

Bug: 184651704
Test: atest FrameworksCoreTests:InteractionJankMonitorTest
Test: atest FrameworksCoreTests:FrameTrackerTest
Change-Id: I56f7759ef39d34cedc795042d3210d9be375cf77
parent 245ce41e

core/java/com/android/internal/jank/FrameTracker.java

+8 −5
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import static android.view.SurfaceControl.JankData.SURFACE_FLINGER_SCHEDULING; 
import static com.android.internal.jank.InteractionJankMonitor.ACTION_METRICS_LOGGED;

import static com.android.internal.jank.InteractionJankMonitor.ACTION_SESSION_BEGIN;

import static com.android.internal.jank.InteractionJankMonitor.ACTION_SESSION_CANCEL;

import static com.android.internal.jank.InteractionJankMonitor.ACTION_SESSION_END;



import android.annotation.IntDef;

import android.annotation.NonNull;
@@ -215,7 +216,7 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
            mSurfaceControlWrapper.addJankStatsListener(this, mSurfaceControl);

        }

        if (mListener != null) {

            mListener.onNotifyCujEvents(mSession, ACTION_SESSION_BEGIN);

            mListener.onCujEvents(mSession, ACTION_SESSION_BEGIN);

        }

    }
@@ -240,7 +241,9 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
            }

            Trace.endAsyncSection(mSession.getName(), (int) mBeginVsyncId);

            mSession.setReason(reason);

            InteractionJankMonitor.getInstance().removeTimeout(mSession.getCuj());

            if (mListener != null) {

                mListener.onCujEvents(mSession, ACTION_SESSION_END);

            }

        }

        // We don't remove observer here,

        // will remove it when all the frame metrics in this duration are called back.
@@ -269,7 +272,7 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
        // Notify the listener the session has been cancelled.

        // We don't notify the listeners if the session never begun.

        if (mListener != null) {

            mListener.onNotifyCujEvents(mSession, ACTION_SESSION_CANCEL);

            mListener.onCujEvents(mSession, ACTION_SESSION_CANCEL);

        }

    }
@@ -445,7 +448,7 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
                    maxFrameTimeNanos,

                    missedSfFramesCounts);

            if (mListener != null) {

                mListener.onNotifyCujEvents(mSession, ACTION_METRICS_LOGGED);

                mListener.onCujEvents(mSession, ACTION_METRICS_LOGGED);

            }

        }

        if (DEBUG) {
@@ -587,6 +590,6 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
         * @param session the CUJ session

         * @param action the specific action

         */

        void onNotifyCujEvents(Session session, String action);

        void onCujEvents(Session session, String action);

    }

}

core/java/com/android/internal/jank/InteractionJankMonitor.java

+42 −7
Original line number Diff line number Diff line
@@ -19,7 +19,9 @@ package com.android.internal.jank; 
import static android.content.Intent.FLAG_RECEIVER_REGISTERED_ONLY;



import static com.android.internal.jank.FrameTracker.ChoreographerWrapper;

import static com.android.internal.jank.FrameTracker.REASON_CANCEL_NORMAL;

import static com.android.internal.jank.FrameTracker.REASON_CANCEL_NOT_BEGUN;

import static com.android.internal.jank.FrameTracker.REASON_END_NORMAL;

import static com.android.internal.jank.FrameTracker.SurfaceControlWrapper;

import static com.android.internal.util.FrameworkStatsLog.UIINTERACTION_FRAME_INFO_REPORTED__INTERACTION_TYPE__LAUNCHER_ALL_APPS_SCROLL;

import static com.android.internal.util.FrameworkStatsLog.UIINTERACTION_FRAME_INFO_REPORTED__INTERACTION_TYPE__LAUNCHER_APP_CLOSE_TO_HOME;
@@ -100,6 +102,7 @@ public class InteractionJankMonitor { 
    private static final int DEFAULT_TRACE_THRESHOLD_FRAME_TIME_MILLIS = 64;



    public static final String ACTION_SESSION_BEGIN = ACTION_PREFIX + ".ACTION_SESSION_BEGIN";

    public static final String ACTION_SESSION_END = ACTION_PREFIX + ".ACTION_SESSION_END";

    public static final String ACTION_SESSION_CANCEL = ACTION_PREFIX + ".ACTION_SESSION_CANCEL";

    public static final String ACTION_METRICS_LOGGED = ACTION_PREFIX + ".ACTION_METRICS_LOGGED";

    public static final String BUNDLE_KEY_CUJ_NAME = ACTION_PREFIX + ".CUJ_NAME";
@@ -275,10 +278,7 @@ public class InteractionJankMonitor { 
    public FrameTracker createFrameTracker(View v, Session session) {

        final Context c = v.getContext().getApplicationContext();

        synchronized (this) {

            boolean needListener = SystemProperties.getBoolean(PROP_NOTIFY_CUJ_EVENT, false);

            FrameTrackerListener eventsListener =

                    !needListener ? null : (s, act) -> notifyEvents(c, act, s);



            FrameTrackerListener eventsListener = (s, act) -> handleCujEvents(c, act, s);

            return new FrameTracker(session, mWorker.getThreadHandler(),

                    new ThreadedRendererWrapper(v.getThreadedRenderer()),

                    new ViewRootWrapper(v.getViewRootImpl()), new SurfaceControlWrapper(),
@@ -287,6 +287,28 @@ public class InteractionJankMonitor { 
        }

    }



    private void handleCujEvents(Context context, String action, Session session) {

        // Clear the running and timeout tasks if the end / cancel was fired within the tracker.

        // Or we might have memory leaks.

        if (needRemoveTasks(action, session)) {

            removeTimeout(session.getCuj());

            removeTracker(session.getCuj());

        }



        // Notify the receivers if necessary.

        if (session.shouldNotify()) {

            notifyEvents(context, action, session);

        }

    }



    private boolean needRemoveTasks(String action, Session session) {

        final boolean badEnd = action.equals(ACTION_SESSION_END)

                && session.getReason() != REASON_END_NORMAL;

        final boolean badCancel = action.equals(ACTION_SESSION_CANCEL)

                && session.getReason() != REASON_CANCEL_NORMAL;

        return badEnd || badCancel;

    }



    private void notifyEvents(Context context, String action, Session session) {

        if (action.equals(ACTION_SESSION_CANCEL)

                && session.getReason() == REASON_CANCEL_NOT_BEGUN) {
@@ -299,7 +321,7 @@ public class InteractionJankMonitor { 
        context.sendBroadcast(intent);

    }



    void removeTimeout(@CujType int cujType) {

    private void removeTimeout(@CujType int cujType) {

        synchronized (this) {

            Runnable timeout = mTimeoutActions.get(cujType);

            if (timeout != null) {
@@ -371,7 +393,7 @@ public class InteractionJankMonitor { 
            // Skip this call since we haven't started a trace yet.

            if (tracker == null) return false;

            tracker.end(FrameTracker.REASON_END_NORMAL);

            mRunningTrackers.remove(cujType);

            removeTracker(cujType);

            return true;

        }

    }
@@ -390,7 +412,7 @@ public class InteractionJankMonitor { 
            // Skip this call since we haven't started a trace yet.

            if (tracker == null) return false;

            tracker.cancel(FrameTracker.REASON_CANCEL_NORMAL);

            mRunningTrackers.remove(cujType);

            removeTracker(cujType);

            return true;

        }

    }
@@ -401,6 +423,12 @@ public class InteractionJankMonitor { 
        }

    }



    private void removeTracker(@CujType int cuj) {

        synchronized (this) {

            mRunningTrackers.remove(cuj);

        }

    }



    private void updateProperties(DeviceConfig.Properties properties) {

        synchronized (this) {

            mSamplingInterval = properties.getInt(SETTINGS_SAMPLING_INTERVAL_KEY,
@@ -516,9 +544,11 @@ public class InteractionJankMonitor { 
        private long mTimeStamp;

        @FrameTracker.Reasons

        private int mReason = FrameTracker.REASON_END_UNKNOWN;

        private boolean mShouldNotify;



        public Session(@CujType int cujType) {

            mCujType = cujType;

            mShouldNotify = SystemProperties.getBoolean(PROP_NOTIFY_CUJ_EVENT, false);

        }



        @CujType
@@ -558,5 +588,10 @@ public class InteractionJankMonitor { 
        public int getReason() {

            return mReason;

        }



        /** Determine if should notify the receivers of cuj events */

        public boolean shouldNotify() {

            return mShouldNotify;

        }

    }

}