Track result from dvmCallMethod

When dvmCallMethod returns an object reference, we need to ensure
that the GC doesn't release or relocate the storage.  Unless we have
a clear view of how the object is used, we need to explicitly track
and release it.

This adds additional tracking, or comments indicating that explicit
tracking is not necessary.

On a similar note, clearing/restoring a pending exception requires
explicit tracking of that exception, since there's a fair chance
that it's no longer in the root set.  That needed fixing in a couple
of places.

Bug 3009076.

Change-Id: I39def8c3a5a628f0ee86fc094e34d7c69248c28b

8 files changed, 32 insertions, 7 deletions

diff --git a/vm/Ddm.c b/vm/Ddm.c
index 3cd3a80ba..ee3e9b249 100644
--- a/vm/Ddm.c
+++ b/vm/Ddm.c
@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 /*
  * Handle Dalvik Debug Monitor requests and events.
  *
@@ -41,6 +42,7 @@ bool dvmDdmHandlePacket(const u1* buf, int dataLen, u1** pReplyBuf,
     Thread* self = dvmThreadSelf();
     const int kChunkHdrLen = 8;
     ArrayObject* dataArray = NULL;
+    Object* chunk = NULL;
     bool result = false;
 
     assert(dataLen >= 0);
@@ -122,17 +124,18 @@ bool dvmDdmHandlePacket(const u1* buf, int dataLen, u1** pReplyBuf,
         goto bail;
     }
 
-    Object* chunk;
     ArrayObject* replyData;
     chunk = (Object*) callRes.l;
     if (chunk == NULL)
         goto bail;
 
+    /* not strictly necessary -- we don't alloc from managed heap here */
+    dvmAddTrackedAlloc(chunk, self);
+
     /*
      * Pull the pieces out of the chunk.  We copy the results into a
      * newly-allocated buffer that the caller can free.  We don't want to
      * continue using the Chunk object because nothing has a reference to it.
-     * (If we do an alloc in here, we need to dvmAddTrackedAlloc it.)
      *
      * We could avoid this by returning type/data/offset/length and having
      * the caller be aware of the object lifetime issues, but that
@@ -175,7 +178,8 @@ bool dvmDdmHandlePacket(const u1* buf, int dataLen, u1** pReplyBuf,
         (char*) reply, reply, length);
 
 bail:
-    dvmReleaseTrackedAlloc((Object*) dataArray, NULL);
+    dvmReleaseTrackedAlloc((Object*) dataArray, self);
+    dvmReleaseTrackedAlloc(chunk, self);
     return result;
 }
 
diff --git a/vm/Debugger.c b/vm/Debugger.c
index 7706efb50..4f392360b 100644
--- a/vm/Debugger.c
+++ b/vm/Debugger.c
@@ -2888,6 +2888,7 @@ void dvmDbgExecuteMethod(DebugInvokeReq* pReq)
      * to preserve that across the method invocation.
      */
     oldExcept = dvmGetException(self);
+    dvmAddTrackedAlloc(oldExcept, self);
     dvmClearException(self);
 
     oldStatus = dvmChangeStatus(self, THREAD_RUNNING);
@@ -2941,6 +2942,7 @@ void dvmDbgExecuteMethod(DebugInvokeReq* pReq)
 
     if (oldExcept != NULL)
         dvmSetException(self, oldExcept);
+    dvmReleaseTrackedAlloc(oldExcept, self);
     dvmChangeStatus(self, oldStatus);
 }
 
diff --git a/vm/Exception.c b/vm/Exception.c
index ddb84e5fa..9ca80e65c 100644
--- a/vm/Exception.c
+++ b/vm/Exception.c
@@ -723,6 +723,7 @@ void dvmPrintExceptionStackTrace(void)
     if (exception == NULL)
         return;
 
+    dvmAddTrackedAlloc(exception, self);
     self->exception = NULL;
     printMethod = dvmFindVirtualMethodHierByDescriptor(exception->clazz,
                     "printStackTrace", "()V");
@@ -740,6 +741,7 @@ void dvmPrintExceptionStackTrace(void)
     }
 
     self->exception = exception;
