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

Commit 65a3d5df authored by Ahan Wu's avatar Ahan Wu Committed by Wu Ahan
Browse files

Fix potential memory leak of FrameTracker 

If begin and end invocations are at the same frame, the end vsync id
will be smaller than the begin vsync id, results in zero size jank info
array, the finish call will be missed as well, a leak happens.

Bug: 192140966
Test: atest FrameTrackerTest InteractionJankMonitorTest
Change-Id: I388558e60bdb84ad248a9afabe7776c4e6e67c57
parent 6b43ea0a

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

+1 −1
Original line number Diff line number Diff line
@@ -251,7 +251,7 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
        // 2. The session never begun.

        if (mBeginVsyncId == INVALID_ID) {

            cancel(REASON_CANCEL_NOT_BEGUN);

        } else if (mEndVsyncId == mBeginVsyncId) {

        } else if (mEndVsyncId <= mBeginVsyncId) {

            cancel(REASON_CANCEL_SAME_VSYNC);

        } else {

            if (DEBUG) {

core/tests/coretests/src/com/android/internal/jank/FrameTrackerTest.java

+21 −8
Original line number Diff line number Diff line
@@ -257,21 +257,14 @@ public class FrameTrackerTest { 
    }



    @Test

    public void testRemoveObserversWhenCancelledInEnd() {

    public void testCancelIfEndVsyncIdEqualsToBeginVsyncId() {

        when(mChoreographer.getVsyncId()).thenReturn(100L);

        mTracker.begin();

        verify(mRenderer, only()).addObserver(any());



        // send first frame - not janky

        sendFrame(4, JANK_NONE, 100L);



        // send another frame - should be considered janky

        sendFrame(40, JANK_APP_DEADLINE_MISSED, 101L);



        // end the trace session

        when(mChoreographer.getVsyncId()).thenReturn(101L);

        mTracker.end(FrameTracker.REASON_END_NORMAL);

        sendFrame(4, JANK_NONE, 102L);



        // Since the begin vsync id (101) equals to the end vsync id (101), will be treat as cancel.

        verify(mTracker).cancel(FrameTracker.REASON_CANCEL_SAME_VSYNC);
@@ -283,6 +276,26 @@ public class FrameTrackerTest { 
        verify(mTracker, never()).triggerPerfetto();

    }



    @Test

    public void testCancelIfEndVsyncIdLessThanBeginVsyncId() {

        when(mChoreographer.getVsyncId()).thenReturn(100L);

        mTracker.begin();

        verify(mRenderer, only()).addObserver(any());



        // end the trace session at the same vsync id, end vsync id will less than the begin one.

        // Because the begin vsync id is supposed to the next frame,

        mTracker.end(FrameTracker.REASON_END_NORMAL);



        // The begin vsync id (101) is larger than the end one (100), will be treat as cancel.

        verify(mTracker).cancel(FrameTracker.REASON_CANCEL_SAME_VSYNC);



        // Observers should be removed in this case, or FrameTracker object will be leaked.

        verify(mTracker).removeObservers();



        // Should never trigger Perfetto since it is a cancel.

        verify(mTracker, never()).triggerPerfetto();

    }



    @Test

    public void testCancelWhenSessionNeverBegun() {

        mTracker.cancel(FrameTracker.REASON_CANCEL_NORMAL);

core/tests/coretests/src/com/android/internal/jank/InteractionJankMonitorTest.java

+4 −0
Original line number Diff line number Diff line
@@ -99,6 +99,8 @@ public class InteractionJankMonitorTest { 
                new FrameMetricsWrapper(), /*traceThresholdMissedFrames=*/ 1,

                /*traceThresholdFrameTimeMillis=*/ -1, null));

        doReturn(tracker).when(monitor).createFrameTracker(any(), any());

        doNothing().when(tracker).triggerPerfetto();

        doNothing().when(tracker).postTraceStartMarker();



        // Simulate a trace session and see if begin / end are invoked.

        assertThat(monitor.begin(mView, session.getCuj())).isTrue();
@@ -146,6 +148,8 @@ public class InteractionJankMonitorTest { 
                new FrameMetricsWrapper(), /*traceThresholdMissedFrames=*/ 1,

                /*traceThresholdFrameTimeMillis=*/ -1, null));

        doReturn(tracker).when(monitor).createFrameTracker(any(), any());

        doNothing().when(tracker).triggerPerfetto();

        doNothing().when(tracker).postTraceStartMarker();



        assertThat(monitor.begin(mView, session.getCuj())).isTrue();

        verify(tracker).begin();