diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index 62fdfaea13037..ddaa71c6b1b59 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -461,6 +461,7 @@ public final class SurfaceControl implements Parcelable { private final CloseGuard mCloseGuard = CloseGuard.get(); private String mName; + private String mCallsite; /** * Note: do not rename, this field is used by native code. @@ -774,7 +775,6 @@ public final class SurfaceControl implements Parcelable { release(); } if (nativeObject != 0) { - mCloseGuard.openWithCallSite("release", callsite); mFreeNativeResources = sRegistry.registerNativeAllocation(this, nativeObject); } @@ -785,6 +785,8 @@ public final class SurfaceControl implements Parcelable { } else { mReleaseStack = null; } + setUnreleasedWarningCallSite(callsite); + addToRegistry(); } /** @@ -1322,6 +1324,23 @@ public final class SurfaceControl implements Parcelable { return; } mCloseGuard.openWithCallSite("release", callsite); + mCallsite = callsite; + } + + /** + * Returns the last provided call site when this SurfaceControl was created. + * @hide + */ + @Nullable String getCallsite() { + return mCallsite; + } + + /** + * Returns the name of this SurfaceControl, mainly for debugging purposes. + * @hide + */ + @NonNull String getName() { + return mName; } /** @@ -1435,6 +1454,7 @@ public final class SurfaceControl implements Parcelable { if (mCloseGuard != null) { mCloseGuard.warnIfOpen(); } + removeFromRegistry(); } finally { super.finalize(); } @@ -1465,6 +1485,7 @@ public final class SurfaceControl implements Parcelable { mChoreographer = null; } } + removeFromRegistry(); } } @@ -4304,6 +4325,26 @@ public final class SurfaceControl implements Parcelable { return -1; } + /** + * Adds this surface control to the registry for this process if it is created. + */ + private void addToRegistry() { + final SurfaceControlRegistry registry = SurfaceControlRegistry.getProcessInstance(); + if (registry != null) { + registry.add(this); + } + } + + /** + * Removes this surface control from the registry for this process. + */ + private void removeFromRegistry() { + final SurfaceControlRegistry registry = SurfaceControlRegistry.getProcessInstance(); + if (registry != null) { + registry.remove(this); + } + } + // Called by native private static void invokeReleaseCallback(Consumer callback, long nativeFencePtr) { SyncFence fence = new SyncFence(nativeFencePtr); diff --git a/core/java/android/view/SurfaceControlRegistry.java b/core/java/android/view/SurfaceControlRegistry.java new file mode 100644 index 0000000000000..095189ad03a78 --- /dev/null +++ b/core/java/android/view/SurfaceControlRegistry.java @@ -0,0 +1,273 @@ +/* + * 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 android.view; + +import static android.Manifest.permission.READ_FRAME_BUFFER; +import static android.content.pm.PackageManager.PERMISSION_GRANTED; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.annotation.RequiresPermission; +import android.content.Context; +import android.os.SystemClock; +import android.util.Log; + +import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.util.GcUtils; + +import java.io.PrintWriter; +import java.util.ArrayList; +import java.util.Map; +import java.util.WeakHashMap; + +/** + * A thread-safe registry used to track surface controls that are active (not yet released) within a + * process, to help debug and identify leaks. + * @hide + */ +public class SurfaceControlRegistry { + private static final String TAG = "SurfaceControlRegistry"; + + /** + * An interface for processing the registered SurfaceControls when the threshold is exceeded. + */ + public interface Reporter { + /** + * Called when the set of layers exceeds the max threshold. This can be called on any + * thread, and must be handled synchronously. + */ + void onMaxLayersExceeded(WeakHashMap surfaceControls, int limit, + PrintWriter pw); + } + + /** + * The default implementation of the reporter which logs the existing registered surfaces to + * logcat. + */ + private static class DefaultReporter implements Reporter { + public void onMaxLayersExceeded(WeakHashMap surfaceControls, + int limit, PrintWriter pw) { + final int size = Math.min(surfaceControls.size(), limit); + final long now = SystemClock.elapsedRealtime(); + final ArrayList> entries = new ArrayList<>(); + for (Map.Entry entry : surfaceControls.entrySet()) { + entries.add(entry); + } + // Sort entries by time registered when dumping + // TODO: Or should it sort by name? + entries.sort((o1, o2) -> (int) (o1.getValue() - o2.getValue())); + + pw.println("SurfaceControlRegistry"); + pw.println("----------------------"); + pw.println("Listing oldest " + size + " of " + surfaceControls.size()); + for (int i = 0; i < size; i++) { + final Map.Entry entry = entries.get(i); + final SurfaceControl sc = entry.getKey(); + final long timeRegistered = entry.getValue(); + pw.print(" "); + pw.print(sc.getName()); + pw.print(" (" + sc.getCallsite() + ")"); + pw.println(" [" + ((now - timeRegistered) / 1000) + "s ago]"); + } + } + } + + // The threshold at which to dump information about all the known active SurfaceControls in the + // process when the number of layers exceeds a certain count. This should be significantly + // smaller than the MAX_LAYERS (currently 4096) defined in SurfaceFlinger.h + private static final int MAX_LAYERS_REPORTING_THRESHOLD = 1024; + + // The threshold at which to reset the dump state. Needs to be smaller than + // MAX_LAYERS_REPORTING_THRESHOLD + private static final int RESET_REPORTING_THRESHOLD = 256; + + // Number of surface controls to dump when the max threshold is exceeded + private static final int DUMP_LIMIT = 256; + + // Static lock, must be held for all registry operations + private static final Object sLock = new Object(); + + // The default reporter for printing out the registered surfaces + private static final DefaultReporter sDefaultReporter = new DefaultReporter(); + + // The registry for a given process + private static volatile SurfaceControlRegistry sProcessRegistry; + + // Mapping of the active SurfaceControls to the elapsed time when they were registered + @GuardedBy("sLock") + private final WeakHashMap mSurfaceControls; + + // The threshold at which we dump information about the current set of registered surfaces. + // Once this threshold is reached, we no longer report until the number of layers drops below + // mResetReportingThreshold to ensure that we don't spam logcat. + private int mMaxLayersReportingThreshold = MAX_LAYERS_REPORTING_THRESHOLD; + private int mResetReportingThreshold = RESET_REPORTING_THRESHOLD; + + // Whether the current set of layers has exceeded mMaxLayersReportingThreshold, and we have + // already reported the set of registered surfaces. + private boolean mHasReportedExceedingMaxThreshold = false; + + // The handler for when the registry exceeds the max threshold + private Reporter mReporter = sDefaultReporter; + + private SurfaceControlRegistry() { + mSurfaceControls = new WeakHashMap<>(256); + } + + /** + * Sets the thresholds at which the registry reports errors. + * @param maxLayersReportingThreshold The max threshold (inclusive) + * @param resetReportingThreshold The reset threshold (inclusive) + * @hide + */ + @VisibleForTesting + public void setReportingThresholds(int maxLayersReportingThreshold, int resetReportingThreshold, + Reporter reporter) { + synchronized (sLock) { + if (maxLayersReportingThreshold <= 0 + || resetReportingThreshold >= maxLayersReportingThreshold) { + throw new IllegalArgumentException("Expected maxLayersReportingThreshold (" + + maxLayersReportingThreshold + ") to be > 0 and resetReportingThreshold (" + + resetReportingThreshold + ") to be < maxLayersReportingThreshold"); + } + if (reporter == null) { + throw new IllegalArgumentException("Expected non-null reporter"); + } + mMaxLayersReportingThreshold = maxLayersReportingThreshold; + mResetReportingThreshold = resetReportingThreshold; + mHasReportedExceedingMaxThreshold = false; + mReporter = reporter; + } + } + + /** + * Creates and initializes the registry for all SurfaceControls in this process. The caller must + * hold the READ_FRAME_BUFFER permission. + * @hide + */ + @RequiresPermission(READ_FRAME_BUFFER) + @NonNull + public static void createProcessInstance(Context context) { + if (context.checkSelfPermission(READ_FRAME_BUFFER) != PERMISSION_GRANTED) { + throw new SecurityException("Expected caller to hold READ_FRAME_BUFFER"); + } + synchronized (sLock) { + if (sProcessRegistry == null) { + sProcessRegistry = new SurfaceControlRegistry(); + } + } + } + + /** + * Destroys the previously created registry this process. + * @hide + */ + public static void destroyProcessInstance() { + synchronized (sLock) { + if (sProcessRegistry == null) { + return; + } + sProcessRegistry = null; + } + } + + /** + * Returns the instance of the registry for this process, only non-null if + * createProcessInstance(Context) was previously called from a valid caller. + * @hide + */ + @Nullable + @VisibleForTesting + public static SurfaceControlRegistry getProcessInstance() { + synchronized (sLock) { + return sProcessRegistry; + } + } + + /** + * Adds a SurfaceControl to the registry. + */ + void add(SurfaceControl sc) { + synchronized (sLock) { + mSurfaceControls.put(sc, SystemClock.elapsedRealtime()); + if (!mHasReportedExceedingMaxThreshold + && mSurfaceControls.size() >= mMaxLayersReportingThreshold) { + // Dump existing info to logcat for debugging purposes (but don't close the + // System.out output stream otherwise we can't print to it after this call) + PrintWriter pw = new PrintWriter(System.out, true /* autoFlush */); + mReporter.onMaxLayersExceeded(mSurfaceControls, DUMP_LIMIT, pw); + mHasReportedExceedingMaxThreshold = true; + } + } + } + + /** + * Removes a SurfaceControl from the registry. + */ + void remove(SurfaceControl sc) { + synchronized (sLock) { + mSurfaceControls.remove(sc); + if (mHasReportedExceedingMaxThreshold + && mSurfaceControls.size() <= mResetReportingThreshold) { + mHasReportedExceedingMaxThreshold = false; + } + } + } + + /** + * Returns a hash of this registry and is a function of all the active surface controls. This + * is useful for testing to determine whether the registry has changed between creating and + * destroying new SurfaceControls. + */ + @Override + public int hashCode() { + synchronized (sLock) { + // Return a hash of the surface controls + return mSurfaceControls.keySet().hashCode(); + } + } + + /** + * Forces the gc and finalizers to run, used prior to dumping to ensure we only dump strongly + * referenced surface controls. + */ + private static void runGcAndFinalizers() { + long t = SystemClock.elapsedRealtime(); + GcUtils.runGcAndFinalizersSync(); + Log.i(TAG, "Ran gc and finalizers (" + (SystemClock.elapsedRealtime() - t) + "ms)"); + } + + /** + * Dumps information about the set of SurfaceControls in the registry. + * + * @param limit the number of layers to report + * @param runGc whether to run the GC and finalizers before dumping + * @hide + */ + public static void dump(int limit, boolean runGc, PrintWriter pw) { + if (runGc) { + // This needs to run outside the lock since finalization isn't synchronous + runGcAndFinalizers(); + } + synchronized (sLock) { + if (sProcessRegistry != null) { + sDefaultReporter.onMaxLayersExceeded(sProcessRegistry.mSurfaceControls, limit, pw); + } + } + } +} diff --git a/core/tests/coretests/src/android/view/SurfaceControlRegistryTests.java b/core/tests/coretests/src/android/view/SurfaceControlRegistryTests.java new file mode 100644 index 0000000000000..d10ba7ccbac47 --- /dev/null +++ b/core/tests/coretests/src/android/view/SurfaceControlRegistryTests.java @@ -0,0 +1,173 @@ +/* + * 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 android.view; + +import static android.Manifest.permission.READ_FRAME_BUFFER; +import static android.content.pm.PackageManager.PERMISSION_DENIED; + +import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; + +import android.content.Context; +import android.platform.test.annotations.Presubmit; + +import androidx.test.runner.AndroidJUnit4; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.io.PrintWriter; +import java.util.WeakHashMap; + +/** + * Class for testing {@link SurfaceControlRegistry}. + * + * Build/Install/Run: + * atest FrameworksCoreTests:android.view.SurfaceControlRegistryTests + */ +@Presubmit +@RunWith(AndroidJUnit4.class) +public class SurfaceControlRegistryTests { + + @BeforeClass + public static void setUpOnce() { + SurfaceControlRegistry.createProcessInstance(getInstrumentation().getTargetContext()); + } + + @AfterClass + public static void tearDownOnce() { + SurfaceControlRegistry.destroyProcessInstance(); + } + + @Test + public void testRequiresPermissionToCreateProcessInstance() { + try { + Context ctx = mock(Context.class); + doReturn(PERMISSION_DENIED).when(ctx).checkSelfPermission(eq(READ_FRAME_BUFFER)); + SurfaceControlRegistry.createProcessInstance(ctx); + fail("Expected SecurityException due to missing permission"); + } catch (SecurityException se) { + // Expected failure + } catch (Exception e) { + fail("Unexpected exception: " + e); + } + } + + @Test + public void testCreateReleaseSurfaceControl() { + int hash0 = SurfaceControlRegistry.getProcessInstance().hashCode(); + SurfaceControl sc = buildTestSurface(); + assertNotEquals(hash0, SurfaceControlRegistry.getProcessInstance().hashCode()); + sc.release(); + assertEquals(hash0, SurfaceControlRegistry.getProcessInstance().hashCode()); + } + + @Test + public void testCreateReleaseMultipleSurfaceControls() { + int hash0 = SurfaceControlRegistry.getProcessInstance().hashCode(); + SurfaceControl sc1 = buildTestSurface(); + int hash1 = SurfaceControlRegistry.getProcessInstance().hashCode(); + assertNotEquals(hash0, hash1); + SurfaceControl sc2 = buildTestSurface(); + int hash1_2 = SurfaceControlRegistry.getProcessInstance().hashCode(); + assertNotEquals(hash0, hash1_2); + assertNotEquals(hash1, hash1_2); + // Release in inverse order to verify hashes still differ + sc1.release(); + int hash2 = SurfaceControlRegistry.getProcessInstance().hashCode(); + assertNotEquals(hash0, hash2); + sc2.release(); + assertEquals(hash0, SurfaceControlRegistry.getProcessInstance().hashCode()); + } + + @Test + public void testThresholds() { + SurfaceControlRegistry registry = SurfaceControlRegistry.getProcessInstance(); + TestReporter reporter = new TestReporter(); + registry.setReportingThresholds(4 /* max */, 2 /* reset */, reporter); + + // Exceed the threshold ensure the callback is made + SurfaceControl sc1 = buildTestSurface(); + SurfaceControl sc2 = buildTestSurface(); + SurfaceControl sc3 = buildTestSurface(); + SurfaceControl sc4 = buildTestSurface(); + reporter.assertMaxThresholdExceededCallCount(1); + reporter.assertLastReportedSetEquals(sc1, sc2, sc3, sc4); + + // Create a few more, ensure we don't report again for the time being + SurfaceControl sc5 = buildTestSurface(); + SurfaceControl sc6 = buildTestSurface(); + reporter.assertMaxThresholdExceededCallCount(1); + reporter.assertLastReportedSetEquals(sc1, sc2, sc3, sc4); + + // Release down to the reset threshold + sc1.release(); + sc2.release(); + sc3.release(); + sc4.release(); + + // Create a few more to hit the max threshold again + SurfaceControl sc7 = buildTestSurface(); + SurfaceControl sc8 = buildTestSurface(); + reporter.assertMaxThresholdExceededCallCount(2); + reporter.assertLastReportedSetEquals(sc5, sc6, sc7, sc8); + } + + private SurfaceControl buildTestSurface() { + return new SurfaceControl.Builder() + .setContainerLayer() + .setName("SurfaceControlRegistryTests") + .setCallsite("SurfaceControlRegistryTests") + .build(); + } + + private static class TestReporter implements SurfaceControlRegistry.Reporter { + WeakHashMap lastSurfaceControls = new WeakHashMap<>(); + int callCount = 0; + + @Override + public void onMaxLayersExceeded(WeakHashMap surfaceControls, + int limit, PrintWriter pw) { + lastSurfaceControls.clear(); + lastSurfaceControls.putAll(surfaceControls); + callCount++; + } + + public void assertMaxThresholdExceededCallCount(int count) { + assertTrue("Expected " + count + " got " + callCount, count == callCount); + } + + public void assertLastReportedSetEquals(SurfaceControl... surfaces) { + WeakHashMap last = new WeakHashMap<>(lastSurfaceControls); + for (int i = 0; i < surfaces.length; i++) { + last.remove(surfaces[i]); + } + if (!last.isEmpty()) { + fail("Sets differ"); + } + } + } +}