From 646cc266d6cfab12e575cf68be06374902d6740a Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 8 Sep 2023 17:17:35 +0900 Subject: binder: fix death recipient leak for apps targeting >= V Before this change, when a death recipient is set on a binder proxy via linkToDeath, a JNI global ref to the recipient object was created. That global ref is cleared only when unlinkToDeath is explicitly called or binderDied is notified. In addition, since binderDied didn't give the IBinder which has died, people has kept a strong reference to IBinder in the death recipient object. Ex: class FooHolder implements Binder.DeathRecipient { private IFoo mFoo; public FooHolder(IFoo foo) { mFoo = foo; // this!!! mFoo.linkToDeath(this, 0); } @Override public void binderDied() { // know that IFoo has died } } Unfortunately, this is prone to leak. Even if there's no reference to FooHolder in your program, it is kept in memory due to the JNI global ref as mentioned above. It means that you keep IFoo as well, and that in turn keeps the binder service in the remote side. As a result, binderDied will never be called (well, except when the server process crashes). The only way to release this object is calling unlinkToDeath explicitly when you drop references to FooHolder. However, it's error prone and keeping that practice is hard to be enforced. Recently, the need for this pattern has become weaker as we introduced binderDied(IBinder who). However, the API is quite new and its use is not mandated. There still are many cases where this pattern is used. This change is an attempt to fix the issue without having to touch the existing uses. The idea is to change the way that death recipient objects are strongly referenced - depending on whether you are targeting Android V+ or not. If targeting Android V+, the death recipient object is "weakly" referenced from JNI. Instead, it is "strongly" referenced from the BinderProxy object it is registered at. This means that if you drop a BinderProxy object, you are dropping its death recipients as well, unless you keep references to the recipients separately. For apps targeting pre-V versions, we keep the JNI strong reference. An important implication of this is that you won't get binderDied if you drop BinderProxy object before the binder actually dies. This actually is the documented behavior and has been the actual behavior "if you don't use the FooHolder pattern mentioned above". I'd argue that this CL fixes the undocumented incorrect behavior. However, we should be conservative when making any behavioral change, thus we are hiding this change behind the target SDK level. Bug: 298374304 Test: atest BinderLeakTest BinderLeakTest_legacy Change-Id: Ibb371f4de45530670d5f783f8ead8404c39381b4 --- core/java/android/os/BinderProxy.java | 28 +++- core/jni/android_util_Binder.cpp | 135 +++++++++++++----- tests/BinderLeakTest/Android.bp | 40 ++++++ tests/BinderLeakTest/AndroidManifest.xml | 17 +++ tests/BinderLeakTest/AndroidManifest_legacy.xml | 20 +++ .../aidl/com/android/test/binder/IFoo.aidl | 5 + .../aidl/com/android/test/binder/IFooProvider.aidl | 10 ++ .../java/com/android/test/binder/BinderTest.java | 153 +++++++++++++++++++++ .../java/com/android/test/binder/MyService.java | 63 +++++++++ 9 files changed, 432 insertions(+), 39 deletions(-) create mode 100644 tests/BinderLeakTest/Android.bp create mode 100644 tests/BinderLeakTest/AndroidManifest.xml create mode 100644 tests/BinderLeakTest/AndroidManifest_legacy.xml create mode 100644 tests/BinderLeakTest/aidl/com/android/test/binder/IFoo.aidl create mode 100644 tests/BinderLeakTest/aidl/com/android/test/binder/IFooProvider.aidl create mode 100644 tests/BinderLeakTest/java/com/android/test/binder/BinderTest.java create mode 100644 tests/BinderLeakTest/java/com/android/test/binder/MyService.java diff --git a/core/java/android/os/BinderProxy.java b/core/java/android/os/BinderProxy.java index 1929a4d562d4..5196b1702cc0 100644 --- a/core/java/android/os/BinderProxy.java +++ b/core/java/android/os/BinderProxy.java @@ -32,7 +32,9 @@ import java.io.FileDescriptor; import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -613,15 +615,35 @@ public final class BinderProxy implements IBinder { */ public native boolean transactNative(int code, Parcel data, Parcel reply, int flags) throws RemoteException; + + /* This list is to hold strong reference to the death recipients that are waiting for the death + * of binder that this proxy references. Previously, the death recipients were strongy + * referenced from JNI, but that can cause memory leak (b/298374304) when the application has a + * strong reference from the death recipient to the proxy object. The JNI reference is now weak. + * And this strong reference is to keep death recipients at least until the proxy is GC'ed. */ + private List mDeathRecipients = Collections.synchronizedList(new ArrayList<>()); + /** * See {@link IBinder#linkToDeath(DeathRecipient, int)} */ - public native void linkToDeath(DeathRecipient recipient, int flags) - throws RemoteException; + public void linkToDeath(DeathRecipient recipient, int flags) + throws RemoteException { + linkToDeathNative(recipient, flags); + mDeathRecipients.add(recipient); + } + /** * See {@link IBinder#unlinkToDeath} */ - public native boolean unlinkToDeath(DeathRecipient recipient, int flags); + public boolean unlinkToDeath(DeathRecipient recipient, int flags) { + mDeathRecipients.remove(recipient); + return unlinkToDeathNative(recipient, flags); + } + + private native void linkToDeathNative(DeathRecipient recipient, int flags) + throws RemoteException; + + private native boolean unlinkToDeathNative(DeathRecipient recipient, int flags); /** * Perform a dump on the remote object diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp index 6ed0a8a047f5..f4bc2675e30d 100644 --- a/core/jni/android_util_Binder.cpp +++ b/core/jni/android_util_Binder.cpp @@ -17,19 +17,8 @@ #define LOG_TAG "JavaBinder" //#define LOG_NDEBUG 0 -#include "android_os_Parcel.h" #include "android_util_Binder.h" -#include -#include -#include -#include -#include -#include -#include -#include -#include - #include #include #include @@ -40,7 +29,16 @@ #include #include #include +#include +#include #include +#include +#include +#include +#include +#include +#include +#include #include #include #include @@ -48,10 +46,11 @@ #include #include -#include -#include -#include +#include +#include +#include +#include "android_os_Parcel.h" #include "core_jni_helpers.h" //#undef ALOGV @@ -557,14 +556,43 @@ public: }; // ---------------------------------------------------------------------------- +#if __BIONIC__ +#include +static bool target_sdk_is_at_least_vic() { + return android_get_application_target_sdk_version() >= __ANDROID_API_V__; +} +#else +static constexpr bool target_sdk_is_at_least_vic() { + // If not built for Android (i.e. glibc host), follow the latest behavior as there's no compat + // requirement there. + return true; +} +#endif // __BIONIC__ class JavaDeathRecipient : public IBinder::DeathRecipient { public: JavaDeathRecipient(JNIEnv* env, jobject object, const sp& list) - : mVM(jnienv_to_javavm(env)), mObject(env->NewGlobalRef(object)), - mObjectWeak(NULL), mList(list) - { + : mVM(jnienv_to_javavm(env)), mObject(NULL), mObjectWeak(NULL), mList(list) { + // b/298374304: For apps targeting Android V or beyond, we no longer hold the global JNI ref + // to the death recipient objects. This is to prevent the memory leak which can happen when + // the death recipient object internally has a strong reference to the proxy object. Under + // the old behavior, you were unable to kill the binder service by dropping all references + // to the proxy object - because it is still strong referenced from JNI (here). The only way + // to cut the strong reference was to call unlinkDeath(), but it was easy to forget. + // + // Now, the strong reference to the death recipient is held in the Java-side proxy object. + // See BinderProxy.mDeathRecipients. From JNI, only the weak reference is kept. An + // implication of this is that you may not receive binderDied() if you drop all references + // to the proxy object before the service dies. This should be okay for most cases because + // you normally are not interested in the death of a binder service which you don't have any + // reference to. If however you want to get binderDied() regardless of the proxy object's + // lifecycle, keep a strong reference to the death recipient object by yourself. + if (target_sdk_is_at_least_vic()) { + mObjectWeak = env->NewWeakGlobalRef(object); + } else { + mObject = env->NewGlobalRef(object); + } // These objects manage their own lifetimes so are responsible for final bookkeeping. // The list holds a strong reference to this object. LOGDEATH("Adding JDR %p to DRL %p", this, list.get()); @@ -577,26 +605,49 @@ public: void binderDied(const wp& who) { LOGDEATH("Receiving binderDied() on JavaDeathRecipient %p\n", this); - if (mObject != NULL) { - JNIEnv* env = javavm_to_jnienv(mVM); - ScopedLocalRef jBinderProxy(env, javaObjectForIBinder(env, who.promote())); - env->CallStaticVoidMethod(gBinderProxyOffsets.mClass, - gBinderProxyOffsets.mSendDeathNotice, mObject, - jBinderProxy.get()); - if (env->ExceptionCheck()) { - jthrowable excep = env->ExceptionOccurred(); - binder_report_exception(env, excep, - "*** Uncaught exception returned from death notification!"); - } + if (mObject == NULL && mObjectWeak == NULL) { + return; + } + JNIEnv* env = javavm_to_jnienv(mVM); + ScopedLocalRef jBinderProxy(env, javaObjectForIBinder(env, who.promote())); + + // Hold a local reference to the recipient. This may fail if the recipient is weakly + // referenced, in which case we can't deliver the death notice. + ScopedLocalRef jRecipient(env, + env->NewLocalRef(mObject != NULL ? mObject + : mObjectWeak)); + if (jRecipient.get() == NULL) { + ALOGW("Binder died, but death recipient is already garbage collected. If your target " + "sdk level is at or above 35, this can happen when you dropped all references to " + "the binder service before it died. If you want to get a death notice for a " + "binder service which you have no reference to, keep a strong reference to the " + "death recipient by yourself."); + return; + } - // Serialize with our containing DeathRecipientList so that we can't - // delete the global ref on mObject while the list is being iterated. + if (mFired) { + ALOGW("Received multiple death notices for the same binder object. Binder driver bug?"); + return; + } + mFired = true; + + env->CallStaticVoidMethod(gBinderProxyOffsets.mClass, gBinderProxyOffsets.mSendDeathNotice, + jRecipient.get(), jBinderProxy.get()); + if (env->ExceptionCheck()) { + jthrowable excep = env->ExceptionOccurred(); + binder_report_exception(env, excep, + "*** Uncaught exception returned from death notification!"); + } + + // Demote from strong ref (if exists) to weak after binderDied() has been delivered, to + // allow the DeathRecipient and BinderProxy to be GC'd if no longer needed. Do this in sync + // with our containing DeathRecipientList so that we can't delete the global ref on mObject + // while the list is being iterated. + if (mObject != NULL) { sp list = mList.promote(); if (list != NULL) { AutoMutex _l(list->lock()); - // Demote from strong ref to weak after binderDied() has been delivered, - // to allow the DeathRecipient and BinderProxy to be GC'd if no longer needed. mObjectWeak = env->NewWeakGlobalRef(mObject); env->DeleteGlobalRef(mObject); mObject = NULL; @@ -663,9 +714,19 @@ protected: private: JavaVM* const mVM; - jobject mObject; // Initial strong ref to Java-side DeathRecipient. Cleared on binderDied(). - jweak mObjectWeak; // Weak ref to the same Java-side DeathRecipient after binderDied(). + + // If target sdk version < 35, the Java-side DeathRecipient is strongly referenced from mObject + // upon linkToDeath() and then after binderDied() is called, the strong reference is demoted to + // a weak reference (mObjectWeak). + // If target sdk version >= 35, the strong reference is never made here (i.e. mObject == NULL + // always). Instead, the strong reference to the Java-side DeathRecipient is made in + // BinderProxy.mDeathRecipients. In the native world, only the weak reference is kept. + jobject mObject; + jweak mObjectWeak; wp mList; + + // Whether binderDied was called or not. + bool mFired = false; }; // ---------------------------------------------------------------------------- @@ -1452,17 +1513,19 @@ static jobject android_os_BinderProxy_getExtension(JNIEnv* env, jobject obj) { // ---------------------------------------------------------------------------- +// clang-format off static const JNINativeMethod gBinderProxyMethods[] = { /* name, signature, funcPtr */ {"pingBinder", "()Z", (void*)android_os_BinderProxy_pingBinder}, {"isBinderAlive", "()Z", (void*)android_os_BinderProxy_isBinderAlive}, {"getInterfaceDescriptor", "()Ljava/lang/String;", (void*)android_os_BinderProxy_getInterfaceDescriptor}, {"transactNative", "(ILandroid/os/Parcel;Landroid/os/Parcel;I)Z", (void*)android_os_BinderProxy_transact}, - {"linkToDeath", "(Landroid/os/IBinder$DeathRecipient;I)V", (void*)android_os_BinderProxy_linkToDeath}, - {"unlinkToDeath", "(Landroid/os/IBinder$DeathRecipient;I)Z", (void*)android_os_BinderProxy_unlinkToDeath}, + {"linkToDeathNative", "(Landroid/os/IBinder$DeathRecipient;I)V", (void*)android_os_BinderProxy_linkToDeath}, + {"unlinkToDeathNative", "(Landroid/os/IBinder$DeathRecipient;I)Z", (void*)android_os_BinderProxy_unlinkToDeath}, {"getNativeFinalizer", "()J", (void*)android_os_BinderProxy_getNativeFinalizer}, {"getExtension", "()Landroid/os/IBinder;", (void*)android_os_BinderProxy_getExtension}, }; +// clang-format on const char* const kBinderProxyPathName = "android/os/BinderProxy"; diff --git a/tests/BinderLeakTest/Android.bp b/tests/BinderLeakTest/Android.bp new file mode 100644 index 000000000000..78b0ede76d4e --- /dev/null +++ b/tests/BinderLeakTest/Android.bp @@ -0,0 +1,40 @@ +package { + // See: http://go/android-license-faq + // A large-scale-change added 'default_applicable_licenses' to import + // all of the 'license_kinds' from "frameworks_base_license" + // to get the below license kinds: + // SPDX-license-identifier-Apache-2.0 + default_applicable_licenses: ["frameworks_base_license"], +} + +filegroup { + name: "binder_leak_test_aidl", + srcs: ["**/*.aidl"], + path: "aidl", +} + +java_defaults { + name: "BinderTest.defaults", + srcs: [ + "**/*.java", + ":binder_leak_test_aidl", + ], + static_libs: [ + "androidx.test.ext.junit", + "androidx.test.rules", + "androidx.test.runner", + ], +} + +// Built with target_sdk_version: current +android_test { + name: "BinderLeakTest", + defaults: ["BinderTest.defaults"], +} + +// Built with target_sdk_version: 33 +android_test { + name: "BinderLeakTest_legacy", + defaults: ["BinderTest.defaults"], + manifest: "AndroidManifest_legacy.xml", +} diff --git a/tests/BinderLeakTest/AndroidManifest.xml b/tests/BinderLeakTest/AndroidManifest.xml new file mode 100644 index 000000000000..756def7ac29d --- /dev/null +++ b/tests/BinderLeakTest/AndroidManifest.xml @@ -0,0 +1,17 @@ + + + + + + + + + + diff --git a/tests/BinderLeakTest/AndroidManifest_legacy.xml b/tests/BinderLeakTest/AndroidManifest_legacy.xml new file mode 100644 index 000000000000..03d1dfd1fd83 --- /dev/null +++ b/tests/BinderLeakTest/AndroidManifest_legacy.xml @@ -0,0 +1,20 @@ + + + + + + + + + + + diff --git a/tests/BinderLeakTest/aidl/com/android/test/binder/IFoo.aidl b/tests/BinderLeakTest/aidl/com/android/test/binder/IFoo.aidl new file mode 100644 index 000000000000..a721959d19b4 --- /dev/null +++ b/tests/BinderLeakTest/aidl/com/android/test/binder/IFoo.aidl @@ -0,0 +1,5 @@ +package com.android.test.binder; + +interface IFoo { + +} diff --git a/tests/BinderLeakTest/aidl/com/android/test/binder/IFooProvider.aidl b/tests/BinderLeakTest/aidl/com/android/test/binder/IFooProvider.aidl new file mode 100644 index 000000000000..b487f51f812c --- /dev/null +++ b/tests/BinderLeakTest/aidl/com/android/test/binder/IFooProvider.aidl @@ -0,0 +1,10 @@ +package com.android.test.binder; +import com.android.test.binder.IFoo; + +interface IFooProvider { + IFoo createFoo(); + + boolean isFooGarbageCollected(); + + oneway void killProcess(); +} diff --git a/tests/BinderLeakTest/java/com/android/test/binder/BinderTest.java b/tests/BinderLeakTest/java/com/android/test/binder/BinderTest.java new file mode 100644 index 000000000000..f07317f7d5f3 --- /dev/null +++ b/tests/BinderLeakTest/java/com/android/test/binder/BinderTest.java @@ -0,0 +1,153 @@ +/* + * 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 com.android.test.binder; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +import android.content.Intent; +import android.os.Build; +import android.os.IBinder; +import android.os.RemoteException; + +import androidx.test.core.app.ApplicationProvider; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.rule.ServiceTestRule; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.lang.ref.PhantomReference; +import java.lang.ref.ReferenceQueue; +import java.util.concurrent.TimeoutException; + +@RunWith(AndroidJUnit4.class) +public class BinderTest { + @Rule + public final ServiceTestRule serviceRule = new ServiceTestRule(); + + @Test + public void testDeathRecipientLeaksOrNot() + throws RemoteException, TimeoutException, InterruptedException { + Intent intent = new Intent(ApplicationProvider.getApplicationContext(), MyService.class); + IFooProvider provider = IFooProvider.Stub.asInterface(serviceRule.bindService(intent)); + FooHolder holder = new FooHolder(provider.createFoo()); + + // ref will get enqueued right after holder is finalized for gc. + ReferenceQueue refQueue = new ReferenceQueue<>(); + PhantomReference ref = new PhantomReference<>(holder, refQueue); + + DeathRecorder deathRecorder = new DeathRecorder(); + holder.registerDeathRecorder(deathRecorder); + + if (getSdkVersion() >= Build.VERSION_CODES.VANILLA_ICE_CREAM) { + ///////////////////////////////////////////// + // New behavior + // + // Reference chain at this moment: + // holder --(java strong ref)--> FooHolder + // FooHolder.mProxy --(java strong ref)--> IFoo.Proxy + // IFoo.Proxy.mRemote --(java strong ref)--> BinderProxy + // BinderProxy --(binder ref)--> Foo.Stub + // In other words, the variable "holder" is the root of the reference chain. + + // By setting the variable to null, we make FooHolder, IFoo.Proxy, BinderProxy, and even + // Foo.Stub unreachable. + holder = null; + + // Ensure that the objects are garbage collected + forceGc(); + assertEquals(ref, refQueue.poll()); + assertTrue(provider.isFooGarbageCollected()); + + // The binder has died, but we don't get notified since the death recipient is GC'ed. + provider.killProcess(); + Thread.sleep(1000); // give some time for the service process to die and reaped + assertFalse(deathRecorder.deathRecorded); + } else { + ///////////////////////////////////////////// + // Legacy behavior + // + // Reference chain at this moment: + // JavaDeathRecipient --(JNI strong ref)--> FooHolder + // holder --(java strong ref)--> FooHolder + // FooHolder.mProxy --(java strong ref)--> IFoo.Proxy + // IFoo.Proxy.mRemote --(java strong ref)--> BinderProxy + // BinderProxy --(binder ref)--> Foo.Stub + // So, BOTH JavaDeathRecipient and holder are roots of the reference chain. + + // Even if we set holder to null, it doesn't make other objects unreachable; they are + // still reachable via the JNI strong ref. + holder = null; + + // Check that objects are not garbage collected + forceGc(); + assertNotEquals(ref, refQueue.poll()); + assertFalse(provider.isFooGarbageCollected()); + + // The legacy behavior is getting notified even when there's no reference + provider.killProcess(); + Thread.sleep(1000); // give some time for the service process to die and reaped + assertTrue(deathRecorder.deathRecorded); + } + } + + static class FooHolder implements IBinder.DeathRecipient { + private IFoo mProxy; + private DeathRecorder mDeathRecorder; + + FooHolder(IFoo proxy) throws RemoteException { + proxy.asBinder().linkToDeath(this, 0); + + // A strong reference from DeathRecipient(this) to the binder proxy is created here + mProxy = proxy; + } + + public void registerDeathRecorder(DeathRecorder dr) { + mDeathRecorder = dr; + } + + @Override + public void binderDied() { + if (mDeathRecorder != null) { + mDeathRecorder.deathRecorded = true; + } + } + } + + static class DeathRecorder { + public boolean deathRecorded = false; + } + + // Try calling System.gc() until an orphaned object is confirmed to be finalized + private static void forceGc() { + Object obj = new Object(); + ReferenceQueue refQueue = new ReferenceQueue<>(); + PhantomReference ref = new PhantomReference<>(obj, refQueue); + obj = null; // make it an orphan + while (refQueue.poll() != ref) { + System.gc(); + } + } + + private static int getSdkVersion() { + return ApplicationProvider.getApplicationContext().getApplicationInfo().targetSdkVersion; + } +} diff --git a/tests/BinderLeakTest/java/com/android/test/binder/MyService.java b/tests/BinderLeakTest/java/com/android/test/binder/MyService.java new file mode 100644 index 000000000000..c701253446f4 --- /dev/null +++ b/tests/BinderLeakTest/java/com/android/test/binder/MyService.java @@ -0,0 +1,63 @@ +/* + * 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 com.android.test.binder; + +import android.app.Service; +import android.content.Intent; +import android.os.IBinder; +import android.os.RemoteException; + +import java.lang.ref.PhantomReference; +import java.lang.ref.ReferenceQueue; + +public class MyService extends Service { + @Override + public IBinder onBind(Intent intent) { + return new IFooProvider.Stub() { + ReferenceQueue mRefQueue = new ReferenceQueue<>(); + PhantomReference mRef; + + @Override + public IFoo createFoo() throws RemoteException { + IFoo binder = new IFoo.Stub() {}; + mRef = new PhantomReference<>(binder, mRefQueue); + return binder; + } + + @Override + public boolean isFooGarbageCollected() throws RemoteException { + forceGc(); + return mRefQueue.poll() == mRef; + } + + @Override + public void killProcess() throws RemoteException { + android.os.Process.killProcess(android.os.Process.myPid()); + } + }; + } + + private static void forceGc() { + Object obj = new Object(); + ReferenceQueue refQueue = new ReferenceQueue<>(); + PhantomReference ref = new PhantomReference<>(obj, refQueue); + obj = null; // make it an orphan + while (refQueue.poll() != ref) { + System.gc(); + } + } +} -- cgit v1.2.3-59-g8ed1b From c4739e7ff8a2049dd5d783cca40e076d7edb6c92 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Mon, 25 Sep 2023 23:15:56 +0900 Subject: Guard the death recipient behavior behind a build flag The death recipient behavior introduced with Ibb371f4de45530670d5f783f8ead8404c39381b4 is guarded with a build flag RELEASE_BINDER_DEATH_RECIPIENT_WEAK_FROM_JNI. Bug: 298374304 Test: build Change-Id: Ie604ee723385676cf3c83f0d9b2a46ceb322903a --- core/jni/Android.bp | 20 +++++++++++++++++++- core/jni/android_util_Binder.cpp | 7 ++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/core/jni/Android.bp b/core/jni/Android.bp index 73c93ac9d06b..81c32550cf14 100644 --- a/core/jni/Android.bp +++ b/core/jni/Android.bp @@ -15,7 +15,19 @@ license { ], } -cc_library_shared { +soong_config_module_type { + name: "cc_library_shared_for_libandroid_runtime", + module_type: "cc_library_shared", + config_namespace: "ANDROID", + bool_variables: [ + "release_binder_death_recipient_weak_from_jni", + ], + properties: [ + "cflags", + ], +} + +cc_library_shared_for_libandroid_runtime { name: "libandroid_runtime", host_supported: true, cflags: [ @@ -46,6 +58,12 @@ cc_library_shared { }, }, + soong_config_variables: { + release_binder_death_recipient_weak_from_jni: { + cflags: ["-DBINDER_DEATH_RECIPIENT_WEAK_FROM_JNI"], + }, + }, + cpp_std: "gnu++20", srcs: [ diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp index f4bc2675e30d..55382cc1d380 100644 --- a/core/jni/android_util_Binder.cpp +++ b/core/jni/android_util_Binder.cpp @@ -556,6 +556,7 @@ public: }; // ---------------------------------------------------------------------------- +#ifdef BINDER_DEATH_RECIPIENT_WEAK_FROM_JNI #if __BIONIC__ #include static bool target_sdk_is_at_least_vic() { @@ -568,6 +569,7 @@ static constexpr bool target_sdk_is_at_least_vic() { return true; } #endif // __BIONIC__ +#endif // BINDER_DEATH_RECIPIENT_WEAK_FROM_JNI class JavaDeathRecipient : public IBinder::DeathRecipient { @@ -588,9 +590,12 @@ public: // you normally are not interested in the death of a binder service which you don't have any // reference to. If however you want to get binderDied() regardless of the proxy object's // lifecycle, keep a strong reference to the death recipient object by yourself. +#ifdef BINDER_DEATH_RECIPIENT_WEAK_FROM_JNI if (target_sdk_is_at_least_vic()) { mObjectWeak = env->NewWeakGlobalRef(object); - } else { + } else +#endif + { mObject = env->NewGlobalRef(object); } // These objects manage their own lifetimes so are responsible for final bookkeeping. -- cgit v1.2.3-59-g8ed1b