DetachCurrentThread should return JNI_ERR on already detached thread
(cherry picked from commit bea414643f7dc41862a6ee46e57d3c95b6aa51ba)
Change-Id: I9bc52ff81638319528f911cdac4852a945a2d885
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index a20bcc9..61f7c3c 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -2724,7 +2724,7 @@
}
static jint DetachCurrentThread(JavaVM* vm) {
- if (vm == NULL) {
+ if (vm == NULL || Thread::Current() == NULL) {
return JNI_ERR;
}
JavaVMExt* raw_vm = reinterpret_cast<JavaVMExt*>(vm);
diff --git a/src/jni_internal_test.cc b/src/jni_internal_test.cc
index b9bb257..b9366c1 100644
--- a/src/jni_internal_test.cc
+++ b/src/jni_internal_test.cc
@@ -35,7 +35,7 @@
// Turn on -verbose:jni for the JNI tests.
gLogVerbosity.jni = true;
- env_ = Thread::Current()->GetJniEnv();
+ vm_->AttachCurrentThread(&env_, NULL);
ScopedLocalRef<jclass> aioobe(env_, env_->FindClass("java/lang/ArrayIndexOutOfBoundsException"));
CHECK(aioobe.get() != NULL);
@@ -50,10 +50,23 @@
sioobe_ = reinterpret_cast<jclass>(env_->NewGlobalRef(sioobe.get()));
}
+ void CleanUpJniEnv() {
+ if (aioobe_ != NULL) {
+ env_->DeleteGlobalRef(aioobe_);
+ aioobe_ = NULL;
+ }
+ if (ase_ != NULL) {
+ env_->DeleteGlobalRef(ase_);
+ ase_ = NULL;
+ }
+ if (sioobe_ != NULL) {
+ env_->DeleteGlobalRef(sioobe_);
+ sioobe_ = NULL;
+ }
+ }
+
virtual void TearDown() {
- env_->DeleteGlobalRef(aioobe_);
- env_->DeleteGlobalRef(ase_);
- env_->DeleteGlobalRef(sioobe_);
+ CleanUpJniEnv();
CommonTest::TearDown();
}
@@ -497,7 +510,7 @@
}
JavaVMExt* vm_;
- JNIEnvExt* env_;
+ JNIEnv* env_;
jclass aioobe_;
jclass ase_;
jclass sioobe_;
@@ -1248,7 +1261,8 @@
// gets a new local reference...
EXPECT_EQ(JNIInvalidRefType, env_->GetObjectRefType(inner2));
// ...but the survivor should be in the local reference table.
- EXPECT_TRUE(env_->locals.ContainsDirectPointer(inner2_direct_pointer));
+ JNIEnvExt* env = reinterpret_cast<JNIEnvExt*>(env_);
+ EXPECT_TRUE(env->locals.ContainsDirectPointer(inner2_direct_pointer));
env_->PopLocalFrame(NULL);
}
@@ -1563,4 +1577,14 @@
}
}
+TEST_F(JniInternalTest, DetachCurrentThread) {
+ CleanUpJniEnv(); // cleanup now so TearDown won't have junk from wrong JNIEnv
+ jint ok = vm_->DetachCurrentThread();
+ EXPECT_EQ(JNI_OK, ok);
+
+ jint err = vm_->DetachCurrentThread();
+ EXPECT_EQ(JNI_ERR, err);
+ vm_->AttachCurrentThread(&env_, NULL); // need attached thread for CommonTest::TearDown
+}
+
} // namespace art
diff --git a/src/runtime.cc b/src/runtime.cc
index aabd86f..c452771 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -836,7 +836,11 @@
}
void Runtime::DetachCurrentThread() {
- if (Thread::Current()->GetTopOfStack().GetSP() != NULL) {
+ Thread* self = Thread::Current();
+ if (self == NULL) {
+ LOG(FATAL) << "attempting to detach thread that is not attached";
+ }
+ if (self->GetTopOfStack().GetSP() != NULL) {
LOG(FATAL) << *Thread::Current() << " attempting to detach while still running code";
}
thread_list_->Unregister();