8296492: Remove ObjectLocker in JVMTI get_subgroups call

Reviewed-by: dholmes, alanb, sspitsyn

src/hotspot/share/classfile/javaClasses.cpp

@@ -1995,10 +1995,6 @@ int java_lang_ThreadGroup::_parent_offset;
 int java_lang_ThreadGroup::_name_offset;
 int java_lang_ThreadGroup::_maxPriority_offset;
 int java_lang_ThreadGroup::_daemon_offset;
-int java_lang_ThreadGroup::_ngroups_offset;
-int java_lang_ThreadGroup::_groups_offset;
-int java_lang_ThreadGroup::_nweaks_offset;
-int java_lang_ThreadGroup::_weaks_offset;
 
 oop  java_lang_ThreadGroup::parent(oop java_thread_group) {
   assert(oopDesc::is_oop(java_thread_group), "thread group must be oop");
@@ -2026,37 +2022,11 @@ bool java_lang_ThreadGroup::is_daemon(oop java_thread_group) {
   return java_thread_group->bool_field(_daemon_offset) != 0;
 }
 
-int java_lang_ThreadGroup::ngroups(oop java_thread_group) {
-  assert(oopDesc::is_oop(java_thread_group), "thread group must be oop");
-  return java_thread_group->int_field(_ngroups_offset);
-}
-
-objArrayOop java_lang_ThreadGroup::groups(oop java_thread_group) {
-  oop groups = java_thread_group->obj_field(_groups_offset);
-  assert(groups == NULL || groups->is_objArray(), "just checking"); // Todo: Add better type checking code
-  return objArrayOop(groups);
-}
-
-int java_lang_ThreadGroup::nweaks(oop java_thread_group) {
-  assert(oopDesc::is_oop(java_thread_group), "thread group must be oop");
-  return java_thread_group->int_field(_nweaks_offset);
-}
-
-objArrayOop java_lang_ThreadGroup::weaks(oop java_thread_group) {
-  oop weaks = java_thread_group->obj_field(_weaks_offset);
-  assert(weaks == NULL || weaks->is_objArray(), "just checking");
-  return objArrayOop(weaks);
-}
-
 #define THREADGROUP_FIELDS_DO(macro) \
   macro(_parent_offset,      k, vmSymbols::parent_name(),      threadgroup_signature,         false); \
   macro(_name_offset,        k, vmSymbols::name_name(),        string_signature,              false); \
   macro(_maxPriority_offset, k, vmSymbols::maxPriority_name(), int_signature,                 false); \
-  macro(_daemon_offset,      k, vmSymbols::daemon_name(),      bool_signature,                false); \
+  macro(_daemon_offset,      k, vmSymbols::daemon_name(),      bool_signature,                false);
-  macro(_ngroups_offset,     k, vmSymbols::ngroups_name(),     int_signature,                 false); \
-  macro(_groups_offset,      k, vmSymbols::groups_name(),      threadgroup_array_signature,   false); \
-  macro(_nweaks_offset,      k, vmSymbols::nweaks_name(),      int_signature,                 false); \
-  macro(_weaks_offset,       k, vmSymbols::weaks_name(),       weakreference_array_signature, false);
 
 void java_lang_ThreadGroup::compute_offsets() {
   assert(_parent_offset == 0, "offsets should be initialized only once");

src/hotspot/share/classfile/javaClasses.hpp

@@ -479,13 +479,7 @@ class java_lang_ThreadGroup : AllStatic {
   static int _maxPriority_offset;
   static int _daemon_offset;
 
-  static int _ngroups_offset;
-  static int _groups_offset;
-  static int _nweaks_offset;
-  static int _weaks_offset;
-
   static void compute_offsets();
-
  public:
   static void serialize_offsets(SerializeClosure* f) NOT_CDS_RETURN;
 
@@ -498,15 +492,6 @@ class java_lang_ThreadGroup : AllStatic {
   // Daemon
   static bool is_daemon(oop java_thread_group);
 
-  // Number of strongly reachable thread groups
-  static int ngroups(oop java_thread_group);
-  // Strongly reachable thread groups
-  static objArrayOop groups(oop java_thread_group);
-  // Number of weakly reachable thread groups
-  static int nweaks(oop java_thread_group);
-  // Weakly reachable thread groups
-  static objArrayOop weaks(oop java_thread_group);
-
   // Debugging
   friend class JavaClasses;
 };

src/hotspot/share/classfile/vmSymbols.hpp

@@ -400,10 +400,6 @@
   template(exit_method_name,                          "exit")                                     \
   template(remove_method_name,                        "remove")                                   \
   template(parent_name,                               "parent")                                   \
-  template(ngroups_name,                              "ngroups")                                  \
-  template(groups_name,                               "groups")                                   \
-  template(nweaks_name,                               "nweaks")                                   \
-  template(weaks_name,                                "weaks")                                    \
   template(maxPriority_name,                          "maxPriority")                              \
   template(shutdown_name,                             "shutdown")                                 \
   template(finalize_method_name,                      "finalize")                                 \
@@ -607,6 +603,7 @@
   template(thread_void_signature,                     "(Ljava/lang/Thread;)V")                                    \
   template(threadgroup_runnable_void_signature,       "(Ljava/lang/ThreadGroup;Ljava/lang/Runnable;)V")           \
   template(threadgroup_string_void_signature,         "(Ljava/lang/ThreadGroup;Ljava/lang/String;)V")             \
+  template(void_threadgroup_array_signature,          "()[Ljava/lang/ThreadGroup;")                               \
   template(string_class_signature,                    "(Ljava/lang/String;)Ljava/lang/Class;")                    \
   template(string_boolean_class_signature,            "(Ljava/lang/String;Z)Ljava/lang/Class;")                   \
   template(object_object_object_signature,            "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;") \

src/hotspot/share/prims/jvmtiEnv.cpp

@@ -1670,7 +1670,7 @@ JvmtiEnv::GetThreadGroupChildren(jthreadGroup group, jint* thread_count_ptr, jth
   NULL_CHECK(group_obj, JVMTI_ERROR_INVALID_THREAD_GROUP);
 
   Handle *thread_objs = NULL;
-  Handle *group_objs  = NULL;
+  objArrayHandle group_objs;
   jint nthreads = 0;
   jint ngroups = 0;
   int hidden_threads = 0;

src/hotspot/share/prims/jvmtiEnvBase.cpp

@@ -26,6 +26,7 @@
 #include "classfile/classLoaderDataGraph.hpp"
 #include "classfile/javaClasses.inline.hpp"
 #include "classfile/moduleEntry.hpp"
+#include "classfile/symbolTable.hpp"
 #include "jvmtifiles/jvmtiEnv.hpp"
 #include "memory/iterator.hpp"
 #include "memory/resourceArea.hpp"
@@ -46,6 +47,7 @@
 #include "runtime/frame.inline.hpp"
 #include "runtime/handles.inline.hpp"
 #include "runtime/interfaceSupport.inline.hpp"
+#include "runtime/javaCalls.hpp"
 #include "runtime/javaThread.inline.hpp"
 #include "runtime/jfieldIDWorkaround.hpp"
 #include "runtime/jniHandles.inline.hpp"
@@ -534,29 +536,34 @@ void JvmtiEnvBase::destroy_jni_reference(JavaThread *thread, jobject jobj) {
 // Threads
 //
 
-jobject *
+jthread *
-JvmtiEnvBase::new_jobjectArray(int length, Handle *handles) {
+JvmtiEnvBase::new_jthreadArray(int length, Handle *handles) {
   if (length == 0) {
     return NULL;
   }
 
-  jobject *objArray = (jobject *) jvmtiMalloc(sizeof(jobject) * length);
+  jthread* objArray = (jthread *) jvmtiMalloc(sizeof(jthread) * length);
   NULL_CHECK(objArray, NULL);
 
-  for (int i=0; i<length; i++) {
+  for (int i = 0; i < length; i++) {
-    objArray[i] = jni_reference(handles[i]);
+    objArray[i] = (jthread)jni_reference(handles[i]);
   }
   return objArray;
 }
 
-jthread *
-JvmtiEnvBase::new_jthreadArray(int length, Handle *handles) {
-  return (jthread *) new_jobjectArray(length,handles);
-}
-
 jthreadGroup *
-JvmtiEnvBase::new_jthreadGroupArray(int length, Handle *handles) {
+JvmtiEnvBase::new_jthreadGroupArray(int length, objArrayHandle groups) {
-  return (jthreadGroup *) new_jobjectArray(length,handles);
+  if (length == 0) {
+    return NULL;
+  }
+
+  jthreadGroup* objArray = (jthreadGroup *) jvmtiMalloc(sizeof(jthreadGroup) * length);
+  NULL_CHECK(objArray, NULL);
+
+  for (int i = 0; i < length; i++) {
+    objArray[i] = (jthreadGroup)JNIHandles::make_local(groups->obj_at(i));
+  }
+  return objArray;
 }
 
 // Return the vframe on the specified thread and depth, NULL if no such frame.
@@ -792,43 +799,33 @@ JvmtiEnvBase::get_live_threads(JavaThread* current_thread, Handle group_hdl, jin
 }
 
 jvmtiError
-JvmtiEnvBase::get_subgroups(JavaThread* current_thread, Handle group_hdl, jint *count_ptr, Handle **group_objs_p) {
+JvmtiEnvBase::get_subgroups(JavaThread* current_thread, Handle group_hdl, jint *count_ptr, objArrayHandle *group_objs_p) {
-  ObjectLocker ol(group_hdl, current_thread);
 
-  int ngroups = java_lang_ThreadGroup::ngroups(group_hdl());
+  // This call collects the strong and weak groups
-  int nweaks = java_lang_ThreadGroup::nweaks(group_hdl());
+  JavaThread* THREAD = current_thread;
-
+  JavaValue result(T_OBJECT);
-  jint count = 0;
+  JavaCalls::call_virtual(&result,
-  Handle *group_objs = NULL;
+                          group_hdl,
-  if (ngroups > 0 || nweaks > 0) {
+                          vmClasses::ThreadGroup_klass(),
-    group_objs = NEW_RESOURCE_ARRAY_RETURN_NULL(Handle, ngroups + nweaks);
+                          SymbolTable::new_permanent_symbol("subgroupsAsArray"),
-    NULL_CHECK(group_objs, JVMTI_ERROR_OUT_OF_MEMORY);
+                          vmSymbols::void_threadgroup_array_signature(),
-
+                          THREAD);
-    if (ngroups > 0) {
+  if (HAS_PENDING_EXCEPTION) {
-      // Strongly reachable subgroups:
+    Symbol* ex_name = PENDING_EXCEPTION->klass()->name();
-      objArrayOop groups = java_lang_ThreadGroup::groups(group_hdl());
+    CLEAR_PENDING_EXCEPTION;
-      for (int j = 0; j < ngroups; j++) {
+    if (ex_name == vmSymbols::java_lang_OutOfMemoryError()) {
-        oop group_obj = groups->obj_at(j);
+      return JVMTI_ERROR_OUT_OF_MEMORY;
-        assert(group_obj != NULL, "group_obj != NULL");
+    } else {
-        group_objs[count++] = Handle(current_thread, group_obj);
+      return JVMTI_ERROR_INTERNAL;
-      }
-    }
-
-    if (nweaks > 0) {
-      // Weakly reachable subgroups:
-      objArrayOop weaks = java_lang_ThreadGroup::weaks(group_hdl());
-      for (int j = 0; j < nweaks; j++) {
-        oop weak_obj = weaks->obj_at(j);
-        assert(weak_obj != NULL, "weak_obj != NULL");
-        oop group_obj = java_lang_ref_Reference::weak_referent(weak_obj);
-        if (group_obj != NULL) {
-          group_objs[count++] = Handle(current_thread, group_obj);
-        }
-      }
     }
   }
-  *group_objs_p = group_objs;
+
-  *count_ptr = count;
+  assert(result.get_type() == T_OBJECT, "just checking");
+  objArrayOop groups = (objArrayOop)result.get_oop();
+
+  *count_ptr = groups == nullptr ? 0 : groups->length();
+  *group_objs_p = objArrayHandle(current_thread, groups);
+
   return JVMTI_ERROR_NONE;
 }
 

src/hotspot/share/prims/jvmtiEnvBase.hpp

@@ -332,9 +332,8 @@ class JvmtiEnvBase : public CHeapObj<mtInternal> {
  protected:
   // helper methods for creating arrays of global JNI Handles from local Handles
   // allocated into environment specific storage
-  jobject * new_jobjectArray(int length, Handle *handles);
   jthread * new_jthreadArray(int length, Handle *handles);
-  jthreadGroup * new_jthreadGroupArray(int length, Handle *handles);
+  jthreadGroup * new_jthreadGroupArray(int length, objArrayHandle groups);
 
   // convert to a jni jclass from a non-null Klass*
   jclass get_jni_class_non_null(Klass* k);
@@ -378,7 +377,7 @@ class JvmtiEnvBase : public CHeapObj<mtInternal> {
   static jvmtiError get_live_threads(JavaThread* current_thread, Handle group_hdl, jint *count_ptr, Handle **thread_objs_p);
 
   // enumerates the subgroups in the given thread group
-  static jvmtiError get_subgroups(JavaThread* current_thread, Handle group_hdl, jint *count_ptr, Handle **group_objs_p);
+  static jvmtiError get_subgroups(JavaThread* current_thread, Handle group_hdl, jint *count_ptr, objArrayHandle *group_objs_p);
 
   // JVMTI API helper functions which are called when target thread is suspended
   // or at safepoint / thread local handshake.

src/java.base/share/classes/java/lang/ThreadGroup.java

@@ -792,6 +792,14 @@ public class ThreadGroup implements Thread.UncaughtExceptionHandler {
         }
     }
 
+    /**
+     * Returns a snapshot of the subgroups as an array, used by JVMTI.
+     */
+    private ThreadGroup[] subgroupsAsArray() {
+        List<ThreadGroup> groups = synchronizedSubgroups();
+        return groups.toArray(new ThreadGroup[0]);
+    }
+
     /**
      * Returns a snapshot of the subgroups.
      */