8311040: JFR: RecordedThread::getOSThreadId() should return -1 if thread is virtual

src/hotspot/share/jfr/recorder/checkpoint/types/jfrThreadState.cpp

@@ -108,22 +108,20 @@ traceid JfrThreadId::jfr_id(const Thread* t, traceid tid) {
 }
 
 // caller needs ResourceMark
-const char* get_java_thread_name(const JavaThread* jt, int& length, oop vthread) {
+static const char* get_java_thread_name(const JavaThread* jt, int& length, oop vthread) {
   assert(jt != nullptr, "invariant");
-  const char* name_str = "<no-name - thread name unresolved>";
-  oop thread_obj = vthread != nullptr ? vthread : jt->threadObj();
-  if (thread_obj == nullptr) {
-    if (jt->is_attaching_via_jni()) {
-      name_str = "<no-name - thread is attaching>";
-    }
+  oop thread_obj;
+  if (vthread != nullptr) {
+    thread_obj = vthread;
   } else {
-    const oop name = java_lang_Thread::name(thread_obj);
-    if (name != nullptr) {
-      name_str = java_lang_String::as_utf8_string(name, length);
+    thread_obj = jt->threadObj();
+    if (thread_obj == nullptr) {
+      return nullptr;
     }
   }
-  assert(name_str != nullptr, "unexpected null thread name");
-  return name_str;
+  assert(thread_obj != nullptr, "invariant");
+  const oop name = java_lang_Thread::name(thread_obj);
+  return name != nullptr ? java_lang_String::as_utf8_string(name, length) : nullptr;
 }
 
 const char* JfrThreadName::name(const Thread* t, int& length, oop vthread) {

src/hotspot/share/jfr/recorder/checkpoint/types/jfrType.cpp

@@ -269,40 +269,48 @@ void ThreadStateConstant::serialize(JfrCheckpointWriter& writer) {
   JfrThreadState::serialize(writer);
 }
 
-void JfrThreadConstant::write_name(JfrCheckpointWriter& writer, const char* name, int length) {
-  if (length == 0) {
+void JfrThreadConstant::write_name(JfrCheckpointWriter& writer) {
+  if (_length == 0) {
     writer.write_empty_string();
     return;
   }
-  writer.write(name);
+  writer.write(_name);
+}
+
+void JfrThreadConstant::write_os_name(JfrCheckpointWriter& writer, bool is_vthread) {
+  if (is_vthread) {
+    // Write the null string categorically as the os name for virtual threads.
+    writer.write((const char*)nullptr);
+    return;
+  }
+  write_name(writer);
 }
 
 void JfrThreadConstant::serialize(JfrCheckpointWriter& writer) {
   assert(_thread != nullptr, "invariant");
-  const bool vthread = _vthread != nullptr;
+  const bool is_vthread = _vthread != nullptr;
   writer.write_key(JfrThreadId::jfr_id(_thread, _tid));
-  int length = -1;
-  const char* const name = JfrThreadName::name(_thread, length, _vthread);
-  write_name(writer, name, length);
-  writer.write(_vthread != nullptr ? static_cast<traceid>(0) : JfrThreadId::os_id(_thread));
+  _name = JfrThreadName::name(_thread, _length, _vthread);
+  write_os_name(writer, is_vthread);
+  writer.write(is_vthread ? static_cast<traceid>(0) : JfrThreadId::os_id(_thread));
   if (!_thread->is_Java_thread()) {
-    write_name(writer, nullptr, 0); // java name
+    writer.write((const char*)nullptr); // java name
     writer.write<traceid>(0); // java thread id
     writer.write<traceid>(0); // java thread group
     writer.write<bool>(false); // isVirtual
-  } else {
-    write_name(writer, name, length);
-    writer.write(JfrThreadId::jfr_id(_thread, _tid));
-    // java thread group - VirtualThread threadgroup reserved id 1
-    const traceid thread_group_id = vthread ? 1 :
-      JfrThreadGroup::thread_group_id(JavaThread::cast(_thread), Thread::current());
-    writer.write(thread_group_id);
-    writer.write<bool>(vthread); // isVirtual
-    if (!vthread) {
-      JfrThreadGroup::serialize(&writer, thread_group_id);
-    }
-    // VirtualThread threadgroup already serialized invariant.
+    return;
+  }
+  write_name(writer);
+  writer.write(JfrThreadId::jfr_id(_thread, _tid));
+  // java thread group - VirtualThread threadgroup reserved id 1
+  const traceid thread_group_id = is_vthread ? 1 :
+    JfrThreadGroup::thread_group_id(JavaThread::cast(_thread), Thread::current());
+  writer.write(thread_group_id);
+  writer.write<bool>(is_vthread); // isVirtual
+  if (!is_vthread) {
+    JfrThreadGroup::serialize(&writer, thread_group_id);
   }
+  // VirtualThread threadgroup already serialized invariant.
 }
 
 void BytecodeConstant::serialize(JfrCheckpointWriter& writer) {

src/hotspot/share/jfr/recorder/checkpoint/types/jfrType.hpp

@@ -107,9 +107,13 @@ class JfrThreadConstant : public JfrSerializer {
   Thread* _thread;
   traceid _tid;
   oop _vthread;
-  void write_name(JfrCheckpointWriter& writer, const char* name, int length);
+  const char* _name;
+  int _length;
+  void write_name(JfrCheckpointWriter& writer);
+  void write_os_name(JfrCheckpointWriter& writer, bool is_vthread);
  public:
-  JfrThreadConstant(Thread* t, traceid tid, oop vthread = nullptr) : _thread(t), _tid(tid), _vthread(vthread) {}
+  JfrThreadConstant(Thread* t, traceid tid, oop vthread = nullptr) :
+    _thread(t), _tid(tid), _vthread(vthread), _name(nullptr), _length(-1) {}
   void serialize(JfrCheckpointWriter& writer);
 };
 

src/jdk.jfr/share/classes/jdk/jfr/consumer/RecordedThread.java

@@ -56,6 +56,9 @@ public String getOSName() {
      * @return the OS thread ID, or {@code -1} if doesn't exist
      */
     public long getOSThreadId() {
+        if (isVirtual()) {
+            return -1L;
+        }
         Long l = getTyped("osThreadId", Long.class, -1L);
         return l.longValue();
     }
@@ -90,7 +93,8 @@ public String getJavaName() {
      */
     public long getJavaThreadId() {
         Long l = getTyped("javaThreadId", Long.class, -1L);
-        return l.longValue();
+        long id = l.longValue();
+        return id == 0 ? -1L : id;
     }
 
     /**

test/jdk/jdk/jfr/threading/TestManyVirtualThreads.java

@@ -89,11 +89,11 @@ public static void main(String... args) throws Exception {
                 RecordedThread t = e.getThread();
                 Asserts.assertNotNull(t);
                 Asserts.assertTrue(t.isVirtual());
+                Asserts.assertEquals(t.getOSName(), null);
+                Asserts.assertEquals(t.getOSThreadId(), -1L);
                 Asserts.assertEquals(t.getJavaName(), ""); // vthreads default name is the empty string.
-                Asserts.assertEquals(t.getOSName(), "");
-                Asserts.assertEquals(t.getThreadGroup().getName(), "VirtualThreads");
                 Asserts.assertGreaterThan(t.getJavaThreadId(), 0L);
-                Asserts.assertEquals(t.getOSThreadId(), 0L);
+                Asserts.assertEquals(t.getThreadGroup().getName(), "VirtualThreads");
             }
         }
     }

test/jdk/jdk/jfr/threading/TestNestedVirtualThreads.java

@@ -74,11 +74,11 @@ public static void main(String... args) throws Exception {
             RecordedEvent e = events.getFirst();
             RecordedThread t = e.getThread();
             Asserts.assertTrue(t.isVirtual());
+            Asserts.assertEquals(t.getOSName(), null);
+            Asserts.assertEquals(t.getOSThreadId(), -1L);
             Asserts.assertEquals(t.getJavaName(), ""); // vthreads default name is the empty string.
-            Asserts.assertEquals(t.getOSName(), "");
-            Asserts.assertEquals(t.getThreadGroup().getName(), "VirtualThreads");
             Asserts.assertGreaterThan(t.getJavaThreadId(), 0L);
-            Asserts.assertEquals(t.getOSThreadId(), 0L);
+            Asserts.assertEquals(t.getThreadGroup().getName(), "VirtualThreads");
         }
     }
 }