From 5409e24105ba32dc524a78b9be3d0f2d3beef69a Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Fri, 19 Jun 2020 13:08:22 -0700 Subject: [PATCH 1/2] SurfaceControlViewHost: Call doDie with true If we pass immediate=false to the doDie without updating the visibility, we may destroy the hardware renderer but then try to draw again anyway, and then crash. It seems theres no reason we can't call immediate=true and properly shut down immediately. Bug: 159250432 Test: Existing tests pass Change-Id: Ibc6be8901be10c0985681d83f4ce16dd1ab54925 --- core/java/android/view/SurfaceControlViewHost.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/java/android/view/SurfaceControlViewHost.java b/core/java/android/view/SurfaceControlViewHost.java index 385078165e844..df4c084e73490 100644 --- a/core/java/android/view/SurfaceControlViewHost.java +++ b/core/java/android/view/SurfaceControlViewHost.java @@ -273,6 +273,6 @@ public class SurfaceControlViewHost { */ public void release() { // ViewRoot will release mSurfaceControl for us. - mViewRoot.die(false /* immediate */); + mViewRoot.die(true /* immediate */); } } From 7a31b8b1df353e2a8c5245b645bb1c836e37c51a Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Fri, 19 Jun 2020 13:14:00 -0700 Subject: [PATCH 2/2] SurfaceControlViewHost: Release ViewRoot from finalizer If we don't explicitly go through the doDie process the WindowManagerGlobal instance will hold the ViewRoot alive indefinitely. This accidental leak could be very expensive so we prevent it with a finalizer. Bug: 157709599 Test: Existing tests pass Change-Id: I4fdf368eed4b4e43faacd9f62b6d9fddfd9b7ef2 --- core/java/android/view/SurfaceControlViewHost.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/core/java/android/view/SurfaceControlViewHost.java b/core/java/android/view/SurfaceControlViewHost.java index df4c084e73490..86a4fe170387f 100644 --- a/core/java/android/view/SurfaceControlViewHost.java +++ b/core/java/android/view/SurfaceControlViewHost.java @@ -177,6 +177,17 @@ public class SurfaceControlViewHost { mAccessibilityEmbeddedConnection = mViewRoot.getAccessibilityEmbeddedConnection(); } + /** + * @hide + */ + @Override + protected void finalize() throws Throwable { + // We aren't on the UI thread here so we need to pass false to + // doDie + mViewRoot.die(false /* immediate */); + } + + /** * Return a SurfacePackage for the root SurfaceControl of the embedded hierarchy. * Rather than be directly reparented using {@link SurfaceControl.Transaction} this