Don't fail unlinking death recipients on dead binders
It's legal to call unlinkToDeath() *after* receiving binderDied(),
as long as the recipient being unlinked was in fact linked
previously. Don't throw an exception in this case. (The
exception was going unhandled in the system process, bringing
down the runtime. That's bad.)
The change here is a bit subtle. In the new implementation, the
lifetime of a JavaDeathRecipient -- the fundamental bridge between
IBinder objects and the Dalvik/JNI world -- is tied to the
lifetime of the Dalvik-side BinderProxy object it's associated
with. That means it's inappropriate for the JavaDeathRecipient
to disappear [for purposes of being referenced from the Dalvik
side] just because the IBinder has sent out its obituaries, and
instead the JavaDeathRecipient objects are kept around until
the Dalvik-side client code actually drops its reference to the
BinderProxy.
This boils down to a one-line change: we no longer unpin the
JavaDeathRecipients and free them immediately in response to
binderDied().
The rest of the CL is #ifdefed-out debugging that is invaluable
when bugs crop up.
Bug 3513703
Change-Id: I743fa49669c9252f71dcabfd8dcf42ed729b70a5
diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp
index 5deed1e..9e00a7d 100644
--- a/core/jni/android_util_Binder.cpp
+++ b/core/jni/android_util_Binder.cpp
@@ -44,6 +44,13 @@
//#undef LOGV
//#define LOGV(...) fprintf(stderr, __VA_ARGS__)
+#define DEBUG_DEATH 0
+#if DEBUG_DEATH
+#define LOGDEATH LOGD
+#else
+#define LOGDEATH LOGV
+#endif
+
using namespace android;
// ----------------------------------------------------------------------------
@@ -363,6 +370,7 @@
Mutex mLock;
public:
+ DeathRecipientList();
~DeathRecipientList();
void add(const sp<JavaDeathRecipient>& recipient);
@@ -380,6 +388,7 @@
{
// 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());
list->add(this);
android_atomic_inc(&gNumDeathRefs);
@@ -390,7 +399,7 @@
{
JNIEnv* env = javavm_to_jnienv(mVM);
- LOGV("Receiving binderDied() on JavaDeathRecipient %p\n", this);
+ LOGDEATH("Receiving binderDied() on JavaDeathRecipient %p\n", this);
env->CallStaticVoidMethod(gBinderProxyOffsets.mClass,
gBinderProxyOffsets.mSendDeathNotice, mObject);
@@ -399,15 +408,16 @@
report_exception(env, excep,
"*** Uncaught exception returned from death notification!");
}
-
- clearReference();
}
void clearReference()
{
sp<DeathRecipientList> list = mList.promote();
if (list != NULL) {
+ LOGDEATH("Removing JDR %p from DRL %p", this, list.get());
list->remove(this);
+ } else {
+ LOGDEATH("clearReference() on JDR %p but DRL wp purged", this);
}
}
@@ -433,7 +443,12 @@
// ----------------------------------------------------------------------------
+DeathRecipientList::DeathRecipientList() {
+ LOGDEATH("New DRL @ %p", this);
+}
+
DeathRecipientList::~DeathRecipientList() {
+ LOGDEATH("Destroy DRL @ %p", this);
AutoMutex _l(mLock);
// Should never happen -- the JavaDeathRecipient objects that have added themselves
@@ -447,6 +462,7 @@
void DeathRecipientList::add(const sp<JavaDeathRecipient>& recipient) {
AutoMutex _l(mLock);
+ LOGDEATH("DRL @ %p : add JDR %p", this, recipient.get());
mList.push_back(recipient);
}
@@ -456,6 +472,7 @@
List< sp<JavaDeathRecipient> >::iterator iter;
for (iter = mList.begin(); iter != mList.end(); iter++) {
if (*iter == recipient) {
+ LOGDEATH("DRL @ %p : remove JDR %p", this, recipient.get());
mList.erase(iter);
return;
}
@@ -518,7 +535,7 @@
object = env->NewObject(gBinderProxyOffsets.mClass, gBinderProxyOffsets.mConstructor);
if (object != NULL) {
- LOGV("objectForBinder %p: created new proxy %p !\n", val.get(), object);
+ LOGDEATH("objectForBinder %p: created new proxy %p !\n", val.get(), object);
// The proxy holds a reference to the native object.
env->SetIntField(object, gBinderProxyOffsets.mObject, (int)val.get());
val->incStrong(object);
@@ -1030,7 +1047,7 @@
assert(false);
}
- LOGV("linkToDeath: binder=%p recipient=%p\n", target, recipient);
+ LOGDEATH("linkToDeath: binder=%p recipient=%p\n", target, recipient);
if (!target->localBinder()) {
DeathRecipientList* list = (DeathRecipientList*)
@@ -1062,7 +1079,7 @@
return JNI_FALSE;
}
- LOGV("unlinkToDeath: binder=%p recipient=%p\n", target, recipient);
+ LOGDEATH("unlinkToDeath: binder=%p recipient=%p\n", target, recipient);
if (!target->localBinder()) {
status_t err = NAME_NOT_FOUND;
@@ -1071,6 +1088,7 @@
DeathRecipientList* list = (DeathRecipientList*)
env->GetIntField(obj, gBinderProxyOffsets.mOrgue);
sp<JavaDeathRecipient> origJDR = list->find(recipient);
+ LOGDEATH(" unlink found list %p and JDR %p", list, origJDR.get());
if (origJDR != NULL) {
wp<IBinder::DeathRecipient> dr;
err = target->unlinkToDeath(origJDR, NULL, flags, &dr);
@@ -1101,7 +1119,7 @@
DeathRecipientList* drl = (DeathRecipientList*)
env->GetIntField(obj, gBinderProxyOffsets.mOrgue);
- LOGV("Destroying BinderProxy %p: binder=%p drl=%p\n", obj, b, drl);
+ LOGDEATH("Destroying BinderProxy %p: binder=%p drl=%p\n", obj, b, drl);
env->SetIntField(obj, gBinderProxyOffsets.mObject, 0);
env->SetIntField(obj, gBinderProxyOffsets.mOrgue, 0);
drl->decStrong((void*)javaObjectForIBinder);