drgreen/android_frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Don't fail unlinking death recipients on dead binders

Browse Source 
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
This commit is contained in:
Christopher Tate
2011-03-04 17:45:00 -08:00
parent 371fd13c31
commit 79dd31f73d
1 changed files with 25 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
core/jni/android_util_Binder.cpp
View File

@ -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 @@ class DeathRecipientList : public RefBase {
Mutex mLock;
public:
DeathRecipientList();
~DeathRecipientList();
void add(const sp<JavaDeathRecipient>& recipient);
@ -380,6 +388,7 @@ public:
{
// 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 @@ public:
{
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 @@ public:
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 @@ private:
// ----------------------------------------------------------------------------
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 @@ DeathRecipientList::~DeathRecipientList() {
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 @@ void DeathRecipientList::remove(const sp<JavaDeathRecipient>& recipient) {
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 @@ jobject javaObjectForIBinder(JNIEnv* env, const sp<IBinder>& val)
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 @@ static void android_os_BinderProxy_linkToDeath(JNIEnv* env, jobject obj,
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 @@ static jboolean android_os_BinderProxy_unlinkToDeath(JNIEnv* env, jobject obj,
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 @@ static jboolean android_os_BinderProxy_unlinkToDeath(JNIEnv* env, jobject obj,
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 @@ static void android_os_BinderProxy_destroy(JNIEnv* env, jobject obj)
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);