Merge "Add additional usage-after-release tracking"

core/java/android/view/SurfaceControl.java

@@ -479,6 +479,14 @@ public final class SurfaceControl implements Parcelable {
 
     private WeakReference<View> mLocalOwnerView;
 
+    // A throwable with the stack filled when this SurfaceControl is released (only if
+    // sDebugUsageAfterRelease) is enabled
+    private Throwable mReleaseStack = null;
+
+    // Triggers the stack to be saved when any SurfaceControl in this process is released, which can
+    // be dumped as additional context
+    private static volatile boolean sDebugUsageAfterRelease = false;
+
     static GlobalTransactionWrapper sGlobalTransaction;
     static long sTransactionNestCount = 0;
 
@@ -751,6 +759,11 @@ public final class SurfaceControl implements Parcelable {
         }
         mNativeObject = nativeObject;
         mNativeHandle = mNativeObject != 0 ? nativeGetHandle(nativeObject) : 0;
+        if (sDebugUsageAfterRelease && mNativeObject == 0) {
+            mReleaseStack = new Throwable("Assigned invalid nativeObject");
+        } else {
+            mReleaseStack = null;
+        }
     }
 
     /**
@@ -1246,6 +1259,9 @@ public final class SurfaceControl implements Parcelable {
 
     @Override
     public void writeToParcel(Parcel dest, int flags) {
+        if (sDebugUsageAfterRelease) {
+            checkNotReleased();
+        }
         dest.writeString8(mName);
         dest.writeInt(mWidth);
         dest.writeInt(mHeight);
@@ -1261,6 +1277,18 @@ public final class SurfaceControl implements Parcelable {
         }
     }
 
+    /**
+     * Enables additional debug logs to track usage-after-release of all SurfaceControls in this
+     * process.
+     * @hide
+     */
+    public static void setDebugUsageAfterRelease(boolean debug) {
+        if (!Build.isDebuggable()) {
+            return;
+        }
+        sDebugUsageAfterRelease = debug;
+    }
+
     /**
      * Checks whether two {@link SurfaceControl} objects represent the same surface.
      *
@@ -1382,6 +1410,9 @@ public final class SurfaceControl implements Parcelable {
             mFreeNativeResources.run();
             mNativeObject = 0;
             mNativeHandle = 0;
+            if (sDebugUsageAfterRelease) {
+                mReleaseStack = new Throwable("Released");
+            }
             mCloseGuard.close();
             synchronized (mChoreographerLock) {
                 if (mChoreographer != null) {
@@ -1403,8 +1434,15 @@ public final class SurfaceControl implements Parcelable {
     }
 
     private void checkNotReleased() {
-        if (mNativeObject == 0) throw new NullPointerException(
-                "Invalid " + this + ", mNativeObject is null. Have you called release() already?");
+        if (mNativeObject == 0) {
+            if (mReleaseStack != null) {
+                throw new IllegalStateException("Invalid usage after release of " + this,
+                        mReleaseStack);
+            } else {
+                throw new NullPointerException("mNativeObject of " + this
+                        + " is null. Have you called release() already?");
+            }
+        }
     }
 
     /**
@@ -2738,7 +2776,7 @@ public final class SurfaceControl implements Parcelable {
          */
         @NonNull
         public Transaction setFrameRateSelectionPriority(@NonNull SurfaceControl sc, int priority) {
-            sc.checkNotReleased();
+            checkPreconditions(sc);
             nativeSetFrameRateSelectionPriority(mNativeObject, sc.mNativeObject, priority);
             return this;
         }

services/tests/wmtests/src/com/android/server/wm/SurfaceControlTests.java

@@ -0,0 +1,116 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.wm;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+
+import android.os.Parcel;
+import android.platform.test.annotations.Presubmit;
+import android.view.SurfaceControl;
+
+import androidx.test.filters.SmallTest;
+import androidx.test.runner.AndroidJUnit4;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/**
+ * Class for testing {@link SurfaceControl}.
+ *
+ * Build/Install/Run:
+ *  atest WmTests:SurfaceControlTests
+ */
+@Presubmit
+@RunWith(AndroidJUnit4.class)
+public class SurfaceControlTests {
+
+    @SmallTest
+    @Test
+    public void testUseValidSurface() {
+        SurfaceControl sc = buildTestSurface();
+        SurfaceControl.Transaction t = new SurfaceControl.Transaction();
+        t.setVisibility(sc, false);
+        sc.release();
+    }
+
+    @SmallTest
+    @Test
+    public void testUseInvalidSurface() {
+        SurfaceControl sc = buildTestSurface();
+        SurfaceControl.Transaction t = new SurfaceControl.Transaction();
+        sc.release();
+        try {
+            t.setVisibility(sc, false);
+            fail("Expected exception from updating invalid surface");
+        } catch (Exception e) {
+            // Expected exception
+        }
+    }
+
+    @SmallTest
+    @Test
+    public void testUseInvalidSurface_debugEnabled() {
+        SurfaceControl sc = buildTestSurface();
+        SurfaceControl.Transaction t = new SurfaceControl.Transaction();
+        try {
+            SurfaceControl.setDebugUsageAfterRelease(true);
+            sc.release();
+            try {
+                t.setVisibility(sc, false);
+                fail("Expected exception from updating invalid surface");
+            } catch (IllegalStateException ise) {
+                assertNotNull(ise.getCause());
+            } catch (Exception e) {
+                fail("Expected IllegalStateException with cause");
+            }
+        } finally {
+            SurfaceControl.setDebugUsageAfterRelease(false);
+        }
+    }
+
+    @SmallTest
+    @Test
+    public void testWriteInvalidSurface_debugEnabled() {
+        SurfaceControl sc = buildTestSurface();
+        SurfaceControl.Transaction t = new SurfaceControl.Transaction();
+        Parcel p = Parcel.obtain();
+        try {
+            SurfaceControl.setDebugUsageAfterRelease(true);
+            sc.release();
+            try {
+                sc.writeToParcel(p, 0 /* flags */);
+                fail("Expected exception from writing invalid surface to parcel");
+            } catch (IllegalStateException ise) {
+                assertNotNull(ise.getCause());
+            } catch (Exception e) {
+                fail("Expected IllegalStateException with cause");
+            }
+        } finally {
+            SurfaceControl.setDebugUsageAfterRelease(false);
+            p.recycle();
+        }
+    }
+
+    private SurfaceControl buildTestSurface() {
+        return new SurfaceControl.Builder()
+                .setContainerLayer()
+                .setName("SurfaceControlTests")
+                .setCallsite("SurfaceControlTests")
+                .build();
+    }
+}