Don't abort when a weak global's referent is cleared.
This also makes us less likely to output spurious warnings when
dealing with nulled-out weak globals, and lets us provide more
helpful warnings when warnings are called for.
Bug: 4991942
Change-Id: I99b88e66e07f79562da2cd9d594b93bff218d595
diff --git a/vm/IndirectRefTable.cpp b/vm/IndirectRefTable.cpp
index f2e7ca2..fcf59af 100644
--- a/vm/IndirectRefTable.cpp
+++ b/vm/IndirectRefTable.cpp
@@ -70,14 +70,16 @@
/*
* Make sure that the entry at "idx" is correctly paired with "iref".
*/
-bool IndirectRefTable::checkEntry(IndirectRef iref, int idx) const
+bool IndirectRefTable::checkEntry(const char* what, IndirectRef iref, int idx) const
{
Object* obj = table[idx];
IndirectRef checkRef = toIndirectRef(obj, idx);
if (checkRef != iref) {
- LOGE("JNI ERROR (app bug): use of stale %s reference (req=%p vs cur=%p; table=%p)",
- indirectRefKindToString(kind), iref, checkRef, this);
- abortMaybe();
+ if (indirectRefKind(iref) != kIndirectKindWeakGlobal) {
+ LOGE("JNI ERROR (app bug): attempt to %s stale %s reference (req=%p vs cur=%p; table=%p)",
+ what, indirectRefKindToString(kind), iref, checkRef, this);
+ abortMaybe();
+ }
return false;
}
return true;
@@ -185,14 +187,7 @@
return false;
}
- Object* obj = table[idx];
- if (obj == NULL) {
- LOGE("JNI ERROR (app bug): use of deleted %s reference (%p)",
- indirectRefKindToString(kind), iref);
- abortMaybe();
- return false;
- }
- if (!checkEntry(iref, idx)) {
+ if (!checkEntry("use", iref, idx)) {
return false;
}
@@ -208,7 +203,7 @@
* required by JNI's DeleteLocalRef function.
*
* Note this is NOT called when a local frame is popped. This is only used
- * for explict single removals.
+ * for explicit single removals.
*
* Returns "false" if nothing was removed.
*/
@@ -242,7 +237,7 @@
* Top-most entry. Scan up and consume holes. No need to NULL
* out the entry, since the test vs. topIndex will catch it.
*/
- if (!checkEntry(iref, idx)) {
+ if (!checkEntry("remove", iref, idx)) {
return false;
}
updateSlotRemove(idx);
@@ -279,7 +274,7 @@
LOGV("--- WEIRD: removing null entry %d", idx);
return false;
}
- if (!checkEntry(iref, idx)) {
+ if (!checkEntry("remove", iref, idx)) {
return false;
}
updateSlotRemove(idx);
diff --git a/vm/IndirectRefTable.h b/vm/IndirectRefTable.h
index 4b91c88..c8d5ab5 100644
--- a/vm/IndirectRefTable.h
+++ b/vm/IndirectRefTable.h
@@ -333,7 +333,7 @@
/* extra debugging checks */
bool getChecked(IndirectRef) const;
- bool checkEntry(IndirectRef, int) const;
+ bool checkEntry(const char*, IndirectRef, int) const;
};
#endif // DALVIK_INDIRECTREFTABLE_H_
diff --git a/vm/Jni.cpp b/vm/Jni.cpp
index f0943f8..218cdb8 100644
--- a/vm/Jni.cpp
+++ b/vm/Jni.cpp
@@ -323,26 +323,36 @@
switch (indirectRefKind(jobj)) {
case kIndirectKindLocal:
- return getLocalRefTable(env)->get(jobj);
+ {
+ Object* result = getLocalRefTable(env)->get(jobj);
+ if (result == NULL) {
+ LOGE("JNI ERROR (app bug): use of deleted local reference (%p)", jobj);
+ dvmAbort();
+ }
+ return result;
+ }
case kIndirectKindGlobal:
{
// TODO: find a way to avoid the mutex activity here
IndirectRefTable* pRefTable = &gDvm.jniGlobalRefTable;
ScopedPthreadMutexLock lock(&gDvm.jniGlobalRefLock);
- return pRefTable->get(jobj);
+ Object* result = pRefTable->get(jobj);
+ if (result == NULL) {
+ LOGE("JNI ERROR (app bug): use of deleted global reference (%p)", jobj);
+ dvmAbort();
+ }
+ return result;
}
case kIndirectKindWeakGlobal:
{
- Object* result;
- {
- // TODO: find a way to avoid the mutex activity here
- IndirectRefTable* pRefTable = &gDvm.jniWeakGlobalRefTable;
- ScopedPthreadMutexLock lock(&gDvm.jniWeakGlobalRefLock);
- result = pRefTable->get(jobj);
- }
+ // TODO: find a way to avoid the mutex activity here
+ IndirectRefTable* pRefTable = &gDvm.jniWeakGlobalRefTable;
+ ScopedPthreadMutexLock lock(&gDvm.jniWeakGlobalRefLock);
+ Object* result = pRefTable->get(jobj);
+ // A cleared weak global reference means we might legitimately return NULL here.
/*
* TODO: this is a temporary workaround for broken weak global
- * refs (bug 4260055). We treat any invalid reference as if it
+ * refs (http://b/4260055). We treat any invalid reference as if it
* were a weak global with a cleared referent. This means that
* actual invalid references won't be detected, and if an empty
* slot gets re-used we will return the new reference instead.