+    dvmReleaseTrackedAlloc(exception, self);
 }
 
 /*
@@ -1250,6 +1252,7 @@ static StringObject* getExceptionMessage(Object* exception)
     StringObject* messageStr = NULL;
 
     assert(exception == self->exception);
+    dvmAddTrackedAlloc(exception, self);
     self->exception = NULL;
 
     getMessageMethod = dvmFindVirtualMethodHierByDescriptor(exception->clazz,
@@ -1276,6 +1279,7 @@ static StringObject* getExceptionMessage(Object* exception)
     }
 
     self->exception = exception;
+    dvmReleaseTrackedAlloc(exception, self);
     return messageStr;
 }
 
diff --git a/vm/Jni.c b/vm/Jni.c
index 71be49c51..43c8b4bd6 100644
--- a/vm/Jni.c
+++ b/vm/Jni.c
@@ -1852,6 +1852,7 @@ static jclass FindClass(JNIEnv* env, const char* name)
     ClassObject* clazz;
     jclass jclazz = NULL;
     Object* loader;
+    Object* trackedLoader = NULL;
     char* descriptor = NULL;
 
     thisMethod = dvmGetCurrentJNIMethod();
@@ -1871,7 +1872,7 @@ static jclass FindClass(JNIEnv* env, const char* name)
     } else if (thisMethod == gDvm.methFakeNativeEntry) {
         /* start point of invocation interface */
         if (!gDvm.initializing)
-            loader = dvmGetSystemClassLoader();
+            loader = trackedLoader = dvmGetSystemClassLoader();
         else
             loader = NULL;
     } else {
@@ -1881,6 +1882,8 @@ static jclass FindClass(JNIEnv* env, const char* name)
     clazz = dvmFindClassNoInit(descriptor, loader);
     jclazz = addLocalReference(env, (Object*) clazz);
 
+    dvmReleaseTrackedAlloc(trackedLoader, _self);
+
 bail:
     free(descriptor);
 
diff --git a/vm/Properties.c b/vm/Properties.c
index 288085b66..243dc3ee3 100644
--- a/vm/Properties.c
+++ b/vm/Properties.c
@@ -246,6 +246,7 @@ void dvmSetCommandLineProperties(Object* propObj)
  */
 char* dvmGetProperty(const char* key)
 {
+    Thread* self = dvmThreadSelf();
     ClassObject* system;
     Method* getProp;
     StringObject* keyObj = NULL;
@@ -270,15 +271,17 @@ char* dvmGetProperty(const char* key)
         goto bail;
 
     JValue val;
-    dvmCallMethod(dvmThreadSelf(), getProp, NULL, &val, keyObj);
+    dvmCallMethod(self, getProp, NULL, &val, keyObj);
     valueObj = (StringObject*) val.l;
     if (valueObj == NULL)
         goto bail;
 
+    /* don't need to call dvmAddTrackedAlloc on result; conv to C string safe */
+
     result = dvmCreateCstrFromString(valueObj);
     /* fall through with result */
 
 bail:
-    dvmReleaseTrackedAlloc((Object*)keyObj, NULL);
+    dvmReleaseTrackedAlloc((Object*)keyObj, self);
     return result;
 }
diff --git a/vm/Thread.c b/vm/Thread.c
index 635fe56c0..afe6f68dc 100644
--- a/vm/Thread.c
+++ b/vm/Thread.c
@@ -887,6 +887,7 @@ bool dvmPrepMainThread(void)
         return false;
     }
     dvmSetFieldObject(threadObj, ctxtClassLoaderOffset, systemLoader);
+    dvmReleaseTrackedAlloc(systemLoader, NULL);
 
     /*
      * Finish our thread prep.
diff --git a/vm/oo/Class.c b/vm/oo/Class.c
index 9bf7118ad..9de277f53 100644
--- a/vm/oo/Class.c
+++ b/vm/oo/Class.c
@@ -4636,9 +4636,12 @@ ClassObject* dvmFindLoadedClass(const char* descriptor)
 
 /*
  * Retrieve the system (a/k/a application) class loader.
+ *
+ * The caller must call dvmReleaseTrackedAlloc on the result.
  */
 Object* dvmGetSystemClassLoader(void)
 {
+    Thread* self = dvmThreadSelf();
     ClassObject* clazz;
     Method* getSysMeth;
     Object* loader;
@@ -4653,8 +4656,9 @@ Object* dvmGetSystemClassLoader(void)
         return NULL;
 
     JValue result;
-    dvmCallMethod(dvmThreadSelf(), getSysMeth, NULL, &result);
+    dvmCallMethod(self, getSysMeth, NULL, &result);
     loader = (Object*)result.l;
+    dvmAddTrackedAlloc(loader, self);
     return loader;
 }
 
diff --git a/vm/reflect/Proxy.c b/vm/reflect/Proxy.c
index eef658dd1..d76877750 100644
--- a/vm/reflect/Proxy.c
+++ b/vm/reflect/Proxy.c
@@ -1013,6 +1013,10 @@ static void proxyInvoker(const u4* args, JValue* pResult,
      *
      * We don't need to repackage exceptions, so if one has been thrown
      * just jump to the end.
+     *
+     * We're not adding invokeResult.l to the tracked allocation list, but
+     * since we're just unboxing it or returning it to interpreted code
+     * that shouldn't be a problem.
      */
     dvmCallMethod(self, invoke, handler, &invokeResult,
         thisObj, methodObj, argArray);