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

Commit ef3f9c55 authored by Daichi Hirono's avatar Daichi Hirono
Browse files

Prevent surface use after release in DragResizeInputListener 

DragResizeInputListener initialized input channels on a background
thread. The decor surface used for initialization could be released on
the main thread before the background initialization completed, leading
to a use-after-release.

This change addresses the issue by having DragResizeInputListener create
and own a new SurfaceControl referencing the same underlying surface as
the decor surface. DragResizeInputListener is then responsible for
releasing this new SurfaceControl after background initialization,
ensuring safe cleanup.

Bug: 401470167
Test: DragResizeInputListenerTest
Flag: com.android.window.flags.enable_drag_resize_set_up_in_bg_thread
Change-Id: I37ba7d97d51079ba07148a425826a0cd9b59c3fd
parent c76ef2ce

libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/DragResizeInputListener.java

+8 −1
Original line number Diff line number Diff line
@@ -138,7 +138,11 @@ class DragResizeInputListener implements AutoCloseable { 
        mHandler = handler;

        mChoreographer = choreographer;

        mDisplayId = displayId;

        mDecorationSurface = decorationSurface;

        // Creates a new SurfaceControl pointing the same underlying surface with decorationSurface

        // to ensure that mDecorationSurface will not be released while it's used on the background

        // thread. Note that the empty name will be overridden by the next copyFrom call.

        mDecorationSurface = surfaceControlBuilderSupplier.get().setName("").build();

        mDecorationSurface.copyFrom(decorationSurface, "DragResizeInputListener");

        mDragPositioningCallback = callback;

        mSurfaceControlBuilderSupplier = surfaceControlBuilderSupplier;

        mSurfaceControlTransactionSupplier = surfaceControlTransactionSupplier;
@@ -427,6 +431,9 @@ class DragResizeInputListener implements AutoCloseable { 
            } catch (RemoteException e) {

                e.rethrowFromSystemServer();

            }

            // Removing this surface on the background thread to ensure that mInitInputChannels has

            // already been finished.

            mSurfaceControlTransactionSupplier.get().remove(mDecorationSurface).apply();

        });

    }

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/DragResizeInputListenerTest.kt

+77 −3
Original line number Diff line number Diff line
@@ -40,11 +40,13 @@ import com.android.wm.shell.windowdecor.DragResizeInputListener.TaskResizeInputE 
import com.google.common.truth.Truth.assertThat

import java.util.function.Consumer

import java.util.function.Supplier

import org.junit.After

import org.junit.Test

import org.junit.runner.RunWith

import org.mockito.ArgumentMatchers.any

import org.mockito.ArgumentMatchers.anyInt

import org.mockito.kotlin.anyOrNull

import org.mockito.kotlin.argThat

import org.mockito.kotlin.mock

import org.mockito.kotlin.never

import org.mockito.kotlin.verify
@@ -65,6 +67,13 @@ class DragResizeInputListenerTest : ShellTestCase() { 
    private val mockInputEventReceiver = mock<TaskResizeInputEventReceiver>()

    private val inputChannel = mock<InputChannel>()

    private val sinkInputChannel = mock<InputChannel>()

    private val decorationSurface = SurfaceControl.Builder().setName("decoration surface").build()

    private val createdSurfaces = ArrayList<SurfaceControl>()



    @After

    fun tearDown() {

        decorationSurface.release()

    }



    @Test

    fun testGrantInputChannelOffMainThread() {
@@ -74,6 +83,35 @@ class DragResizeInputListenerTest : ShellTestCase() { 
        verifyNoInputChannelGrantRequests()

    }



    @Test

    fun testGrantInputChannelAfterDecorSurfaceReleased() {

        // Keep tracking the underlying surface that the decorationSurface points to.

        val forVerification = SurfaceControl(decorationSurface, "forVerification")

        try {

            create()

            decorationSurface.release()

            testBgExecutor.flushAll()



            verify(mockWindowSession)

                .grantInputChannel(

                    anyInt(),

                    argThat<SurfaceControl> { isValid && isSameSurface(forVerification) },

                    any(),

                    anyOrNull(),

                    anyInt(),

                    anyInt(),

                    anyInt(),

                    anyInt(),

                    anyOrNull(),

                    any(),

                    any(),

                    any(),

                )

        } finally {

            forVerification.release()

        }

    }



    @Test

    fun testInitializationCallback_waitsForBgSetup() {

        val inputListener = create()
@@ -155,6 +193,30 @@ class DragResizeInputListenerTest : ShellTestCase() { 
        verify(sinkInputChannel).dispose()

    }



    @Test

    fun testClose_beforeBgSetup_releaseSurfaces() {

        val inputListener = create()

        inputListener.close()

        testBgExecutor.flushAll()

        testMainExecutor.flushAll()



        assertThat(createdSurfaces).hasSize(1)

        assertThat(createdSurfaces[0].isValid).isFalse()

    }



    @Test

    fun testClose_afterBgSetup_releaseSurfaces() {

        val inputListener = create()

        testBgExecutor.flushAll()

        inputListener.close()

        testMainExecutor.flushAll()

        testBgExecutor.flushAll()



        assertThat(createdSurfaces).hasSize(2)

        assertThat(createdSurfaces[0].isValid).isFalse()

        assertThat(createdSurfaces[1].isValid).isFalse()

    }



    private fun verifyNoInputChannelGrantRequests() {

        verify(mockWindowSession, never())

            .grantInputChannel(
@@ -184,10 +246,22 @@ class DragResizeInputListenerTest : ShellTestCase() { 
            TestHandler(Looper.getMainLooper()),

            mock<Choreographer>(),

            Display.DEFAULT_DISPLAY,

            mock<SurfaceControl>(),

            decorationSurface,

            mock<DragPositioningCallback>(),

            { SurfaceControl.Builder() },

            { StubTransaction() },

            {

                object : SurfaceControl.Builder() {

                    override fun build(): SurfaceControl {

                        return super.build().also { createdSurfaces.add(it) }

                    }

                }

            },

            {

                object : StubTransaction() {

                    override fun remove(sc: SurfaceControl): SurfaceControl.Transaction {

                        return super.remove(sc).also { sc.release() }

                    }

                }

            },

            mock<DisplayController>(),

            mock<DesktopModeEventLogger>(),

            inputChannel,