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

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

SyncRtSurfaceTransactionApplier: Improve thread safety 

If the calling thread releases the SurfaceControl passed
to SyncRtSurfaceTransactionApplier concurrently with the
applier preparing the transaction, this can lead to a
synchronization error and a crash. Once the state is
inside the transaction no further synchronization is required
as the native transaction will hold its own sp<SurfaceControl>
reference. By constructing the Transaction on the calling thread
and deferring application to the RenderThread we enable the calling
thread to not have any release synchronization requirements with
RenderThread.

Bug: 186391509
Change-Id: I585e1a9d3baf9ea384b00408b6253f34487d5037
parent 0e4d7f14

core/java/android/view/SyncRtSurfaceTransactionApplier.java

+10 −7
Original line number Diff line number Diff line
@@ -54,20 +54,21 @@ public class SyncRtSurfaceTransactionApplier { 
    /**

     * Schedules applying surface parameters on the next frame.

     *

     * @param params The surface parameters to apply. DO NOT MODIFY the list after passing into

     *               this method to avoid synchronization issues.

     * @param params The surface parameters to apply.

     */

    public void scheduleApply(final SurfaceParams... params) {

        if (mTargetViewRootImpl == null) {

            return;

        }

        mTargetSc = mTargetViewRootImpl.getSurfaceControl();

        final Transaction t = new Transaction();

        applyParams(t, params);



        mTargetViewRootImpl.registerRtFrameCallback(frame -> {

            if (mTargetSc == null || !mTargetSc.isValid()) {

                return;

            }

            Transaction t = new Transaction();

            applyParams(t, frame, params);

            applyTransaction(t, frame);

        });



        // Make sure a frame gets scheduled.
@@ -78,15 +79,17 @@ public class SyncRtSurfaceTransactionApplier { 
     * Applies surface parameters on the next frame.

     * @param t transaction to apply all parameters in.

     * @param frame frame to synchronize to. Set -1 when sync is not required.

     * @param params The surface parameters to apply. DO NOT MODIFY the list after passing into

     *               this method to avoid synchronization issues.

     * @param params The surface parameters to apply.

     */

     void applyParams(Transaction t, long frame, final SurfaceParams... params) {

     void applyParams(Transaction t, final SurfaceParams... params) {

        for (int i = params.length - 1; i >= 0; i--) {

            SurfaceParams surfaceParams = params[i];

            SurfaceControl surface = surfaceParams.surface;

            applyParams(t, surfaceParams, mTmpFloat9);

        }

    }



    void applyTransaction(Transaction t, long frame) {

        if (mTargetViewRootImpl != null) {

            mTargetViewRootImpl.mergeWithNextTransaction(t, frame);

        } else {

core/java/android/view/ViewRootInsetsControllerHost.java

+3 −1
Original line number Diff line number Diff line
@@ -127,7 +127,9 @@ public class ViewRootInsetsControllerHost implements InsetsController.Host { 
            // Window doesn't support hardware acceleration, no synchronization for now.

            // TODO(b/149342281): use mViewRoot.mSurface.getNextFrameNumber() to sync on every

            //  frame instead.

            mApplier.applyParams(new SurfaceControl.Transaction(), -1 /* frame */, params);

            final SurfaceControl.Transaction t = new SurfaceControl.Transaction();

            mApplier.applyParams(t, params);

            mApplier.applyTransaction(t, -1);

        }

    }