commit a0b525d50b8b514e1e8f5c9b2852ce056a5eb89d
author Andy McFadden <fadden@android.com> Wed Aug 04 12:47:21 2010 -0700
committer Andy McFadden <fadden@android.com> Wed Aug 04 12:47:21 2010 -0700
tree a7666b3854360163bab26a54aa1078c1985571ef
parent f13e71fd254774440b6c6895f024934587e5f05d

Move dvmFreeClassInnards responsibility.

The GC has been responsible for ensuring that the "innards" of a
class object are freed before the ClassObject itself is discarded.
Since we don't currently unload classes, this situation can only arise
if something went wrong during the loading of a class (e.g. memory
alloc failure part way through), or another thread loaded the same
class at the same time and won the race.  It makes more sense to
have the cleanup occur at the point we decide that our class object
is not useful.

For the most part this was already being done, but there were a couple
of places where it wasn't.

This change
 - adds the missing dvmFreeClassInnards calls
 - changes the GC code to warn without taking action
 - enables the GC code only when assertions are enabled

The code in MarkSweep.c looks like it merits further rearranging, but
I'll leave that to Team GC.

Also, "dalvik_assert" is now only printed on the VM features line when
assertions are enabled.

For bug 2751668.

Change-Id: I90e4455e830766f94f921c7f23b44b423bce3321

vm/Init.c

diff --git a/vm/Init.c b/vm/Init.c
index 06ceabc..872956b 100644
--- a/vm/Init.c
+++ b/vm/Init.c
@@ -171,7 +171,7 @@
 #ifdef WITH_EXTRA_GC_CHECKS
         " extra_gc_checks"
 #endif
-#ifdef WITH_DALVIK_ASSERT
+#if !defined(NDEBUG) && defined(WITH_DALVIK_ASSERT)
         " dalvik_assert"
 #endif
 #ifdef WITH_JNI_STACK_CHECK

vm/alloc/MarkSweep.c

diff --git a/vm/alloc/MarkSweep.c b/vm/alloc/MarkSweep.c
index d35f3dc..2ccecba 100644
--- a/vm/alloc/MarkSweep.c
+++ b/vm/alloc/MarkSweep.c
@@ -964,6 +964,11 @@
 static void sweepBitmapCallback(size_t numPtrs, void **ptrs,
                                 const void *finger, void *arg)
 {
+#ifndef NDEBUG
+    /*
+     * Verify that any classes we're freeing have had their innards
+     * discarded.
+     */
     const ClassObject *const classJavaLangClass = gDvm.classJavaLangClass;
     size_t i;
 
@@ -979,12 +984,22 @@
          * value.
          */
         if (obj->clazz == classJavaLangClass) {
+            const char* descr = ((ClassObject*)obj)->descriptor;
+            if (descr != NULL) {
+                LOGW("WARNING: unfreed innards in class '%s'\n", descr);
+                /* not that important; keep going */
+            } else {
+                LOGV("found freed innards on object %p\n", obj);
+            }
+
             /* dvmFreeClassInnards() may have already been called,
              * but it's safe to call on the same ClassObject twice.
              */
-            dvmFreeClassInnards((ClassObject *)obj);
+            //dvmFreeClassInnards((ClassObject *)obj);
         }
     }
+#endif
+
     // TODO: dvmHeapSourceFreeList has a loop, just like the above
     // does. Consider collapsing the two loops to save overhead.
     dvmHeapSourceFreeList(numPtrs, ptrs);

vm/oo/Class.c

diff --git a/vm/oo/Class.c b/vm/oo/Class.c
index f10a9c6..1f6e685 100644
--- a/vm/oo/Class.c
+++ b/vm/oo/Class.c
@@ -1444,7 +1444,10 @@
         clazz = loadClassFromDex(pDvmDex, pClassDef, loader);
         if (dvmCheckException(self)) {
             /* class was found but had issues */
-            dvmReleaseTrackedAlloc((Object*) clazz, NULL);
+            if (clazz != NULL) {
+                dvmFreeClassInnards(clazz);
+                dvmReleaseTrackedAlloc((Object*) clazz, NULL);
+            }
             goto bail;
         }
 
@@ -1476,6 +1479,7 @@
 
             /* Let the GC free the class.
              */
+            dvmFreeClassInnards(clazz);
             dvmReleaseTrackedAlloc((Object*) clazz, NULL);
 
             /* Grab the winning class.