Revert "Binder linkage no longer depends on JNI objrefs as persistent tokens"

This reverts commit c2d55dd89743c8a38deb809f3cdf1ad2d1dbac2b.

1 files changed, 51 insertions, 145 deletions

diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp
index bd5305d..7a53874 100644
--- a/core/jni/android_util_Binder.cpp
+++ b/core/jni/android_util_Binder.cpp
@@ -31,8 +31,6 @@
 #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>
@@ -324,15 +322,25 @@ private:
 class JavaBBinderHolder : public RefBase
 {
 public:
-    sp<JavaBBinder> get(JNIEnv* env, jobject obj)
+    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)
     {
         AutoMutex _l(mLock);
         sp<JavaBBinder> b = mBinder.promote();
         if (b == NULL) {
-            b = new JavaBBinder(env, obj);
+            b = new JavaBBinder(env, mObject);
             mBinder = b;
             LOGV("Creating JavaBinder %p (refs %p) for Object %p, weakCount=%d\n",
-                 b.get(), b->getWeakRefs(), obj, b->getWeakRefs()->getWeakCount());
+                 b.get(), b->getWeakRefs(), mObject, b->getWeakRefs()->getWeakCount());
         }
 
         return b;
@@ -346,41 +354,20 @@ public:
 
 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, sp<DeathRecipientList>& list)
-        : mVM(jnienv_to_javavm(env)), mObject(env->NewGlobalRef(object)), mList(list)
+    JavaDeathRecipient(JNIEnv* env, jobject object)
+        : mVM(jnienv_to_javavm(env)), mObject(env->NewGlobalRef(object)),
+          mHoldsRef(true)
     {
-        // These objects manage their own lifetimes so are responsible for final bookkeeping.
-        // The list holds a strong reference to this object.
-        mList->add(this);
-
+        incStrong(this);
         android_atomic_inc(&gNumDeathRefs);
         incRefsCreated(env);
     }
@@ -404,12 +391,16 @@ public:
 
     void clearReference()
     {
-        mList->remove(this);
-    }
-
-    bool matches(jobject obj) {
-        JNIEnv* env = javavm_to_jnienv(mVM);
-        return env->IsSameObject(obj, mObject);
+        bool release = false;
+        mLock.lock();
+        if (mHoldsRef) {
+            mHoldsRef = false;
+            release = true;
+        }
+        mLock.unlock();
+        if (release) {
+            decStrong(this);
+        }
     }
 
 protected:
@@ -424,57 +415,12 @@ protected:
 private:
     JavaVM* const   mVM;
     jobject const   mObject;
-    sp<DeathRecipientList> mList;
+    Mutex           mLock;
+    bool            mHoldsRef;
 };
 
 // ----------------------------------------------------------------------------
 
-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)
@@ -544,7 +490,7 @@ sp<IBinder> ibinderForJavaObject(JNIEnv* env, jobject obj)
     if (env->IsInstanceOf(obj, gBinderOffsets.mClass)) {
         JavaBBinderHolder* jbh = (JavaBBinderHolder*)
             env->GetIntField(obj, gBinderOffsets.mObject);
-        return jbh != NULL ? jbh->get(env, obj) : NULL;
+        return jbh != NULL ? jbh->get(env) : NULL;
     }
 
     if (env->IsInstanceOf(obj, gBinderProxyOffsets.mClass)) {
@@ -675,26 +621,26 @@ static void android_os_Binder_flushPendingCommands(JNIEnv* env, jobject clazz)
     IPCThreadState::self()->flushCommands();
 }
 
-static void android_os_Binder_init(JNIEnv* env, jobject obj)
+static void android_os_Binder_init(JNIEnv* env, jobject clazz)
 {
-    JavaBBinderHolder* jbh = new JavaBBinderHolder();
+    JavaBBinderHolder* jbh = new JavaBBinderHolder(env, clazz);
     if (jbh == NULL) {
         jniThrowException(env, "java/lang/OutOfMemoryError", NULL);
         return;
     }
-    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);
+    LOGV("Java Binder %p: acquiring first ref on holder %p", clazz, jbh);
+    jbh->incStrong(clazz);
+    env->SetIntField(clazz, gBinderOffsets.mObject, (int)jbh);
 }
 
-static void android_os_Binder_destroy(JNIEnv* env, jobject obj)
+static void android_os_Binder_destroy(JNIEnv* env, jobject clazz)
 {
     JavaBBinderHolder* jbh = (JavaBBinderHolder*)
-        env->GetIntField(obj, gBinderOffsets.mObject);
+        env->GetIntField(clazz, gBinderOffsets.mObject);
     if (jbh != NULL) {
-        env->SetIntField(obj, gBinderOffsets.mObject, 0);
-        LOGV("Java Binder %p: removing ref on holder %p", obj, jbh);
-        jbh->decStrong((void*)android_os_Binder_init);
+        env->SetIntField(clazz, gBinderOffsets.mObject, 0);
+        LOGV("Java Binder %p: removing ref on holder %p", clazz, jbh);
+        jbh->decStrong(clazz);
     } else {
         // Encountering an uninitialized binder is harmless.  All it means is that
         // the Binder was only partially initialized when its finalizer ran and called
@@ -702,7 +648,7 @@ static void android_os_Binder_destroy(JNIEnv* env, jobject obj)
         // 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", obj);
+        LOGV("Java Binder %p: ignoring uninitialized binder", clazz);
     }
 }
 
@@ -1027,25 +973,8 @@ static void android_os_BinderProxy_linkToDeath(JNIEnv* env, jobject obj,
     LOGV("linkToDeath: binder=%p recipient=%p\n", target, recipient);
 
     if (!target->localBinder()) {
-        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);
+        sp<JavaDeathRecipient> jdr = new JavaDeathRecipient(env, recipient);
+        status_t err = target->linkToDeath(jdr, recipient, flags);
         if (err != NO_ERROR) {
             // Failure adding the death recipient, so clear its reference
             // now.
@@ -1074,29 +1003,15 @@ static jboolean android_os_BinderProxy_unlinkToDeath(JNIEnv* env, jobject obj,
     LOGV("unlinkToDeath: binder=%p recipient=%p\n", target, recipient);
 
     if (!target->localBinder()) {
-        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();
-                }
+        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();
             }
         }
-
         if (err == NO_ERROR || err == DEAD_OBJECT) {
             res = JNI_TRUE;
         } else {
@@ -1116,15 +1031,6 @@ static void android_os_BinderProxy_destroy(JNIEnv* env, jobject obj)
     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);
-        }
-    }
 }
 
 // ----------------------------------------------------------------------------