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

Commit 28ab86f0 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

SurfaceView: Fix SurfaceControl synchronization issues 

Hold the surface control lock in PositionUpdateListener
callbacks before checking if the SurfaceControl will
be null. UI tread might release it after the checks
causing crashes.

Also fix a case where the ViewRootImpl may be null
in the positionChanged callback.

Bug: 199261027
Test: run steps in bug
Test: go/wm-smoke
Change-Id: I5a4cac35fe14356389b29268fddf9703b25c03aa
(cherry picked from commit 2cafd8b4)
parent 6c740e93

core/java/android/view/SurfaceView.java

+20 −18
Original line number Diff line number Diff line
@@ -1464,10 +1464,10 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 


        @Override

        public void positionChanged(long frameNumber, int left, int top, int right, int bottom) {

            synchronized(mSurfaceControlLock) {

                if (mSurfaceControl == null) {

                    return;

                }



                // TODO: This is teensy bit racey in that a brand new SurfaceView moving on

                // its 2nd frame if RenderThread is running slowly could potentially see

                // this as false, enter the branch, get pre-empted, then this comes along
@@ -1476,7 +1476,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                // the synchronization would violate the rule that RT must never block

                // on the UI thread which would open up potential deadlocks. The risk of

                // a single-frame desync is therefore preferable for now.

            synchronized(mSurfaceControlLock) {

                mRtHandlingPositionUpdates = true;

            }

            if (mRTLastReportedPosition.left == left
@@ -1506,8 +1505,11 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                if (mViewVisibility) {

                    mPositionChangedTransaction.show(mSurfaceControl);

                }

                final ViewRootImpl viewRoot = getViewRootImpl();

                if (viewRoot != null) {

                    applyChildSurfaceTransaction_renderWorker(mPositionChangedTransaction,

                        getViewRootImpl().mSurface, frameNumber);

                            viewRoot.mSurface, frameNumber);

                }

                applyOrMergeTransaction(mPositionChangedTransaction, frameNumber);

                mPendingTransaction = false;

            } catch (Exception ex) {
@@ -1528,7 +1530,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 


        @Override

        public void positionLost(long frameNumber) {

            if (DEBUG) {

            if (DEBUG_POSITION) {

                Log.d(TAG, String.format("%d windowPositionLost, frameNr = %d",

                        System.identityHashCode(this), frameNumber));

            }
@@ -1540,15 +1542,15 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                mPositionChangedTransaction.clear();

                mPendingTransaction = false;

            }

            if (mSurfaceControl == null) {

                return;

            }



            /**

             * positionLost can be called while UI thread is un-paused so we

             * need to hold the lock here.

             */

            synchronized (mSurfaceControlLock) {

                if (mSurfaceControl == null) {

                    return;

                }

                mRtTransaction.hide(mSurfaceControl);

                if (mRtReleaseSurfaces) {

                    mRtReleaseSurfaces = false;