JNI: Fix failure to unlock for pending exception.
And add a regression test. This was broken by
https://android-review.googlesource.com/1898923
but it was not caught by any direct tests.
Test: Additional test in JniCompilerTest.
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Bug: 172332525
Bug: 208831945
Change-Id: I41d4999bbf43f8e58c88b87da47be6f7409d9ce1
diff --git a/compiler/jni/jni_compiler_test.cc b/compiler/jni/jni_compiler_test.cc
index 8de5c9c..abff4b6 100644
--- a/compiler/jni/jni_compiler_test.cc
+++ b/compiler/jni/jni_compiler_test.cc
@@ -1268,6 +1268,12 @@
env->ThrowNew(c, "hello");
}
+void Java_MyClassNatives_synchronizedThrowException(JNIEnv* env, jobject) {
+ JniCompilerTest::AssertCallerObjectLocked(env);
+ jclass c = env->FindClass("java/lang/RuntimeException");
+ env->ThrowNew(c, "hello");
+}
+
void JniCompilerTest::ExceptionHandlingImpl() {
{
ASSERT_FALSE(runtime_->IsStarted());
@@ -1277,7 +1283,9 @@
// all compilation needs to happen before Runtime::Start
CompileForTestWithCurrentJni(class_loader_, false, "foo", "()V");
CompileForTestWithCurrentJni(class_loader_, false, "throwException", "()V");
- CompileForTestWithCurrentJni(class_loader_, false, "foo", "()V");
+ if (gCurrentJni == enum_cast<uint32_t>(JniKind::kNormal)) {
+ CompileForTestWithCurrentJni(class_loader_, false, "synchronizedThrowException", "()V");
+ }
}
// Start runtime to avoid re-initialization in SetupForTest.
Thread::Current()->TransitionFromSuspendedToRunnable();
@@ -1298,17 +1306,39 @@
CURRENT_JNI_WRAPPER(Java_MyClassNatives_throwException));
// Call Java_MyClassNatives_throwException (JNI method that throws exception)
env_->CallNonvirtualVoidMethod(jobj_, jklass_, jmethod_);
- EXPECT_EQ(1, gJava_MyClassNatives_foo_calls[gCurrentJni]);
EXPECT_TRUE(env_->ExceptionCheck() == JNI_TRUE);
ScopedLocalRef<jthrowable> exception(env_, env_->ExceptionOccurred());
env_->ExceptionClear();
EXPECT_TRUE(env_->IsInstanceOf(exception.get(), jlre.get()));
// Check a single call of a JNI method is ok
+ EXPECT_EQ(1, gJava_MyClassNatives_foo_calls[gCurrentJni]);
SetUpForTest(false, "foo", "()V", reinterpret_cast<void*>(&Java_MyClassNatives_foo));
env_->CallNonvirtualVoidMethod(jobj_, jklass_, jmethod_);
EXPECT_EQ(2, gJava_MyClassNatives_foo_calls[gCurrentJni]);
+ if (gCurrentJni == enum_cast<uint32_t>(JniKind::kNormal)) {
+ SetUpForTest(false, "synchronizedThrowException", "()V",
+ CURRENT_JNI_WRAPPER(Java_MyClassNatives_synchronizedThrowException));
+ LockWord lock_word = GetLockWord(jobj_);
+ ASSERT_EQ(lock_word.GetState(), LockWord::kUnlocked);
+ // Call Java_MyClassNatives_synchronizedThrowException (synchronized JNI method
+ // that throws exception) to check that we correctly unlock the object.
+ env_->CallNonvirtualVoidMethod(jobj_, jklass_, jmethod_);
+ EXPECT_TRUE(env_->ExceptionCheck() == JNI_TRUE);
+ ScopedLocalRef<jthrowable> exception2(env_, env_->ExceptionOccurred());
+ env_->ExceptionClear();
+ EXPECT_TRUE(env_->IsInstanceOf(exception2.get(), jlre.get()));
+ lock_word = GetLockWord(jobj_);
+ EXPECT_EQ(lock_word.GetState(), LockWord::kUnlocked);
+
+ // Check a single call of a JNI method is ok
+ EXPECT_EQ(2, gJava_MyClassNatives_foo_calls[gCurrentJni]);
+ SetUpForTest(false, "foo", "()V", reinterpret_cast<void*>(&Java_MyClassNatives_foo));
+ env_->CallNonvirtualVoidMethod(jobj_, jklass_, jmethod_);
+ EXPECT_EQ(3, gJava_MyClassNatives_foo_calls[gCurrentJni]);
+ }
+
gJava_MyClassNatives_foo_calls[gCurrentJni] = 0;
}
diff --git a/compiler/jni/quick/jni_compiler.cc b/compiler/jni/quick/jni_compiler.cc
index bc1c842..8bb6cc5 100644
--- a/compiler/jni/quick/jni_compiler.cc
+++ b/compiler/jni/quick/jni_compiler.cc
@@ -524,22 +524,8 @@
current_frame_size -= current_out_arg_size;
}
- // 7.2. Process pending exceptions from JNI call or monitor exit.
- // @CriticalNative methods do not need exception poll in the stub.
- // @FastNative methods with reference return emit the exception poll earlier.
- if (LIKELY(!is_critical_native) && (LIKELY(!is_fast_native) || !reference_return)) {
- __ ExceptionPoll(exception_slow_path.get());
- }
-
- // 7.3. For @FastNative, we never transitioned out of runnable, so there is no transition back.
- // Perform a suspend check if there is a flag raised, unless we have done that above
- // for reference return.
- if (UNLIKELY(is_fast_native) && !reference_return) {
- __ SuspendCheck(suspend_check_slow_path.get());
- __ Bind(suspend_check_resume.get());
- }
-
- // 7.4 Unlock the synchronization object for synchronized methods.
+ // 7.2 Unlock the synchronization object for synchronized methods.
+ // Do this before exception poll to avoid extra unlocking in the exception slow path.
if (UNLIKELY(is_synchronized)) {
ManagedRegister to_lock = main_jni_conv->LockingArgumentRegister();
mr_conv->ResetIterator(FrameOffset(current_frame_size));
@@ -558,6 +544,21 @@
__ CallFromThread(QUICK_ENTRYPOINT_OFFSET(kPointerSize, pJniUnlockObject));
}
+ // 7.3. Process pending exceptions from JNI call or monitor exit.
+ // @CriticalNative methods do not need exception poll in the stub.
+ // @FastNative methods with reference return emit the exception poll earlier.
+ if (LIKELY(!is_critical_native) && (LIKELY(!is_fast_native) || !reference_return)) {
+ __ ExceptionPoll(exception_slow_path.get());
+ }
+
+ // 7.4. For @FastNative, we never transitioned out of runnable, so there is no transition back.
+ // Perform a suspend check if there is a flag raised, unless we have done that above
+ // for reference return.
+ if (UNLIKELY(is_fast_native) && !reference_return) {
+ __ SuspendCheck(suspend_check_slow_path.get());
+ __ Bind(suspend_check_resume.get());
+ }
+
// 7.5. Remove activation - need to restore callee save registers since the GC
// may have changed them.
DCHECK_EQ(jni_asm->cfi().GetCurrentCFAOffset(), static_cast<int>(current_frame_size));
diff --git a/test/MyClassNatives/MyClassNatives.java b/test/MyClassNatives/MyClassNatives.java
index 463fe3f..d3bf5ff 100644
--- a/test/MyClassNatives/MyClassNatives.java
+++ b/test/MyClassNatives/MyClassNatives.java
@@ -27,6 +27,8 @@
// Normal native
native void throwException();
// Normal native
+ synchronized native void synchronizedThrowException();
+ // Normal native
native void foo();
// Normal native
native int bar(int count);