diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index 866301399fd65..7da141bc392ad 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -479,6 +479,14 @@ public final class SurfaceControl implements Parcelable { private WeakReference 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; } diff --git a/services/tests/wmtests/src/com/android/server/wm/SurfaceControlTests.java b/services/tests/wmtests/src/com/android/server/wm/SurfaceControlTests.java new file mode 100644 index 0000000000000..a27a5fd8a2869 --- /dev/null +++ b/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(); + } +}