Merge "Add registry for tracking leaking surface controls" into udc-dev am: 61df74d896

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/21647025

Change-Id: I57b24449b17eda61364ecf268a70c9bbeed491ef
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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<SyncFence> callback, long nativeFencePtr) {
         SyncFence fence = new SyncFence(nativeFencePtr);

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<SurfaceControl, Long> 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<SurfaceControl, Long> surfaceControls,
+                int limit, PrintWriter pw) {
+            final int size = Math.min(surfaceControls.size(), limit);
+            final long now = SystemClock.elapsedRealtime();
+            final ArrayList<Map.Entry<SurfaceControl, Long>> entries = new ArrayList<>();
+            for (Map.Entry<SurfaceControl, Long> 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<SurfaceControl, Long> 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<SurfaceControl, Long> 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);
+            }
+        }
+    }
+}

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<SurfaceControl, Long> lastSurfaceControls = new WeakHashMap<>();
+        int callCount = 0;
+
+        @Override
+        public void onMaxLayersExceeded(WeakHashMap<SurfaceControl, Long> 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<SurfaceControl, Long> last = new WeakHashMap<>(lastSurfaceControls);
+            for (int i = 0; i < surfaces.length; i++) {
+                last.remove(surfaces[i]);
+            }
+            if (!last.isEmpty()) {
+                fail("Sets differ");
+            }
+        }
+    }
+}