8353117: Crash: assert(id >= ThreadIdentifier::initial() && id < ThreadIdentifier::current()) failed: must be reasonable)

Reviewed-by: dholmes, fbredberg

Author: pchilano

src/hotspot/share/prims/jni.cpp

@@ -3804,6 +3804,10 @@ static jint attach_current_thread(JavaVM *vm, void **penv, void *_args, bool dae
 
   thread->cache_global_variables();
 
+  // Set the _monitor_owner_id to the next thread_id temporarily while initialization runs.
+  // Do it now before we make this thread visible in Threads::add().
+  thread->set_monitor_owner_id(ThreadIdentifier::next());
+
   // This thread will not do a safepoint check, since it has
   // not been added to the Thread list yet.
   { MutexLocker ml(Threads_lock);
@@ -3843,6 +3847,9 @@ static jint attach_current_thread(JavaVM *vm, void **penv, void *_args, bool dae
     return JNI_ERR;
   }
 
+  // Update the _monitor_owner_id with the tid value.
+  thread->set_monitor_owner_id(java_lang_Thread::thread_id(thread->threadObj()));
+
   // Want this inside 'attaching via jni'.
   JFR_ONLY(Jfr::on_thread_start(thread);)
 

src/hotspot/share/runtime/javaThread.cpp

@@ -236,8 +236,6 @@ void JavaThread::allocate_threadObj(Handle thread_group, const char* thread_name
   // constructor calls Thread.current(), which must be set here.
   java_lang_Thread::set_thread(thread_oop(), this);
   set_threadOopHandles(thread_oop());
-  // Set the _monitor_owner_id to the next thread_id temporarily while initialization runs.
-  set_monitor_owner_id(ThreadIdentifier::next());
 
   JavaValue result(T_VOID);
   if (thread_name != nullptr) {
@@ -263,8 +261,6 @@ void JavaThread::allocate_threadObj(Handle thread_group, const char* thread_name
                             Handle(),
                             CHECK);
   }
-  // Update the _monitor_owner_id with the tid value.
-  set_monitor_owner_id(java_lang_Thread::thread_id(thread_oop()));
 
   os::set_priority(this, NormPriority);
 

src/hotspot/share/runtime/javaThread.hpp

@@ -170,10 +170,14 @@ class JavaThread: public Thread {
 
  public:
   void set_monitor_owner_id(int64_t id) {
-    assert(id >= ThreadIdentifier::initial() && id < ThreadIdentifier::current(), "");
+    ThreadIdentifier::verify_id(id);
     _monitor_owner_id = id;
   }
-  int64_t monitor_owner_id() const { return _monitor_owner_id; }
+  int64_t monitor_owner_id() const {
+    int64_t id = _monitor_owner_id;
+    ThreadIdentifier::verify_id(id);
+    return id;
+  }
 
   // For tracking the heavyweight monitor the thread is pending on.
   ObjectMonitor* current_pending_monitor() {

src/hotspot/share/runtime/objectMonitor.inline.hpp

@@ -41,14 +41,12 @@
 #include "utilities/globalDefinitions.hpp"
 
 inline int64_t ObjectMonitor::owner_id_from(JavaThread* thread) {
-  int64_t id = thread->monitor_owner_id();
-  assert(id >= ThreadIdentifier::initial() && id < ThreadIdentifier::current(), "must be reasonable");
-  return id;
+  return thread->monitor_owner_id();
 }
 
 inline int64_t ObjectMonitor::owner_id_from(oop vthread) {
   int64_t id = java_lang_Thread::thread_id(vthread);
-  assert(id >= ThreadIdentifier::initial() && id < ThreadIdentifier::current(), "must be reasonable");
+  ThreadIdentifier::verify_id(id);
   return id;
 }
 

src/hotspot/share/runtime/threadIdentifier.cpp

@@ -48,3 +48,10 @@ int64_t ThreadIdentifier::next() {
   } while (Atomic::cmpxchg(&next_thread_id, next_tid, next_tid + 1) != next_tid);
   return next_tid;
 }
+
+#ifdef ASSERT
+void ThreadIdentifier::verify_id(int64_t id) {
+  int64_t current_id = current();
+  assert(id >= initial() && id < current_id, "invalid id, " INT64_FORMAT " and current is " INT64_FORMAT, id, current_id);
+}
+#endif

src/hotspot/share/runtime/threadIdentifier.hpp

@@ -39,6 +39,7 @@ class ThreadIdentifier : AllStatic {
   static int64_t current();
   static int64_t unsafe_offset();
   static int64_t initial();
+  static void verify_id(int64_t id) NOT_DEBUG_RETURN;
 };
 
 #endif // SHARE_RUNTIME_THREADIDENTIFIER_HPP

src/hotspot/share/runtime/threads.cpp

@@ -1031,6 +1031,7 @@ jboolean Threads::is_supported_jni_version(jint version) {
 void Threads::add(JavaThread* p, bool force_daemon) {
   // The threads lock must be owned at this point
   assert(Threads_lock->owned_by_self(), "must have threads lock");
+  assert(p->monitor_owner_id() != 0, "should be set");
 
   BarrierSet::barrier_set()->on_thread_attach(p);