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

Commit cfeff743 authored by Jorim Jaggi's avatar Jorim Jaggi
Browse files

Fix starting window leak when adding/removing quickly 

The following race condition may happen when transferring starting
windows:
- Add thread (android.anim) calls Snapshot.create
- Remove thread (android.display!) checks that yes there is a
starting window, but no surface yet. Thus, it nulls out everything.
- Add thread returns from Snapshot.create: container.startingData
is null but startingWindow is also null. Thus, we never set
abort=true and we never set the starting window properly or remove
it properly.

Test: Some basic starting window sanity testing. Otherwise, pray
to the race condition gods.
Test: AppWindowContainerControllerTests#testAddRemoveRace
Fixes: 37888853

Change-Id: Ia287bfa52f708b36ff93ef7cd45a0d080bd4dd9c
parent 23cc9aa5

services/core/java/com/android/server/wm/AppWindowContainerController.java

+10 −12
Original line number Diff line number Diff line
@@ -107,7 +107,7 @@ public class AppWindowContainerController 
            if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM, "Remove starting " + mContainer

                    + ": startingWindow=" + mContainer.startingWindow

                    + " startingView=" + mContainer.startingSurface);

            if (mContainer.startingWindow != null) {

            if (mContainer.startingData != null) {

                surface = mContainer.startingSurface;

                mContainer.startingData = null;

                mContainer.startingSurface = null;
@@ -164,10 +164,9 @@ public class AppWindowContainerController 
        if (surface != null) {

            boolean abort = false;

            synchronized(mWindowMap) {

                if (container.removed || container.startingData == null) {

                // If the window was successfully added, then

                // we need to remove it.

                    if (container.startingWindow != null) {

                if (container.removed || container.startingData == null) {

                    if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM,

                            "Aborted starting " + container

                                    + ": removed=" + container.removed
@@ -175,7 +174,6 @@ public class AppWindowContainerController 
                    container.startingWindow = null;

                    container.startingData = null;

                    abort = true;

                    }

                } else {

                    container.startingSurface = surface;

                }

services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java

+25 −0
Original line number Diff line number Diff line
@@ -19,18 +19,24 @@ package com.android.server.wm; 
import org.junit.Test;



import android.platform.test.annotations.Presubmit;

import android.platform.test.annotations.SecurityTest;

import android.support.test.InstrumentationRegistry;

import android.support.test.filters.SmallTest;

import android.support.test.runner.AndroidJUnit4;

import android.view.WindowManager;



import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE;

import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED;

import static android.content.res.Configuration.EMPTY;

import static android.view.WindowManager.LayoutParams.TYPE_APPLICATION_STARTING;

import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertNotNull;

import static org.junit.Assert.assertNull;

import static org.junit.Assert.fail;



import java.util.function.Consumer;



/**

 * Test class for {@link AppWindowContainerController}.

 *
@@ -90,6 +96,9 @@ public class AppWindowContainerControllerTests extends WindowTestsBase { 
        assertNull(atoken.startingSurface);

        assertNull(atoken.startingWindow);

        assertNull(atoken.startingData);

        atoken.forAllWindows(windowState -> {

            assertFalse(windowState.getBaseType() == TYPE_APPLICATION_STARTING);

        }, true);

    }



    @Test
@@ -107,6 +116,22 @@ public class AppWindowContainerControllerTests extends WindowTestsBase { 
        assertNoStartingWindow(atoken);

    }



    @Test

    public void testAddRemoveRace() throws Exception {



        // There was once a race condition between adding and removing starting windows

        for (int i = 0; i < 1000; i++) {

            final WindowTestUtils.TestAppWindowContainerController controller =

                    createAppWindowController();

            controller.addStartingWindow(InstrumentationRegistry.getContext().getPackageName(),

                    android.R.style.Theme, null, "Test", 0, 0, 0, 0, null, true, true, false, true,

                    false);

            controller.removeStartingWindow();

            waitUntilHandlersIdle();

            assertNoStartingWindow(controller.getAppWindowToken(mDisplayContent));

        }

    }



    @Test

    public void testTransferStartingWindow() throws Exception {

        final WindowTestUtils.TestAppWindowContainerController controller1 =