commit 4d5714378c3433ba7e351cbf6a6713fb49c31784
author Brian Carlstrom <bdc@google.com> Wed May 16 00:21:41 2012 -0700
committer Brian Carlstrom <bdc@google.com> Wed May 16 12:10:24 2012 -0700
tree 61cff5862955102a5d6aaa20ea9b7dee9cf1388a
parent 0aac50652ef961ba3aafa44d43871190d23be1ce

DetachCurrentThread should return JNI_ERR on already detached thread

(cherry picked from commit bea414643f7dc41862a6ee46e57d3c95b6aa51ba)

Change-Id: I9bc52ff81638319528f911cdac4852a945a2d885

src/jni_internal.cc

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);

src/jni_internal_test.cc

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

src/runtime.cc

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();