Merge "Fix potential ASurfaceTransactionCallback leaks" into sc-dev

graphics/java/android/graphics/HardwareRenderer.java

@@ -753,8 +753,12 @@ public class HardwareRenderer {
         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

@@ -500,6 +500,28 @@ private:
     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 @@ static void android_view_ThreadedRenderer_setASurfaceTransactionCallback(
     } else {
         JavaVM* vm = nullptr;
         LOG_ALWAYS_FATAL_IF(env->GetJavaVM(&vm) != JNI_OK, "Unable to get Java VM");
-        auto globalCallbackRef = std::make_shared<JGlobalRefHolder>(
+        auto globalCallbackRef =
-                vm, env->NewGlobalRef(aSurfaceTransactionCallback));
+                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(),
+                    jobject localref = env->NewLocalRef(globalCallbackRef->ref());
-                                        gASurfaceTransactionCallback.onMergeTransaction,
+                    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);
                 });
     }
 }