commit 30646836b4a1c6e5e80ddaea246cf9669eaa0628
author Elliott Hughes <enh@google.com> Thu Oct 13 16:59:46 2011 -0700
committer Elliott Hughes <enh@google.com> Thu Oct 13 16:59:46 2011 -0700
tree 5713b45d0e42e4ec319d8aea75b6d2a6bdb079c3
parent 95caa791e560da97363c0c0d22bfda4a7e7377c3

Fix the double-OOME case again.

The key ingredient to this change is that the NULL check on the NewStringUTF
call in ClassLinker::FindClass was accidentally checking the input to the
function (which we know isn't null) rather than the output (which will be
if allocation fails).

I've also fixed a bunch of places where we were missing null checks on
allocation, but there's a lot more to do.

I've stopped -Xcheck:jni from trying to dump pending OutOfMemoryError stacks
because that's highly likely to go horribly wrong and just gets in the way
of debugging the actual problem. (We could perhaps do better, so I've added
a TODO.)

In DexFile, we were always allocating java.lang.Strings for the names of
each local variable, even if we were only parsing the debug info for the
purposes of translating a line number. This was less of a problem in Dalvik
where the names were just const char*s, but since we go straight to Strings,
this was causing yet more OOME carnage (and is a lot of wasted effort).

This fixes the regression we were seeing in 061.

Change-Id: I6241602756db3070b214ccb35e3760b37e1e3d10

src/thread.cc

diff --git a/src/thread.cc b/src/thread.cc
index 3c19fe4..62b3290 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -1076,6 +1076,9 @@
   } else {
     // Create java_trace array and place in local reference table
     java_traces = class_linker->AllocStackTraceElementArray(depth);
+    if (java_traces == NULL) {
+      return NULL;
+    }
     result = AddLocalReference<jobjectArray>(ts.Env(), java_traces);
   }
 
@@ -1096,8 +1099,10 @@
         StackTraceElement::Alloc(String::AllocFromModifiedUtf8(class_name.c_str()),
                                  method->GetName(),
                                  klass->GetSourceFile(),
-                                 dex_file.GetLineNumFromPC(method,
-                                     method->ToDexPC(native_pc)));
+                                 dex_file.GetLineNumFromPC(method, method->ToDexPC(native_pc)));
+    if (obj == NULL) {
+      return NULL;
+    }
 #ifdef MOVING_GARBAGE_COLLECTOR
     // Re-read after potential GC
     java_traces = Decode<ObjectArray<Object>*>(ts.Env(), result);
@@ -1131,14 +1136,25 @@
 
   JNIEnv* env = GetJniEnv();
   ScopedLocalRef<jclass> exception_class(env, env->FindClass(descriptor.c_str()));
-  CHECK(exception_class.get() != NULL) << "descriptor=\"" << descriptor << "\"";
+  if (exception_class.get() == NULL) {
+    LOG(ERROR) << "Couldn't throw new " << descriptor << " because JNI FindClass failed: "
+               << PrettyTypeOf(GetException());
+    CHECK(IsExceptionPending());
+    return;
+  }
   int rc = env->ThrowNew(exception_class.get(), msg);
-  CHECK_EQ(rc, JNI_OK);
+  if (rc != JNI_OK) {
+    LOG(ERROR) << "Couldn't throw new " << descriptor << " because JNI ThrowNew failed: "
+               << PrettyTypeOf(GetException());
+    CHECK(IsExceptionPending());
+    return;
+  }
 }
 
 void Thread::ThrowOutOfMemoryError(Class* c, size_t byte_count) {
-  LOG(ERROR) << "Failed to allocate a " << PrettyDescriptor(c->GetDescriptor())
-             << " (" << byte_count << " bytes)" << (throwing_OutOfMemoryError_ ? " recursive case" : "");
+  LOG(ERROR) << "Failed to allocate a " << byte_count << "-byte "
+             << PrettyDescriptor(c->GetDescriptor())
+             << (throwing_OutOfMemoryError_ ? " (recursive case)" : "");
   if (!throwing_OutOfMemoryError_) {
     throwing_OutOfMemoryError_ = true;
     ThrowNewException("Ljava/lang/OutOfMemoryError;", NULL);