8288139: JavaThread touches oop after GC barrier is detached

Reviewed-by: pchilanomate, dholmes, rehn, eosterlund

src/hotspot/share/runtime/sharedRuntime.cpp

@@ -996,11 +996,12 @@ JRT_ENTRY_NO_ASYNC(void, SharedRuntime::register_finalizer(JavaThread* current,
 JRT_END
 
 jlong SharedRuntime::get_java_tid(Thread* thread) {
-  if (thread != NULL) {
+  if (thread != NULL && thread->is_Java_thread()) {
-    if (thread->is_Java_thread()) {
+    Thread* current = Thread::current();
-      oop obj = JavaThread::cast(thread)->threadObj();
+    guarantee(current != thread || JavaThread::cast(thread)->is_oop_safe(),
-      return (obj == NULL) ? 0 : java_lang_Thread::thread_id(obj);
+              "current cannot touch oops after its GC barrier is detached.");
-    }
+    oop obj = JavaThread::cast(thread)->threadObj();
+    return (obj == NULL) ? 0 : java_lang_Thread::thread_id(obj);
   }
   return 0;
 }

src/hotspot/share/runtime/thread.cpp

@@ -123,6 +123,7 @@
 #include "services/attachListener.hpp"
 #include "services/management.hpp"
 #include "services/memTracker.hpp"
+#include "services/threadIdTable.hpp"
 #include "services/threadService.hpp"
 #include "utilities/align.hpp"
 #include "utilities/copy.hpp"
@@ -3594,6 +3595,13 @@ void Threads::remove(JavaThread* p, bool is_daemon) {
   // that we do not remove thread without safepoint code notice
   { MonitorLocker ml(Threads_lock);
 
+    if (ThreadIdTable::is_initialized()) {
+      // This cleanup must be done before the current thread's GC barrier
+      // is detached since we need to touch the threadObj oop.
+      jlong tid = SharedRuntime::get_java_tid(p);
+      ThreadIdTable::remove_thread(tid);
+    }
+
     // BarrierSet state must be destroyed after the last thread transition
     // before the thread terminates. Thread transitions result in calls to
     // StackWatermarkSet::on_safepoint(), which performs GC processing,

src/hotspot/share/runtime/threadSMR.cpp

@@ -716,7 +716,7 @@ JavaThread* ThreadsList::find_JavaThread_from_java_tid(jlong java_tid) const {
       if (tobj != NULL && java_tid == java_lang_Thread::thread_id(tobj)) {
         MutexLocker ml(Threads_lock);
         // Must be inside the lock to ensure that we don't add a thread to the table
-        // that has just passed the removal point in ThreadsSMRSupport::remove_thread()
+        // that has just passed the removal point in Threads::remove().
         if (!thread->is_exiting()) {
           ThreadIdTable::add_thread(java_tid, thread);
           return thread;
@@ -1000,12 +1000,6 @@ void ThreadsSMRSupport::release_stable_list_wake_up(bool is_nested) {
 }
 
 void ThreadsSMRSupport::remove_thread(JavaThread *thread) {
-
-  if (ThreadIdTable::is_initialized()) {
-    jlong tid = SharedRuntime::get_java_tid(thread);
-    ThreadIdTable::remove_thread(tid);
-  }
-
   ThreadsList *new_list = ThreadsList::remove_thread(ThreadsSMRSupport::get_java_thread_list(), thread);
   if (EnableThreadSMRStatistics) {
     ThreadsSMRSupport::inc_java_thread_list_alloc_cnt();

src/hotspot/share/services/threadIdTable.cpp

@@ -108,7 +108,7 @@ void ThreadIdTable::lazy_initialize(const ThreadsList *threads) {
         MutexLocker ml(Threads_lock);
         if (!thread->is_exiting()) {
           // Must be inside the lock to ensure that we don't add a thread to the table
-          // that has just passed the removal point in ThreadsSMRSupport::remove_thread()
+          // that has just passed the removal point in Threads::remove().
           add_thread(java_tid, thread);
         }
       }