Finish the OutOfMemoryError implementation.
This copes with the double-OOME case.
Also check that I don't leave local references in a newly-attached thread's
local reference table, and fix the leaks this discovered.
Also fix the code that implements fillInNativeStackTrace to cope with the
situation where we're not able to allocate (because we're throwing
OutOfMemoryError and there's not enough space left for our arrays).
Also fix the order of checking for a pending exception and popping the
local references in the JNI native method invocation stub. (This fixes
the warnings we'd been seeing from the IndirectReferenceTable in test 064.)
Also improve some -Xcheck:jni output.
This fixes test 061.
Change-Id: Icc04a2c06339bd28d6772190350a86abfc5734b8
diff --git a/src/check_jni.cc b/src/check_jni.cc
index cb33d93..26068fd 100644
--- a/src/check_jni.cc
+++ b/src/check_jni.cc
@@ -268,13 +268,16 @@
return;
}
if (*expectedType != m->GetShorty()->CharAt(0)) {
- LOG(ERROR) << "JNI ERROR: expected return type '" << *expectedType << "' calling " << PrettyMethod(m);
+ LOG(ERROR) << "JNI ERROR: the return type of " << function_name_ << " does not match "
+ << PrettyMethod(m);
JniAbort();
} else if (isStatic && !m->IsStatic()) {
if (isStatic) {
- LOG(ERROR) << "JNI ERROR: calling non-static method " << PrettyMethod(m) << " with static call";
+ LOG(ERROR) << "JNI ERROR: calling non-static method "
+ << PrettyMethod(m) << " with " << function_name_;
} else {
- LOG(ERROR) << "JNI ERROR: calling static method " << PrettyMethod(m) << " with non-static call";
+ LOG(ERROR) << "JNI ERROR: calling static method "
+ << PrettyMethod(m) << " with non-static " << function_name_;
}
JniAbort();
}
@@ -723,7 +726,8 @@
* make any JNI calls other than the Exception* methods.
*/
if ((flags & kFlag_ExcepOkay) == 0 && self->IsExceptionPending()) {
- LOG(ERROR) << "JNI ERROR: JNI method called with exception pending";
+ LOG(ERROR) << "JNI ERROR: JNI " << function_name_ << " called with "
+ << PrettyTypeOf(self->GetException()) << " pending";
LOG(ERROR) << "Pending exception is:";
LOG(ERROR) << jniGetStackTrace(env_);
JniAbort();
diff --git a/src/indirect_reference_table.cc b/src/indirect_reference_table.cc
index e78b362..4283b83 100644
--- a/src/indirect_reference_table.cc
+++ b/src/indirect_reference_table.cc
@@ -158,6 +158,13 @@
return result;
}
+void IndirectReferenceTable::AssertEmpty() {
+ if (begin() != end()) {
+ Dump();
+ LOG(FATAL) << "Internal Error: non-empty local reference table";
+ }
+}
+
/*
* Verify that the indirect table lookup is valid.
*
@@ -250,15 +257,15 @@
}
if (idx < bottomIndex) {
- /* wrong segment */
- LOG(INFO) << "Attempt to remove index outside index area (" << idx
- << " vs " << bottomIndex << "-" << topIndex << ")";
+ // Wrong segment.
+ LOG(WARNING) << "Attempt to remove index outside index area (" << idx
+ << " vs " << bottomIndex << "-" << topIndex << ")";
return false;
}
if (idx >= topIndex) {
- /* bad -- stale reference? */
- LOG(INFO) << "Attempt to remove invalid index " << idx
- << " (bottom=" << bottomIndex << " top=" << topIndex << ")";
+ // Bad --- stale reference?
+ LOG(WARNING) << "Attempt to remove invalid index " << idx
+ << " (bottom=" << bottomIndex << " top=" << topIndex << ")";
return false;
}
diff --git a/src/indirect_reference_table.h b/src/indirect_reference_table.h
index 5dca3d3..ba20d4d 100644
--- a/src/indirect_reference_table.h
+++ b/src/indirect_reference_table.h
@@ -242,6 +242,10 @@
size_t capacity_;
};
+bool inline operator==(const IrtIterator& lhs, const IrtIterator& rhs) {
+ return lhs.equals(rhs);
+}
+
bool inline operator!=(const IrtIterator& lhs, const IrtIterator& rhs) {
return !lhs.equals(rhs);
}
@@ -290,6 +294,8 @@
*/
bool Remove(uint32_t cookie, IndirectRef iref);
+ void AssertEmpty();
+
void Dump() const;
/*
diff --git a/src/jni_compiler.cc b/src/jni_compiler.cc
index ffcbc11..127b53d 100644
--- a/src/jni_compiler.cc
+++ b/src/jni_compiler.cc
@@ -368,10 +368,7 @@
jni_conv->ReturnRegister(), return_save_location,
jni_conv->SizeOfReturnValue());
- // 13. Check for pending exception and forward if there
- __ ExceptionPoll(jni_conv->InterproceduralScratchRegister());
-
- // 14. Place result in correct register possibly loading from indirect
+ // 13. Place result in correct register possibly loading from indirect
// reference table
if (jni_conv->IsReturnAReference()) {
__ IncreaseFrameSize(out_arg_size);
@@ -401,7 +398,7 @@
}
__ Move(mr_conv->ReturnRegister(), jni_conv->ReturnRegister());
- // 15. Restore segment state and remove SIRT from thread
+ // 14. Restore segment state and remove SIRT from thread
{
ManagedRegister jni_env = jni_conv->InterproceduralScratchRegister();
__ LoadRawPtrFromThread(jni_env, Thread::JniEnvOffset());
@@ -417,10 +414,15 @@
__ CopyRawPtrToThread(Thread::TopSirtOffset(), jni_conv->SirtLinkOffset(),
jni_conv->InterproceduralScratchRegister());
+ // 15. Check for pending exception and forward if there
+ __ ExceptionPoll(jni_conv->InterproceduralScratchRegister());
+
// 16. Remove activation
if (native_method->IsSynchronized()) {
__ RemoveFrame(frame_size, callee_save_regs);
} else {
+ // no need to restore callee save registers because we didn't
+ // clobber them while locking the monitor.
__ RemoveFrame(frame_size, std::vector<ManagedRegister>());
}
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index f9d536b..9ca4034 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -92,10 +92,11 @@
template Class* Decode<Class*>(JNIEnv*, jobject);
template ClassLoader* Decode<ClassLoader*>(JNIEnv*, jobject);
template Object* Decode<Object*>(JNIEnv*, jobject);
-template String* Decode<String*>(JNIEnv*, jobject);
template ObjectArray<Class>* Decode<ObjectArray<Class>*>(JNIEnv*, jobject);
template ObjectArray<Object>* Decode<ObjectArray<Object>*>(JNIEnv*, jobject);
template ObjectArray<StackTraceElement>* Decode<ObjectArray<StackTraceElement>*>(JNIEnv*, jobject);
+template String* Decode<String*>(JNIEnv*, jobject);
+template Throwable* Decode<Throwable*>(JNIEnv*, jobject);
namespace {
diff --git a/src/jni_internal_test.cc b/src/jni_internal_test.cc
index 95e22ba..ddd39b5 100644
--- a/src/jni_internal_test.cc
+++ b/src/jni_internal_test.cc
@@ -7,6 +7,7 @@
#include <cmath>
#include "common_test.h"
+#include "ScopedLocalRef.h"
namespace art {
@@ -22,11 +23,19 @@
env_ = Thread::Current()->GetJniEnv();
- aioobe_ = env_->FindClass("java/lang/ArrayIndexOutOfBoundsException");
- CHECK(aioobe_ != NULL);
+ ScopedLocalRef<jclass> aioobe(env_, env_->FindClass("java/lang/ArrayIndexOutOfBoundsException"));
+ CHECK(aioobe.get() != NULL);
+ aioobe_ = reinterpret_cast<jclass>(env_->NewGlobalRef(aioobe.get()));
- sioobe_ = env_->FindClass("java/lang/StringIndexOutOfBoundsException");
- CHECK(sioobe_ != NULL);
+ ScopedLocalRef<jclass> sioobe(env_, env_->FindClass("java/lang/StringIndexOutOfBoundsException"));
+ CHECK(sioobe.get() != NULL);
+ sioobe_ = reinterpret_cast<jclass>(env_->NewGlobalRef(sioobe.get()));
+ }
+
+ virtual void TearDown() {
+ env_->DeleteGlobalRef(aioobe_);
+ env_->DeleteGlobalRef(sioobe_);
+ CommonTest::TearDown();
}
JavaVMExt* vm_;
diff --git a/src/runtime.cc b/src/runtime.cc
index e46cf88..64df1d3 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -14,6 +14,7 @@
#include "intern_table.h"
#include "jni_internal.h"
#include "oat_file.h"
+#include "ScopedLocalRef.h"
#include "signal_catcher.h"
#include "space.h"
#include "thread.h"
@@ -326,6 +327,8 @@
StartDaemonThreads();
+ Thread::Current()->GetJniEnv()->locals.AssertEmpty();
+
if (IsVerboseStartup()) {
LOG(INFO) << "Runtime::Start exiting";
}
@@ -336,11 +339,11 @@
Thread* self = Thread::Current();
JNIEnv* env = self->GetJniEnv();
- jclass c = env->FindClass("java/lang/Daemons");
- CHECK(c != NULL);
- jmethodID mid = env->GetStaticMethodID(c, "start", "()V");
+ ScopedLocalRef<jclass> c(env, env->FindClass("java/lang/Daemons"));
+ CHECK(c.get() != NULL);
+ jmethodID mid = env->GetStaticMethodID(c.get(), "start", "()V");
CHECK(mid != NULL);
- env->CallStaticVoidMethod(c, mid);
+ env->CallStaticVoidMethod(c.get(), mid);
}
bool Runtime::IsStarted() const {
diff --git a/src/thread.cc b/src/thread.cc
index 9f7fc20..400da0c 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -35,6 +35,7 @@
#include "object.h"
#include "runtime.h"
#include "runtime_support.h"
+#include "ScopedLocalRef.h"
#include "scoped_jni_thread_state.h"
#include "stack.h"
#include "stack_indirect_reference_table.h"
@@ -266,41 +267,48 @@
self->CreatePeer(name, as_daemon);
}
+ self->GetJniEnv()->locals.AssertEmpty();
+
return self;
}
jobject GetWellKnownThreadGroup(JNIEnv* env, const char* field_name) {
- jclass thread_group_class = env->FindClass("java/lang/ThreadGroup");
- jfieldID fid = env->GetStaticFieldID(thread_group_class, field_name, "Ljava/lang/ThreadGroup;");
- jobject thread_group = env->GetStaticObjectField(thread_group_class, fid);
- // This will be null in the compiler (and tests), but never in a running system.
- //CHECK(thread_group != NULL) << "java.lang.ThreadGroup." << field_name << " not initialized";
- return thread_group;
+ ScopedLocalRef<jclass> thread_group_class(env, env->FindClass("java/lang/ThreadGroup"));
+ jfieldID fid = env->GetStaticFieldID(thread_group_class.get(), field_name, "Ljava/lang/ThreadGroup;");
+ return env->GetStaticObjectField(thread_group_class.get(), fid);
}
void Thread::CreatePeer(const char* name, bool as_daemon) {
JNIEnv* env = jni_env_;
const char* field_name = (GetThinLockId() == ThreadList::kMainId) ? "mMain" : "mSystem";
- jobject thread_group = GetWellKnownThreadGroup(env, field_name);
- jobject thread_name = env->NewStringUTF(name);
+ ScopedLocalRef<jobject> thread_group(env, GetWellKnownThreadGroup(env, field_name));
+ ScopedLocalRef<jobject> thread_name(env, env->NewStringUTF(name));
jint thread_priority = GetNativePriority();
jboolean thread_is_daemon = as_daemon;
- jclass c = env->FindClass("java/lang/Thread");
- jmethodID mid = env->GetMethodID(c, "<init>", "(Ljava/lang/ThreadGroup;Ljava/lang/String;IZ)V");
+ ScopedLocalRef<jclass> c(env, env->FindClass("java/lang/Thread"));
+ jmethodID mid = env->GetMethodID(c.get(), "<init>", "(Ljava/lang/ThreadGroup;Ljava/lang/String;IZ)V");
- jobject peer = env->NewObject(c, mid, thread_group, thread_name, thread_priority, thread_is_daemon);
- peer_ = DecodeJObject(peer);
+ ScopedLocalRef<jobject> peer(env,
+ env->NewObject(c.get(), mid, thread_group.get(), thread_name.get(), thread_priority, thread_is_daemon));
+ peer_ = DecodeJObject(peer.get());
SetVmData(peer_, Thread::Current());
// Because we mostly run without code available (in the compiler, in tests), we
// manually assign the fields the constructor should have set.
// TODO: lose this.
gThread_daemon->SetBoolean(peer_, thread_is_daemon);
- gThread_group->SetObject(peer_, Decode<Object*>(env, thread_group));
- gThread_name->SetObject(peer_, Decode<Object*>(env, thread_name));
+ gThread_group->SetObject(peer_, Decode<Object*>(env, thread_group.get()));
+ gThread_name->SetObject(peer_, Decode<Object*>(env, thread_name.get()));
gThread_priority->SetInt(peer_, thread_priority);
+
+ // Pre-allocate an OutOfMemoryError for the double-OOME case.
+ ThrowNewException("Ljava/lang/OutOfMemoryError;",
+ "OutOfMemoryError thrown while trying to throw OutOfMemoryError; no stack available");
+ ScopedLocalRef<jthrowable> exception(env, env->ExceptionOccurred());
+ env->ExceptionClear();
+ pre_allocated_OutOfMemoryError_ = Decode<Throwable*>(env, exception.get());
}
void Thread::InitStackHwm() {
@@ -697,7 +705,8 @@
suspend_count_(0),
class_loader_override_(NULL),
long_jump_context_(NULL),
- throwing_OOME_(false) {
+ throwing_OutOfMemoryError_(false),
+ pre_allocated_OutOfMemoryError_(NULL) {
CHECK_EQ((sizeof(Thread) % 4), 0U) << sizeof(Thread);
}
@@ -812,11 +821,6 @@
}
}
-void Thread::PopSirt() {
- CHECK(top_sirt_ != NULL);
- top_sirt_ = top_sirt_->Link();
-}
-
Object* Thread::DecodeJObject(jobject obj) {
DCHECK(CanAccessDirectReferences());
if (obj == NULL) {
@@ -913,12 +917,18 @@
class BuildInternalStackTraceVisitor : public Thread::StackVisitor {
public:
explicit BuildInternalStackTraceVisitor(int depth, int skip_depth, ScopedJniThreadState& ts)
- : skip_depth_(skip_depth), count_(0) {
+ : skip_depth_(skip_depth), count_(0), pc_trace_(NULL), method_trace_(NULL), local_ref_(NULL) {
// Allocate method trace with an extra slot that will hold the PC trace
method_trace_ = Runtime::Current()->GetClassLinker()->AllocObjectArray<Object>(depth + 1);
+ if (method_trace_ == NULL) {
+ return;
+ }
// Register a local reference as IntArray::Alloc may trigger GC
local_ref_ = AddLocalReference<jobject>(ts.Env(), method_trace_);
pc_trace_ = IntArray::Alloc(depth);
+ if (pc_trace_ == NULL) {
+ return;
+ }
#ifdef MOVING_GARBAGE_COLLECTOR
// Re-read after potential GC
method_trace = Decode<ObjectArray<Object>*>(ts.Env(), local_ref_);
@@ -931,6 +941,9 @@
virtual ~BuildInternalStackTraceVisitor() {}
virtual void VisitFrame(const Frame& frame, uintptr_t pc) {
+ if (method_trace_ == NULL || pc_trace_ == NULL) {
+ return; // We're probably trying to fillInStackTrace for an OutOfMemoryError.
+ }
if (skip_depth_ > 0) {
skip_depth_--;
return;
@@ -1109,22 +1122,22 @@
descriptor.erase(descriptor.length() - 1);
JNIEnv* env = GetJniEnv();
- jclass exception_class = env->FindClass(descriptor.c_str());
- CHECK(exception_class != NULL) << "descriptor=\"" << descriptor << "\"";
- int rc = env->ThrowNew(exception_class, msg);
+ ScopedLocalRef<jclass> exception_class(env, env->FindClass(descriptor.c_str()));
+ CHECK(exception_class.get() != NULL) << "descriptor=\"" << descriptor << "\"";
+ int rc = env->ThrowNew(exception_class.get(), msg);
CHECK_EQ(rc, JNI_OK);
- env->DeleteLocalRef(exception_class);
}
void Thread::ThrowOutOfMemoryError(Class* c, size_t byte_count) {
- if (!throwing_OOME_) {
- throwing_OOME_ = true;
+ LOG(ERROR) << "Failed to allocate a " << PrettyDescriptor(c->GetDescriptor())
+ << " (" << byte_count << " bytes)" << (throwing_OutOfMemoryError_ ? " recursive case" : "");
+ if (!throwing_OutOfMemoryError_) {
+ throwing_OutOfMemoryError_ = true;
ThrowNewException("Ljava/lang/OutOfMemoryError;", NULL);
- LOG(ERROR) << "Failed to allocate a " << PrettyDescriptor(c->GetDescriptor()) << " (" << byte_count << " bytes)";
} else {
- UNIMPLEMENTED(FATAL) << "throw one i prepared earlier...";
+ SetException(pre_allocated_OutOfMemoryError_);
}
- throwing_OOME_ = false;
+ throwing_OutOfMemoryError_ = false;
}
class CatchBlockStackVisitor : public Thread::StackVisitor {
@@ -1192,14 +1205,6 @@
CatchBlockStackVisitor catch_finder(exception->GetClass(), long_jump_context);
WalkStackUntilUpCall(&catch_finder, true);
- // Pop any SIRT
- if (catch_finder.native_method_count_ == 1) {
- PopSirt();
- } else {
- // We only expect the stack crawl to have passed 1 native method as it's terminated
- // by an up call
- DCHECK_EQ(catch_finder.native_method_count_, 0u);
- }
long_jump_context->SetSP(reinterpret_cast<intptr_t>(catch_finder.handler_frame_.GetSP()));
long_jump_context->SetPC(catch_finder.handler_pc_);
long_jump_context->DoLongJump();
@@ -1302,6 +1307,9 @@
if (peer_ != NULL) {
visitor(peer_, arg);
}
+ if (pre_allocated_OutOfMemoryError_ != NULL) {
+ visitor(pre_allocated_OutOfMemoryError_, arg);
+ }
jni_env_->locals.VisitRoots(visitor, arg);
jni_env_->monitors.VisitRoots(visitor, arg);
diff --git a/src/thread.h b/src/thread.h
index 0d0f107..52e01f2 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -313,9 +313,6 @@
void SirtVisitRoots(Heap::RootVisitor* visitor, void* arg);
- // Pop the top SIRT
- void PopSirt();
-
// Convert a jobject into a Object*
Object* DecodeJObject(jobject obj);
@@ -574,7 +571,9 @@
Context* long_jump_context_;
// A boolean telling us whether we're recursively throwing OOME.
- uint32_t throwing_OOME_;
+ uint32_t throwing_OutOfMemoryError_;
+
+ Throwable* pre_allocated_OutOfMemoryError_;
// TLS key used to retrieve the VM thread object.
static pthread_key_t pthread_key_self_;