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
(cherry picked from commit cfeff74317f19d5b90335c3e458ef055bdbd17cd)
diff --git a/services/core/java/com/android/server/wm/AppWindowContainerController.java b/services/core/java/com/android/server/wm/AppWindowContainerController.java
index c982f08..65efa70 100644
--- a/services/core/java/com/android/server/wm/AppWindowContainerController.java
+++ b/services/core/java/com/android/server/wm/AppWindowContainerController.java
@@ -107,7 +107,7 @@
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,18 +164,16 @@
if (surface != null) {
boolean abort = false;
synchronized(mWindowMap) {
+ // If the window was successfully added, then
+ // we need to remove it.
if (container.removed || container.startingData == null) {
- // If the window was successfully added, then
- // we need to remove it.
- if (container.startingWindow != null) {
- if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM,
- "Aborted starting " + container
- + ": removed=" + container.removed
- + " startingData=" + container.startingData);
- container.startingWindow = null;
- container.startingData = null;
- abort = true;
- }
+ if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM,
+ "Aborted starting " + container
+ + ": removed=" + container.removed
+ + " startingData=" + container.startingData);
+ container.startingWindow = null;
+ container.startingData = null;
+ abort = true;
} else {
container.startingSurface = surface;
}
diff --git a/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java b/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java
index da3b9c9..65a5632 100644
--- a/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java
+++ b/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java
@@ -19,18 +19,24 @@
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 @@
assertNull(atoken.startingSurface);
assertNull(atoken.startingWindow);
assertNull(atoken.startingData);
+ atoken.forAllWindows(windowState -> {
+ assertFalse(windowState.getBaseType() == TYPE_APPLICATION_STARTING);
+ }, true);
}
@Test
@@ -108,6 +117,22 @@
}
@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 =
createAppWindowController();
