From a0b525d50b8b514e1e8f5c9b2852ce056a5eb89d Mon Sep 17 00:00:00 2001
From: Andy McFadden <fadden@android.com>
Date: Wed, 4 Aug 2010 12:47:21 -0700
Subject: [PATCH] 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            |  2 +-
 vm/alloc/MarkSweep.c | 17 ++++++++++++++++-
 vm/oo/Class.c        |  6 +++++-
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/vm/Init.c b/vm/Init.c
index 06ceabc7b..872956b8e 100644
--- a/vm/Init.c
+++ b/vm/Init.c
@@ -171,7 +171,7 @@ static void dvmUsage(const char* progName)
 #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
diff --git a/vm/alloc/MarkSweep.c b/vm/alloc/MarkSweep.c
index d35f3dc78..2ccecba90 100644
--- a/vm/alloc/MarkSweep.c
+++ b/vm/alloc/MarkSweep.c
@@ -964,6 +964,11 @@ void dvmHeapFinishMarkStep()
 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 @@ static void sweepBitmapCallback(size_t numPtrs, void **ptrs,
          * 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);
diff --git a/vm/oo/Class.c b/vm/oo/Class.c
index f10a9c601..1f6e68544 100644
--- a/vm/oo/Class.c
+++ b/vm/oo/Class.c
@@ -1444,7 +1444,10 @@ static ClassObject* findClassNoInit(const char* descriptor, Object* loader,
         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 @@ static ClassObject* findClassNoInit(const char* descriptor, Object* loader,
 
             /* Let the GC free the class.
              */
+            dvmFreeClassInnards(clazz);
             dvmReleaseTrackedAlloc((Object*) clazz, NULL);
 
             /* Grab the winning class.
-- 
2.11.0