Merge "Revert "Fix the potential memory leak issue caused by setExtension."" into main am: d34c9b3d13 am: b4e9bd1333

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

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

2 files changed, 23 insertions, 27 deletions

diff --git a/core/java/android/os/Binder.java b/core/java/android/os/Binder.java
index 6b1e918a3c47..ee62dea7f9e5 100644
--- a/core/java/android/os/Binder.java
+++ b/core/java/android/os/Binder.java
@@ -149,11 +149,6 @@ public class Binder implements IBinder {
     private static volatile boolean sStackTrackingEnabled = false;
 
     /**
-     * The extension binder object
-     */
-    private IBinder mExtension = null;
-
-    /**
      * Enable Binder IPC stack tracking. If enabled, every binder transaction will be logged to
      * {@link TransactionTracker}.
      *
@@ -1242,9 +1237,7 @@ public class Binder implements IBinder {
 
     /** @hide */
     @Override
-    public final @Nullable IBinder getExtension() {
-        return mExtension;
-    }
+    public final native @Nullable IBinder getExtension();
 
     /**
      * Set the binder extension.
@@ -1252,12 +1245,7 @@ public class Binder implements IBinder {
      *
      * @hide
      */
-    public final void setExtension(@Nullable IBinder extension) {
-        mExtension = extension;
-        setExtensionNative(extension);
-    }
-
-    private final native void setExtensionNative(@Nullable IBinder extension);
+    public final native void setExtension(@Nullable IBinder extension);
 
     /**
      * Default implementation rewinds the parcels and calls onTransact. On
diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp
index 91b25c2bda06..639f5bff7614 100644
--- a/core/jni/android_util_Binder.cpp
+++ b/core/jni/android_util_Binder.cpp
@@ -74,7 +74,6 @@ static struct bindernative_offsets_t
     jmethodID mExecTransact;
     jmethodID mGetInterfaceDescriptor;
     jmethodID mTransactionCallback;
-    jmethodID mGetExtension;
 
     // Object state.
     jfieldID mObject;
@@ -490,12 +489,8 @@ public:
             if (mVintf) {
                 ::android::internal::Stability::markVintf(b.get());
             }
-            if (mSetExtensionCalled) {
-                jobject javaIBinderObject = env->CallObjectMethod(obj, gBinderOffsets.mGetExtension);
-                sp<IBinder> extensionFromJava = ibinderForJavaObject(env, javaIBinderObject);
-                if (extensionFromJava != nullptr) {
-                    b.get()->setExtension(extensionFromJava);
-                }
+            if (mExtension != nullptr) {
+                b.get()->setExtension(mExtension);
             }
             mBinder = b;
             ALOGV("Creating JavaBinder %p (refs %p) for Object %p, weakCount=%" PRId32 "\n",
@@ -521,12 +516,21 @@ public:
         mVintf = false;
     }
 
+    sp<IBinder> getExtension() {
+        AutoMutex _l(mLock);
+        sp<JavaBBinder> b = mBinder.promote();
+        if (b != nullptr) {
+            return b.get()->getExtension();
+        }
+        return mExtension;
+    }
+
     void setExtension(const sp<IBinder>& extension) {
         AutoMutex _l(mLock);
-        mSetExtensionCalled = true;
+        mExtension = extension;
         sp<JavaBBinder> b = mBinder.promote();
         if (b != nullptr) {
-            b.get()->setExtension(extension);
+            b.get()->setExtension(mExtension);
         }
     }
 
@@ -538,7 +542,8 @@ private:
     // is too much binder state here, we can think about making JavaBBinder an
     // sp here (avoid recreating it)
     bool            mVintf = false;
-    bool            mSetExtensionCalled = false;
+
+    sp<IBinder>     mExtension;
 };
 
 // ----------------------------------------------------------------------------
@@ -1244,6 +1249,10 @@ static void android_os_Binder_blockUntilThreadAvailable(JNIEnv* env, jobject cla
     return IPCThreadState::self()->blockUntilThreadAvailable();
 }
 
+static jobject android_os_Binder_getExtension(JNIEnv* env, jobject obj) {
+    JavaBBinderHolder* jbh = (JavaBBinderHolder*) env->GetLongField(obj, gBinderOffsets.mObject);
+    return javaObjectForIBinder(env, jbh->getExtension());
+}
 
 static void android_os_Binder_setExtension(JNIEnv* env, jobject obj, jobject extensionObject) {
     JavaBBinderHolder* jbh = (JavaBBinderHolder*) env->GetLongField(obj, gBinderOffsets.mObject);
@@ -1286,7 +1295,8 @@ static const JNINativeMethod gBinderMethods[] = {
     { "getNativeBBinderHolder", "()J", (void*)android_os_Binder_getNativeBBinderHolder },
     { "getNativeFinalizer", "()J", (void*)android_os_Binder_getNativeFinalizer },
     { "blockUntilThreadAvailable", "()V", (void*)android_os_Binder_blockUntilThreadAvailable },
-    { "setExtensionNative", "(Landroid/os/IBinder;)V", (void*)android_os_Binder_setExtension },
+    { "getExtension", "()Landroid/os/IBinder;", (void*)android_os_Binder_getExtension },
+    { "setExtension", "(Landroid/os/IBinder;)V", (void*)android_os_Binder_setExtension },
 };
 // clang-format on
 
@@ -1303,8 +1313,6 @@ static int int_register_android_os_Binder(JNIEnv* env)
     gBinderOffsets.mTransactionCallback =
             GetStaticMethodIDOrDie(env, clazz, "transactionCallback", "(IIII)V");
     gBinderOffsets.mObject = GetFieldIDOrDie(env, clazz, "mObject", "J");
-    gBinderOffsets.mGetExtension = GetMethodIDOrDie(env, clazz, "getExtension",
-                                                        "()Landroid/os/IBinder;");
 
     return RegisterMethodsOrDie(
         env, kBinderPathName,