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 (file)
--- a/vm/IndirectRefTable.cpp
+++ b/vm/IndirectRefTable.cpp
@@ -70,14 +70,16 @@ void IndirectRefTable::destroy()
 /*
  * 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 @@ bool IndirectRefTable::getChecked(IndirectRef iref) const
         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 @@ bool IndirectRefTable::getChecked(IndirectRef iref) const
  * 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 @@ bool IndirectRefTable::remove(u4 cookie, IndirectRef iref)
          * 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 @@ bool IndirectRefTable::remove(u4 cookie, IndirectRef iref)
             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 (file)
--- a/vm/IndirectRefTable.h
+++ b/vm/IndirectRefTable.h
@@ -333,7 +333,7 @@ private:
 
     /* 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 (file)
--- a/vm/Jni.cpp
+++ b/vm/Jni.cpp
@@ -323,26 +323,36 @@ Object* dvmDecodeIndirectRef(JNIEnv* env, jobject jobj) {
 
     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.