From a61894d88fabe45677f491c9f6bde30059a49026 Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Thu, 23 Apr 2015 16:32:54 -0700 Subject: Fix reflection handling and test flakiness Fixed reflection invoke to handle exceptions which occur from FindClass or NewObject by throwing these instead of the expected InvocationTargetException. Added test case to 080 for this reflection invoke. Fixed println throwing OOM in 104-growth-limit. Change-Id: I65766e7c3478e299da06fdc3a521fe3f3e8fdba9 --- runtime/reflection.cc | 12 ++++++++- runtime/thread.cc | 11 ++++++--- runtime/thread.h | 1 + runtime/well_known_classes.cc | 2 ++ runtime/well_known_classes.h | 1 + test/080-oom-throw/expected.txt | 1 + test/080-oom-throw/src/Main.java | 49 +++++++++++++++++++++++++++++++++++++ test/104-growth-limit/src/Main.java | 6 +++-- test/etc/run-test-jar | 3 ++- 9 files changed, 79 insertions(+), 7 deletions(-) diff --git a/runtime/reflection.cc b/runtime/reflection.cc index e54673831..3099094ed 100644 --- a/runtime/reflection.cc +++ b/runtime/reflection.cc @@ -615,11 +615,21 @@ jobject InvokeMethod(const ScopedObjectAccessAlreadyRunnable& soa, jobject javaM // Wrap any exception with "Ljava/lang/reflect/InvocationTargetException;" and return early. if (soa.Self()->IsExceptionPending()) { + // If we get another exception when we are trying to wrap, then just use that instead. jthrowable th = soa.Env()->ExceptionOccurred(); - soa.Env()->ExceptionClear(); + soa.Self()->ClearException(); jclass exception_class = soa.Env()->FindClass("java/lang/reflect/InvocationTargetException"); + if (exception_class == nullptr) { + soa.Self()->AssertPendingOOMException(); + return nullptr; + } jmethodID mid = soa.Env()->GetMethodID(exception_class, "", "(Ljava/lang/Throwable;)V"); + CHECK(mid != nullptr); jobject exception_instance = soa.Env()->NewObject(exception_class, mid, th); + if (exception_instance == nullptr) { + soa.Self()->AssertPendingOOMException(); + return nullptr; + } soa.Env()->Throw(reinterpret_cast(exception_instance)); return nullptr; } diff --git a/runtime/thread.cc b/runtime/thread.cc index fa65bceff..b27ad4ae3 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -1171,9 +1171,14 @@ bool Thread::IsStillStarting() const { } void Thread::AssertPendingException() const { - if (UNLIKELY(!IsExceptionPending())) { - LOG(FATAL) << "Pending exception expected."; - } + CHECK(IsExceptionPending()) << "Pending exception expected."; +} + +void Thread::AssertPendingOOMException() const { + AssertPendingException(); + auto* e = GetException(); + CHECK_EQ(e->GetClass(), DecodeJObject(WellKnownClasses::java_lang_OutOfMemoryError)->AsClass()) + << e->Dump(); } void Thread::AssertNoPendingException() const { diff --git a/runtime/thread.h b/runtime/thread.h index dd9e7345d..35b785df6 100644 --- a/runtime/thread.h +++ b/runtime/thread.h @@ -336,6 +336,7 @@ class Thread { } void AssertPendingException() const; + void AssertPendingOOMException() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); void AssertNoPendingException() const; void AssertNoPendingExceptionForNewException(const char* msg) const; diff --git a/runtime/well_known_classes.cc b/runtime/well_known_classes.cc index a803df8b7..a2d042724 100644 --- a/runtime/well_known_classes.cc +++ b/runtime/well_known_classes.cc @@ -39,6 +39,7 @@ jclass WellKnownClasses::java_lang_ClassNotFoundException; jclass WellKnownClasses::java_lang_Daemons; jclass WellKnownClasses::java_lang_Error; jclass WellKnownClasses::java_lang_Object; +jclass WellKnownClasses::java_lang_OutOfMemoryError; jclass WellKnownClasses::java_lang_reflect_AbstractMethod; jclass WellKnownClasses::java_lang_reflect_ArtMethod; jclass WellKnownClasses::java_lang_reflect_Constructor; @@ -176,6 +177,7 @@ void WellKnownClasses::Init(JNIEnv* env) { java_lang_ClassNotFoundException = CacheClass(env, "java/lang/ClassNotFoundException"); java_lang_Daemons = CacheClass(env, "java/lang/Daemons"); java_lang_Object = CacheClass(env, "java/lang/Object"); + java_lang_OutOfMemoryError = CacheClass(env, "java/lang/OutOfMemoryError"); java_lang_Error = CacheClass(env, "java/lang/Error"); java_lang_reflect_AbstractMethod = CacheClass(env, "java/lang/reflect/AbstractMethod"); java_lang_reflect_ArtMethod = CacheClass(env, "java/lang/reflect/ArtMethod"); diff --git a/runtime/well_known_classes.h b/runtime/well_known_classes.h index 2df1c0e6b..cef9d5552 100644 --- a/runtime/well_known_classes.h +++ b/runtime/well_known_classes.h @@ -50,6 +50,7 @@ struct WellKnownClasses { static jclass java_lang_Daemons; static jclass java_lang_Error; static jclass java_lang_Object; + static jclass java_lang_OutOfMemoryError; static jclass java_lang_reflect_AbstractMethod; static jclass java_lang_reflect_ArtMethod; static jclass java_lang_reflect_Constructor; diff --git a/test/080-oom-throw/expected.txt b/test/080-oom-throw/expected.txt index 73cc0d8b3..904393bc3 100644 --- a/test/080-oom-throw/expected.txt +++ b/test/080-oom-throw/expected.txt @@ -1,2 +1,3 @@ +Test reflection correctly threw NEW_ARRAY correctly threw OOME NEW_INSTANCE correctly threw OOME diff --git a/test/080-oom-throw/src/Main.java b/test/080-oom-throw/src/Main.java index c93f8bbc5..f007b2535 100644 --- a/test/080-oom-throw/src/Main.java +++ b/test/080-oom-throw/src/Main.java @@ -14,6 +14,9 @@ * limitations under the License. */ +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + public class Main { static class ArrayMemEater { static boolean sawOome; @@ -68,6 +71,10 @@ public class Main { } public static void main(String[] args) { + if (triggerReflectionOOM()) { + System.out.println("Test reflection correctly threw"); + } + if (triggerArrayOOM()) { System.out.println("NEW_ARRAY correctly threw OOME"); } @@ -76,4 +83,46 @@ public class Main { System.out.println("NEW_INSTANCE correctly threw OOME"); } } + + static Object[] holder; + + public static void blowup() throws Exception { + int size = 32 * 1024 * 1024; + for (int i = 0; i < holder.length; ) { + try { + holder[i] = new char[size]; + i++; + } catch (OutOfMemoryError oome) { + size = size / 2; + if (size == 0) { + break; + } + } + } + holder[0] = new char[100000]; + } + + static boolean triggerReflectionOOM() { + try { + Class c = Main.class; + Method m = c.getMethod("blowup", (Class[]) null); + holder = new Object[1000000]; + m.invoke(null); + holder = null; + System.out.println("Didn't throw from blowup"); + } catch (OutOfMemoryError e) { + holder = null; + } catch (InvocationTargetException e) { + holder = null; + if (!(e.getCause() instanceof OutOfMemoryError)) { + System.out.println("InvocationTargetException cause not OOME " + e.getCause()); + return false; + } + } catch (Exception e) { + holder = null; + System.out.println("Unexpected exception " + e); + return false; + } + return true; + } } diff --git a/test/104-growth-limit/src/Main.java b/test/104-growth-limit/src/Main.java index d666377b5..d31cbf1fe 100644 --- a/test/104-growth-limit/src/Main.java +++ b/test/104-growth-limit/src/Main.java @@ -29,26 +29,28 @@ public class Main { final Method get_runtime = vm_runtime.getDeclaredMethod("getRuntime"); final Object runtime = get_runtime.invoke(null); final Method clear_growth_limit = vm_runtime.getDeclaredMethod("clearGrowthLimit"); + List l = new ArrayList(); try { - List l = new ArrayList(); while (true) { // Allocate a MB at a time l.add(new byte[1048576]); alloc1++; } } catch (OutOfMemoryError e) { + l = null; } // Expand the heap to the maximum size. clear_growth_limit.invoke(runtime); int alloc2 = 1; + l = new ArrayList(); try { - List l = new ArrayList(); while (true) { // Allocate a MB at a time l.add(new byte[1048576]); alloc2++; } } catch (OutOfMemoryError e2) { + l = null; if (alloc1 > alloc2) { System.out.println("ERROR: Allocated less memory after growth" + "limit cleared (" + alloc1 + " MBs > " + alloc2 + " MBs"); diff --git a/test/etc/run-test-jar b/test/etc/run-test-jar index 414e4df9f..8dd757397 100755 --- a/test/etc/run-test-jar +++ b/test/etc/run-test-jar @@ -225,7 +225,8 @@ if [ "$DEBUGGER" = "y" ]; then fi if [ "$USE_JVM" = "y" ]; then - ${JAVA} ${DEBUGGER_OPTS} ${JVM_VERIFY_ARG} -classpath classes $MAIN "$@" + # Xmx is necessary since we don't pass down the ART flags to JVM. + ${JAVA} ${DEBUGGER_OPTS} ${JVM_VERIFY_ARG} -Xmx256m -classpath classes $MAIN "$@" exit fi -- cgit v1.2.3