From 26ed47443cdaafee8c09268b3755835b33276ef6 Mon Sep 17 00:00:00 2001
From: Vishnu Nair <vishnun@google.com>
Date: Mon, 30 Aug 2021 15:01:29 -0700
Subject: [PATCH] Surface: Release references to BlastBufferQueue and
 SurfaceControl on Surface#destroy

SurfaceView clients may hold on to surface references. In S this means
they would extend the lifetime of the SurfaceControl resulting in
"leaking" buffers until the references are cleared or the app is
terminated.

Fix this by calling a new destroy function on Surface which will
explicitly remove references to the SurfaceControl and BBQ it holds.
This is safe because SurfaceView controls the lifecycle of the Surface
and knows when the Surface will become invalid. Once invalid, the Surface
cannot become valid again.

Test: repro steps in bug
Bug: 198133921
Change-Id: I5c7e43736f025fc0965eae2f19719ba40df3cb70
---
 core/java/android/view/Surface.java     | 4 ++++
 core/java/android/view/SurfaceView.java | 2 +-
 core/jni/android_view_Surface.cpp       | 6 ++++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/core/java/android/view/Surface.java b/core/java/android/view/Surface.java
index aaf53ee1cfc10..904aa73f6ac46 100644
--- a/core/java/android/view/Surface.java
+++ b/core/java/android/view/Surface.java
@@ -99,6 +99,7 @@ public class Surface implements Parcelable {
 
     private static native int nativeSetFrameRate(
             long nativeObject, float frameRate, int compatibility, int changeFrameRateStrategy);
+    private static native void nativeDestroy(long nativeObject);
 
     public static final @android.annotation.NonNull Parcelable.Creator<Surface> CREATOR =
             new Parcelable.Creator<Surface>() {
@@ -340,6 +341,9 @@ public class Surface implements Parcelable {
      */
     @UnsupportedAppUsage
     public void destroy() {
+        if (mNativeObject != 0) {
+            nativeDestroy(mNativeObject);
+        }
         release();
     }
 
diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java
index a4e7a43cc9347..51910e09735d9 100644
--- a/core/java/android/view/SurfaceView.java
+++ b/core/java/android/view/SurfaceView.java
@@ -903,7 +903,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
         mSurfaceAlpha = 1f;
 
         synchronized (mSurfaceControlLock) {
-            mSurface.release();
+            mSurface.destroy();
             if (mBlastBufferQueue != null) {
                 mBlastBufferQueue.destroy();
                 mBlastBufferQueue = null;
diff --git a/core/jni/android_view_Surface.cpp b/core/jni/android_view_Surface.cpp
index 0957067de603b..869b53df28376 100644
--- a/core/jni/android_view_Surface.cpp
+++ b/core/jni/android_view_Surface.cpp
@@ -449,6 +449,11 @@ static jint nativeSetFrameRate(JNIEnv* env, jclass clazz, jlong nativeObject, jf
                         int(changeFrameRateStrategy));
 }
 
+static void nativeDestroy(JNIEnv* env, jclass clazz, jlong nativeObject) {
+    sp<Surface> surface(reinterpret_cast<Surface*>(nativeObject));
+    surface->destroy();
+}
+
 // ----------------------------------------------------------------------------
 
 static const JNINativeMethod gSurfaceMethods[] = {
@@ -477,6 +482,7 @@ static const JNINativeMethod gSurfaceMethods[] = {
         {"nativeSetAutoRefreshEnabled", "(JZ)I", (void*)nativeSetAutoRefreshEnabled},
         {"nativeSetFrameRate", "(JFII)I", (void*)nativeSetFrameRate},
         {"nativeGetFromBlastBufferQueue", "(JJ)J", (void*)nativeGetFromBlastBufferQueue},
+        {"nativeDestroy", "(J)V", (void*)nativeDestroy},
 };
 
 int register_android_view_Surface(JNIEnv* env)