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

Commit ebaaca1a authored by Robert Carr's avatar Robert Carr Committed by Rob Carr
Browse files

SurfaceView: positionLost locking fix 

positionLost can be called from CanvasContext::destroyHardwareResources
which runs asynchronously to the UI thread. This means we could be
simultaneously executing releaseSurfaces on the UI thread. We need
to expand the scope of mSurfaceControl lock in positionLost. While
we are here we add a block comment explaining the previously
undocumented locking strategy.

Bug: 156264048
Test: Existing tests pass
Change-Id: I9cdb6a0f7aeffd878f1755f240e8896f0fb8bf01
parent 43d32906

core/java/android/view/SurfaceView.java

+28 −7
Original line number Diff line number Diff line
@@ -134,6 +134,23 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
    // we need to preserve the old one until the new one has drawn.

    SurfaceControl mDeferredDestroySurfaceControl;

    SurfaceControl mBackgroundControl;



    /**

     * We use this lock in SOME cases when reading or writing SurfaceControl,

     * but use the following model so that the RenderThread can run locklessly

     * in the position up-date case.

     *

     * 1. UI Thread can read from mSurfaceControl (use in Transactions) without

     * holding the lock.

     * 2. UI Thread will hold the lock when writing to mSurfaceControl (calling release

     * or remove).

     * 3. Render thread will also hold the lock when writing to mSurfaceControl (e.g.

     * calling release from positionLost).

     * 3. RenderNode.PositionUpdateListener::positionChanged will only be called

     * when the UI thread is paused (blocked on the Render thread).

     * 4. positionChanged thus will not be required to hold the lock as the

     * UI thread is blocked, and the other writer is the RT itself.

     */

    final Object mSurfaceControlLock = new Object();

    final Rect mTmpRect = new Rect();
@@ -1297,6 +1314,11 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                (viewRoot != null ? viewRoot.getBLASTSyncTransaction() : mRtTransaction) :

                mRtTransaction;



            /**

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

             * need to hold the lock here.

             */

            synchronized (mSurfaceControlLock) {

                if (frameNumber > 0 && viewRoot !=  null && !useBLAST) {

                    if (viewRoot.mSurface.isValid()) {

                        mRtTransaction.deferTransactionUntil(mSurfaceControl,
@@ -1305,7 +1327,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                }

                t.hide(mSurfaceControl);



            synchronized (mSurfaceControlLock) {

                if (mRtReleaseSurfaces) {

                    mRtReleaseSurfaces = false;

                    mRtTransaction.remove(mSurfaceControl);