Merge changes from topic "death_recipient_flag" into main am: e916262ee7 am: bbe1f9da3d (81d562a1) · Commits · e / os / android_frameworks_base

core/java/android/os/BinderProxy.java

+25 −3

Original line number	Original line	Diff line number	Diff line
	@@ -32,7 +32,9 @@ import java.io.FileDescriptor;
	import java.lang.ref.WeakReference;		import java.lang.ref.WeakReference;
	import java.util.ArrayList;		import java.util.ArrayList;
	import java.util.Arrays;		import java.util.Arrays;
			import java.util.Collections;
	import java.util.HashMap;		import java.util.HashMap;
			import java.util.List;
	import java.util.Map;		import java.util.Map;
	import java.util.concurrent.ExecutorService;		import java.util.concurrent.ExecutorService;
	import java.util.concurrent.Executors;		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,		public native boolean transactNative(int code, Parcel data, Parcel reply,
	int flags) throws RemoteException;		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<DeathRecipient> mDeathRecipients = Collections.synchronizedList(new ArrayList<>());

	/**		/**
	* See {@link IBinder#linkToDeath(DeathRecipient, int)}		* See {@link IBinder#linkToDeath(DeathRecipient, int)}
	*/		*/
	public native void linkToDeath(DeathRecipient recipient, int flags)		public void linkToDeath(DeathRecipient recipient, int flags)
	throws RemoteException;		throws RemoteException {
			linkToDeathNative(recipient, flags);
			mDeathRecipients.add(recipient);
			}

	/**		/**
	* See {@link IBinder#unlinkToDeath}		* 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		* Perform a dump on the remote object

core/jni/Android.bp

+19 −1

Original line number	Original line	Diff line number	Diff line
	@@ -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",		name: "libandroid_runtime",
	host_supported: true,		host_supported: true,
	cflags: [		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",		cpp_std: "gnu++20",

	srcs: [		srcs: [

core/jni/android_util_Binder.cpp

+104 −36

Original line number	Original line	Diff line number	Diff line
	@@ -17,19 +17,8 @@
	#define LOG_TAG "JavaBinder"		#define LOG_TAG "JavaBinder"
	//#define LOG_NDEBUG 0		//#define LOG_NDEBUG 0

	#include "android_os_Parcel.h"
	#include "android_util_Binder.h"		#include "android_util_Binder.h"

	#include <atomic>
	#include <fcntl.h>
	#include <inttypes.h>
	#include <mutex>
	#include <stdio.h>
	#include <string>
	#include <sys/stat.h>
	#include <sys/types.h>
	#include <unistd.h>

	#include <android-base/stringprintf.h>		#include <android-base/stringprintf.h>
	#include <binder/BpBinder.h>		#include <binder/BpBinder.h>
	#include <binder/IInterface.h>		#include <binder/IInterface.h>
	@@ -40,7 +29,16 @@
	#include <binder/Stability.h>		#include <binder/Stability.h>
	#include <binderthreadstate/CallerUtils.h>		#include <binderthreadstate/CallerUtils.h>
	#include <cutils/atomic.h>		#include <cutils/atomic.h>
			#include <fcntl.h>
			#include <inttypes.h>
	#include <log/log.h>		#include <log/log.h>
			#include <nativehelper/JNIHelp.h>
			#include <nativehelper/ScopedLocalRef.h>
			#include <nativehelper/ScopedUtfChars.h>
			#include <stdio.h>
			#include <sys/stat.h>
			#include <sys/types.h>
			#include <unistd.h>
	#include <utils/KeyedVector.h>		#include <utils/KeyedVector.h>
	#include <utils/List.h>		#include <utils/List.h>
	#include <utils/Log.h>		#include <utils/Log.h>
	@@ -48,10 +46,11 @@
	#include <utils/SystemClock.h>		#include <utils/SystemClock.h>
	#include <utils/threads.h>		#include <utils/threads.h>

	#include <nativehelper/JNIHelp.h>		#include <atomic>
	#include <nativehelper/ScopedLocalRef.h>		#include <mutex>
	#include <nativehelper/ScopedUtfChars.h>		#include <string>

			#include "android_os_Parcel.h"
	#include "core_jni_helpers.h"		#include "core_jni_helpers.h"

	//#undef ALOGV		//#undef ALOGV
	@@ -557,14 +556,48 @@ public:
	};		};

	// ----------------------------------------------------------------------------		// ----------------------------------------------------------------------------
			#ifdef BINDER_DEATH_RECIPIENT_WEAK_FROM_JNI
			#if __BIONIC__
			#include <android/api-level.h>
			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__
			#endif // BINDER_DEATH_RECIPIENT_WEAK_FROM_JNI

	class JavaDeathRecipient : public IBinder::DeathRecipient		class JavaDeathRecipient : public IBinder::DeathRecipient
	{		{
	public:		public:
	JavaDeathRecipient(JNIEnv* env, jobject object, const sp<DeathRecipientList>& list)		JavaDeathRecipient(JNIEnv* env, jobject object, const sp<DeathRecipientList>& list)
	: mVM(jnienv_to_javavm(env)), mObject(env->NewGlobalRef(object)),		: mVM(jnienv_to_javavm(env)), mObject(NULL), mObjectWeak(NULL), mList(list) {
	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.
			#ifdef BINDER_DEATH_RECIPIENT_WEAK_FROM_JNI
			if (target_sdk_is_at_least_vic()) {
			mObjectWeak = env->NewWeakGlobalRef(object);
			} else
			#endif
	{		{
			mObject = env->NewGlobalRef(object);
			}
	// These objects manage their own lifetimes so are responsible for final bookkeeping.		// These objects manage their own lifetimes so are responsible for final bookkeeping.
	// The list holds a strong reference to this object.		// The list holds a strong reference to this object.
	LOGDEATH("Adding JDR %p to DRL %p", this, list.get());		LOGDEATH("Adding JDR %p to DRL %p", this, list.get());
	@@ -577,26 +610,49 @@ public:
	void binderDied(const wp<IBinder>& who)		void binderDied(const wp<IBinder>& who)
	{		{
	LOGDEATH("Receiving binderDied() on JavaDeathRecipient %p\n", this);		LOGDEATH("Receiving binderDied() on JavaDeathRecipient %p\n", this);
	if (mObject != NULL) {		if (mObject == NULL && mObjectWeak == NULL) {
			return;
			}
	JNIEnv* env = javavm_to_jnienv(mVM);		JNIEnv* env = javavm_to_jnienv(mVM);
	ScopedLocalRef<jobject> jBinderProxy(env, javaObjectForIBinder(env, who.promote()));		ScopedLocalRef<jobject> jBinderProxy(env, javaObjectForIBinder(env, who.promote()));
	env->CallStaticVoidMethod(gBinderProxyOffsets.mClass,
	gBinderProxyOffsets.mSendDeathNotice, mObject,		// Hold a local reference to the recipient. This may fail if the recipient is weakly
	jBinderProxy.get());		// referenced, in which case we can't deliver the death notice.
			ScopedLocalRef<jobject> 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;
			}

			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()) {		if (env->ExceptionCheck()) {
	jthrowable excep = env->ExceptionOccurred();		jthrowable excep = env->ExceptionOccurred();
	binder_report_exception(env, excep,		binder_report_exception(env, excep,
	"*** Uncaught exception returned from death notification!");		"*** Uncaught exception returned from death notification!");
	}		}

	// Serialize with our containing DeathRecipientList so that we can't		// Demote from strong ref (if exists) to weak after binderDied() has been delivered, to
	// delete the global ref on mObject while the list is being iterated.		// 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<DeathRecipientList> list = mList.promote();		sp<DeathRecipientList> list = mList.promote();
	if (list != NULL) {		if (list != NULL) {
	AutoMutex _l(list->lock());		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);		mObjectWeak = env->NewWeakGlobalRef(mObject);
	env->DeleteGlobalRef(mObject);		env->DeleteGlobalRef(mObject);
	mObject = NULL;		mObject = NULL;
	@@ -663,9 +719,19 @@ protected:

	private:		private:
	JavaVM* const mVM;		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<DeathRecipientList> mList;		wp<DeathRecipientList> mList;

			// Whether binderDied was called or not.
			bool mFired = false;
	};		};

	// ----------------------------------------------------------------------------		// ----------------------------------------------------------------------------
	@@ -1452,17 +1518,19 @@ static jobject android_os_BinderProxy_getExtension(JNIEnv* env, jobject obj) {

	// ----------------------------------------------------------------------------		// ----------------------------------------------------------------------------

			// clang-format off
	static const JNINativeMethod gBinderProxyMethods[] = {		static const JNINativeMethod gBinderProxyMethods[] = {
	/* name, signature, funcPtr */		/* name, signature, funcPtr */
	{"pingBinder", "()Z", (void*)android_os_BinderProxy_pingBinder},		{"pingBinder", "()Z", (void*)android_os_BinderProxy_pingBinder},
	{"isBinderAlive", "()Z", (void*)android_os_BinderProxy_isBinderAlive},		{"isBinderAlive", "()Z", (void*)android_os_BinderProxy_isBinderAlive},
	{"getInterfaceDescriptor", "()Ljava/lang/String;", (void*)android_os_BinderProxy_getInterfaceDescriptor},		{"getInterfaceDescriptor", "()Ljava/lang/String;", (void*)android_os_BinderProxy_getInterfaceDescriptor},
	{"transactNative", "(ILandroid/os/Parcel;Landroid/os/Parcel;I)Z", (void*)android_os_BinderProxy_transact},		{"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},		{"linkToDeathNative", "(Landroid/os/IBinder$DeathRecipient;I)V", (void*)android_os_BinderProxy_linkToDeath},
	{"unlinkToDeath", "(Landroid/os/IBinder$DeathRecipient;I)Z", (void*)android_os_BinderProxy_unlinkToDeath},		{"unlinkToDeathNative", "(Landroid/os/IBinder$DeathRecipient;I)Z", (void*)android_os_BinderProxy_unlinkToDeath},
	{"getNativeFinalizer", "()J", (void*)android_os_BinderProxy_getNativeFinalizer},		{"getNativeFinalizer", "()J", (void*)android_os_BinderProxy_getNativeFinalizer},
	{"getExtension", "()Landroid/os/IBinder;", (void*)android_os_BinderProxy_getExtension},		{"getExtension", "()Landroid/os/IBinder;", (void*)android_os_BinderProxy_getExtension},
	};		};
			// clang-format on

	const char* const kBinderProxyPathName = "android/os/BinderProxy";		const char* const kBinderProxyPathName = "android/os/BinderProxy";

tests/BinderLeakTest/Android.bp

0 → 100644

+40 −0

Original line number	Original line	Diff line number	Diff line
			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",
			}

tests/BinderLeakTest/AndroidManifest.xml

0 → 100644

+17 −0

Original line number	Original line	Diff line number	Diff line
			<?xml version="1.0" encoding="utf-8"?>
			<manifest xmlns:android="http://schemas.android.com/apk/res/android"
			package="com.android.test.binder">
			<application>
			<service
			android:name=".MyService"
			android:enabled="true"
			android:exported="true"
			android:process=":service">
			</service>
			</application>

			<instrumentation android:name="androidx.test.runner.AndroidJUnitRunner"
			android:targetPackage="com.android.test.binder"
			android:label="Binder leak test">
			</instrumentation>
			</manifest>