commit 1d88151a380c2a8f102c76b8413d17989215ccf8
author Huihong Luo <huisinro@google.com> Mon Jun 14 12:01:40 2021 -0700
committer Huihong Luo <huisinro@google.com> Wed Jun 16 09:47:04 2021 -0700
tree 4ce87940806d456e8ad0c5d835c06bd80b29cb7a
parent 41e7f367edbf9d1819b8e593f71d2fbc1bdc143a

Fix potential ASurfaceTransactionCallback leaks

Change the rerferene to java object from global reference to weak
reference.

Bug: 188555236
Test: check number of ViewRootImpl objects from Systrace
Change-Id: I52347b9efb211691826635d046259bc3c54b194f

graphics/java/android/graphics/HardwareRenderer.java

diff --git a/graphics/java/android/graphics/HardwareRenderer.java b/graphics/java/android/graphics/HardwareRenderer.java
index 954d062..6aa74cb 100644
--- a/graphics/java/android/graphics/HardwareRenderer.java
+++ b/graphics/java/android/graphics/HardwareRenderer.java
@@ -753,8 +753,12 @@
         nCancelLayerUpdate(mNativeProxy, layer.getDeferredLayerUpdater());
     }
 
+    private ASurfaceTransactionCallback mASurfaceTransactionCallback;
+
     /** @hide */
     public void setASurfaceTransactionCallback(ASurfaceTransactionCallback callback) {
+        // ensure callback is kept alive on the java side since weak ref is used in native code
+        mASurfaceTransactionCallback = callback;
         nSetASurfaceTransactionCallback(mNativeProxy, callback);
     }
 

libs/hwui/jni/android_graphics_HardwareRenderer.cpp

diff --git a/libs/hwui/jni/android_graphics_HardwareRenderer.cpp b/libs/hwui/jni/android_graphics_HardwareRenderer.cpp
index 602c32a..819a34b 100644
--- a/libs/hwui/jni/android_graphics_HardwareRenderer.cpp
+++ b/libs/hwui/jni/android_graphics_HardwareRenderer.cpp
@@ -500,6 +500,28 @@
     jobject mObject;
 };
 
+class JWeakGlobalRefHolder {
+public:
+    JWeakGlobalRefHolder(JavaVM* vm, jobject object) : mVm(vm) {
+        mWeakRef = getenv(vm)->NewWeakGlobalRef(object);
+    }
+
+    virtual ~JWeakGlobalRefHolder() {
+        if (mWeakRef != nullptr) getenv(mVm)->DeleteWeakGlobalRef(mWeakRef);
+        mWeakRef = nullptr;
+    }
+
+    jobject ref() { return mWeakRef; }
+    JavaVM* vm() { return mVm; }
+
+private:
+    JWeakGlobalRefHolder(const JWeakGlobalRefHolder&) = delete;
+    void operator=(const JWeakGlobalRefHolder&) = delete;
+
+    JavaVM* mVm;
+    jobject mWeakRef;
+};
+
 using TextureMap = std::unordered_map<uint32_t, sk_sp<SkImage>>;
 
 struct PictureCaptureState {
@@ -633,15 +655,19 @@
     } else {
         JavaVM* vm = nullptr;
         LOG_ALWAYS_FATAL_IF(env->GetJavaVM(&vm) != JNI_OK, "Unable to get Java VM");
-        auto globalCallbackRef = std::make_shared<JGlobalRefHolder>(
-                vm, env->NewGlobalRef(aSurfaceTransactionCallback));
+        auto globalCallbackRef =
+                std::make_shared<JWeakGlobalRefHolder>(vm, aSurfaceTransactionCallback);
         proxy->setASurfaceTransactionCallback(
                 [globalCallbackRef](int64_t transObj, int64_t scObj, int64_t frameNr) {
                     JNIEnv* env = getenv(globalCallbackRef->vm());
-                    env->CallVoidMethod(globalCallbackRef->object(),
-                                        gASurfaceTransactionCallback.onMergeTransaction,
+                    jobject localref = env->NewLocalRef(globalCallbackRef->ref());
+                    if (CC_UNLIKELY(!localref)) {
+                        return;
+                    }
+                    env->CallVoidMethod(localref, gASurfaceTransactionCallback.onMergeTransaction,
                                         static_cast<jlong>(transObj), static_cast<jlong>(scObj),
                                         static_cast<jlong>(frameNr));
+                    env->DeleteLocalRef(localref);
                 });
     }
 }