Merge "Binder linkage no longer depends on JNI objrefs as persistent tokens"
diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp
index 7a53874..bd5305d 100644
--- a/core/jni/android_util_Binder.cpp
+++ b/core/jni/android_util_Binder.cpp
@@ -31,6 +31,8 @@
#include <binder/IPCThreadState.h>
#include <utils/Log.h>
#include <utils/SystemClock.h>
+#include <utils/List.h>
+#include <utils/KeyedVector.h>
#include <cutils/logger.h>
#include <binder/Parcel.h>
#include <binder/ProcessState.h>
@@ -322,25 +324,15 @@
class JavaBBinderHolder : public RefBase
{
public:
- JavaBBinderHolder(JNIEnv* env, jobject object)
- : mObject(object)
- {
- LOGV("Creating JavaBBinderHolder for Object %p\n", object);
- }
- ~JavaBBinderHolder()
- {
- LOGV("Destroying JavaBBinderHolder for Object %p\n", mObject);
- }
-
- sp<JavaBBinder> get(JNIEnv* env)
+ sp<JavaBBinder> get(JNIEnv* env, jobject obj)
{
AutoMutex _l(mLock);
sp<JavaBBinder> b = mBinder.promote();
if (b == NULL) {
- b = new JavaBBinder(env, mObject);
+ b = new JavaBBinder(env, obj);
mBinder = b;
LOGV("Creating JavaBinder %p (refs %p) for Object %p, weakCount=%d\n",
- b.get(), b->getWeakRefs(), mObject, b->getWeakRefs()->getWeakCount());
+ b.get(), b->getWeakRefs(), obj, b->getWeakRefs()->getWeakCount());
}
return b;
@@ -354,20 +346,41 @@
private:
Mutex mLock;
- jobject mObject;
wp<JavaBBinder> mBinder;
};
// ----------------------------------------------------------------------------
+// Per-IBinder death recipient bookkeeping. This is how we reconcile local jobject
+// death recipient references passed in through JNI with the permanent corresponding
+// JavaDeathRecipient objects.
+
+class JavaDeathRecipient;
+
+class DeathRecipientList : public RefBase {
+ List< sp<JavaDeathRecipient> > mList;
+ Mutex mLock;
+
+public:
+ ~DeathRecipientList();
+
+ void add(const sp<JavaDeathRecipient>& recipient);
+ void remove(const sp<JavaDeathRecipient>& recipient);
+ sp<JavaDeathRecipient> find(jobject recipient);
+};
+
+// ----------------------------------------------------------------------------
+
class JavaDeathRecipient : public IBinder::DeathRecipient
{
public:
- JavaDeathRecipient(JNIEnv* env, jobject object)
- : mVM(jnienv_to_javavm(env)), mObject(env->NewGlobalRef(object)),
- mHoldsRef(true)
+ JavaDeathRecipient(JNIEnv* env, jobject object, sp<DeathRecipientList>& list)
+ : mVM(jnienv_to_javavm(env)), mObject(env->NewGlobalRef(object)), mList(list)
{
- incStrong(this);
+ // These objects manage their own lifetimes so are responsible for final bookkeeping.
+ // The list holds a strong reference to this object.
+ mList->add(this);
+
android_atomic_inc(&gNumDeathRefs);
incRefsCreated(env);
}
@@ -391,16 +404,12 @@
void clearReference()
{
- bool release = false;
- mLock.lock();
- if (mHoldsRef) {
- mHoldsRef = false;
- release = true;
- }
- mLock.unlock();
- if (release) {
- decStrong(this);
- }
+ mList->remove(this);
+ }
+
+ bool matches(jobject obj) {
+ JNIEnv* env = javavm_to_jnienv(mVM);
+ return env->IsSameObject(obj, mObject);
}
protected:
@@ -415,12 +424,57 @@
private:
JavaVM* const mVM;
jobject const mObject;
- Mutex mLock;
- bool mHoldsRef;
+ sp<DeathRecipientList> mList;
};
// ----------------------------------------------------------------------------
+DeathRecipientList::~DeathRecipientList() {
+ AutoMutex _l(mLock);
+
+ // Should never happen -- the JavaDeathRecipient objects that have added themselves
+ // to the list are holding references on the list object. Only when they are torn
+ // down can the list header be destroyed.
+ if (mList.size() > 0) {
+ LOGE("Retiring binder %p with extant death recipients\n", this);
+ }
+}
+
+void DeathRecipientList::add(const sp<JavaDeathRecipient>& recipient) {
+ AutoMutex _l(mLock);
+
+ mList.push_back(recipient);
+}
+
+void DeathRecipientList::remove(const sp<JavaDeathRecipient>& recipient) {
+ AutoMutex _l(mLock);
+
+ List< sp<JavaDeathRecipient> >::iterator iter;
+ for (iter = mList.begin(); iter != mList.end(); iter++) {
+ if (*iter == recipient) {
+ mList.erase(iter);
+ return;
+ }
+ }
+}
+
+sp<JavaDeathRecipient> DeathRecipientList::find(jobject recipient) {
+ AutoMutex _l(mLock);
+
+ List< sp<JavaDeathRecipient> >::iterator iter;
+ for (iter = mList.begin(); iter != mList.end(); iter++) {
+ if ((*iter)->matches(recipient)) {
+ return *iter;
+ }
+ }
+ return NULL;
+}
+
+static KeyedVector<IBinder*, sp<DeathRecipientList> > gDeathRecipientsByIBinder;
+static Mutex gDeathRecipientMapLock;
+
+// ----------------------------------------------------------------------------
+
namespace android {
static void proxy_cleanup(const void* id, void* obj, void* cleanupCookie)
@@ -490,7 +544,7 @@
if (env->IsInstanceOf(obj, gBinderOffsets.mClass)) {
JavaBBinderHolder* jbh = (JavaBBinderHolder*)
env->GetIntField(obj, gBinderOffsets.mObject);
- return jbh != NULL ? jbh->get(env) : NULL;
+ return jbh != NULL ? jbh->get(env, obj) : NULL;
}
if (env->IsInstanceOf(obj, gBinderProxyOffsets.mClass)) {
@@ -621,26 +675,26 @@
IPCThreadState::self()->flushCommands();
}
-static void android_os_Binder_init(JNIEnv* env, jobject clazz)
+static void android_os_Binder_init(JNIEnv* env, jobject obj)
{
- JavaBBinderHolder* jbh = new JavaBBinderHolder(env, clazz);
+ JavaBBinderHolder* jbh = new JavaBBinderHolder();
if (jbh == NULL) {
jniThrowException(env, "java/lang/OutOfMemoryError", NULL);
return;
}
- LOGV("Java Binder %p: acquiring first ref on holder %p", clazz, jbh);
- jbh->incStrong(clazz);
- env->SetIntField(clazz, gBinderOffsets.mObject, (int)jbh);
+ LOGV("Java Binder %p: acquiring first ref on holder %p", obj, jbh);
+ jbh->incStrong((void*)android_os_Binder_init);
+ env->SetIntField(obj, gBinderOffsets.mObject, (int)jbh);
}
-static void android_os_Binder_destroy(JNIEnv* env, jobject clazz)
+static void android_os_Binder_destroy(JNIEnv* env, jobject obj)
{
JavaBBinderHolder* jbh = (JavaBBinderHolder*)
- env->GetIntField(clazz, gBinderOffsets.mObject);
+ env->GetIntField(obj, gBinderOffsets.mObject);
if (jbh != NULL) {
- env->SetIntField(clazz, gBinderOffsets.mObject, 0);
- LOGV("Java Binder %p: removing ref on holder %p", clazz, jbh);
- jbh->decStrong(clazz);
+ env->SetIntField(obj, gBinderOffsets.mObject, 0);
+ LOGV("Java Binder %p: removing ref on holder %p", obj, jbh);
+ jbh->decStrong((void*)android_os_Binder_init);
} else {
// Encountering an uninitialized binder is harmless. All it means is that
// the Binder was only partially initialized when its finalizer ran and called
@@ -648,7 +702,7 @@
// For example, a Binder subclass constructor might have thrown an exception before
// it could delegate to its superclass's constructor. Consequently init() would
// not have been called and the holder pointer would remain NULL.
- LOGV("Java Binder %p: ignoring uninitialized binder", clazz);
+ LOGV("Java Binder %p: ignoring uninitialized binder", obj);
}
}
@@ -973,8 +1027,25 @@
LOGV("linkToDeath: binder=%p recipient=%p\n", target, recipient);
if (!target->localBinder()) {
- sp<JavaDeathRecipient> jdr = new JavaDeathRecipient(env, recipient);
- status_t err = target->linkToDeath(jdr, recipient, flags);
+ sp<JavaDeathRecipient> jdr;
+
+ {
+ sp<DeathRecipientList> list;
+ AutoMutex _maplocker(gDeathRecipientMapLock);
+
+ ssize_t listIndex = gDeathRecipientsByIBinder.indexOfKey(target);
+ if (listIndex < 0) {
+ // Set up the death notice bookkeeping for this binder lazily
+ list = new DeathRecipientList;
+ gDeathRecipientsByIBinder.add(target, list);
+ } else {
+ list = gDeathRecipientsByIBinder.valueAt(listIndex);
+ }
+
+ jdr = new JavaDeathRecipient(env, recipient, list);
+ }
+
+ status_t err = target->linkToDeath(jdr, NULL, flags);
if (err != NO_ERROR) {
// Failure adding the death recipient, so clear its reference
// now.
@@ -1003,15 +1074,29 @@
LOGV("unlinkToDeath: binder=%p recipient=%p\n", target, recipient);
if (!target->localBinder()) {
- wp<IBinder::DeathRecipient> dr;
- status_t err = target->unlinkToDeath(NULL, recipient, flags, &dr);
- if (err == NO_ERROR && dr != NULL) {
- sp<IBinder::DeathRecipient> sdr = dr.promote();
- JavaDeathRecipient* jdr = static_cast<JavaDeathRecipient*>(sdr.get());
- if (jdr != NULL) {
- jdr->clearReference();
+ status_t err = NAME_NOT_FOUND;
+ sp<JavaDeathRecipient> origJDR;
+ {
+ AutoMutex _maplocker(gDeathRecipientMapLock);
+ ssize_t listIndex = gDeathRecipientsByIBinder.indexOfKey(target);
+ if (listIndex >= 0) {
+ sp<DeathRecipientList> list = gDeathRecipientsByIBinder.valueAt(listIndex);
+ origJDR = list->find(recipient);
}
}
+ // If we found the matching recipient, proceed to unlink using that
+ if (origJDR != NULL) {
+ wp<IBinder::DeathRecipient> dr;
+ err = target->unlinkToDeath(origJDR, NULL, flags, &dr);
+ if (err == NO_ERROR && dr != NULL) {
+ sp<IBinder::DeathRecipient> sdr = dr.promote();
+ JavaDeathRecipient* jdr = static_cast<JavaDeathRecipient*>(sdr.get());
+ if (jdr != NULL) {
+ jdr->clearReference();
+ }
+ }
+ }
+
if (err == NO_ERROR || err == DEAD_OBJECT) {
res = JNI_TRUE;
} else {
@@ -1031,6 +1116,15 @@
env->SetIntField(obj, gBinderProxyOffsets.mObject, 0);
b->decStrong(obj);
IPCThreadState::self()->flushCommands();
+
+ // tear down the death recipient bookkeeping
+ {
+ AutoMutex _maplocker(gDeathRecipientMapLock);
+ ssize_t listIndex = gDeathRecipientsByIBinder.indexOfKey(b);
+ if (listIndex >= 0) {
+ gDeathRecipientsByIBinder.removeItemsAt((size_t)listIndex);
+ }
+ }
}
// ----------------------------------------------------------------------------
diff --git a/include/binder/IBinder.h b/include/binder/IBinder.h
index 749a977..81b56c2 100644
--- a/include/binder/IBinder.h
+++ b/include/binder/IBinder.h
@@ -98,7 +98,7 @@
* Register the @a recipient for a notification if this binder
* goes away. If this binder object unexpectedly goes away
* (typically because its hosting process has been killed),
- * then DeathRecipient::binderDied() will be called with a referene
+ * then DeathRecipient::binderDied() will be called with a reference
* to this.
*
* The @a cookie is optional -- if non-NULL, it should be a