7196045: Possible JVM deadlock in ThreadTimesClosure when using HotspotInternal non-public API.
authorkevinw
Wed, 19 Sep 2012 15:24:32 +0100
changeset 13870 dd2c6a5a4722
parent 13862 bf7fe634e5d7
child 13871 6f15b84496b0
7196045: Possible JVM deadlock in ThreadTimesClosure when using HotspotInternal non-public API. Reviewed-by: sspitsyn, dholmes
hotspot/src/share/vm/services/management.cpp
hotspot/test/runtime/7196045/Test7196045.java
--- a/hotspot/src/share/vm/services/management.cpp	Tue Sep 18 11:37:26 2012 -0700
+++ b/hotspot/src/share/vm/services/management.cpp	Wed Sep 19 15:24:32 2012 +0100
@@ -1804,31 +1804,37 @@
 
 class ThreadTimesClosure: public ThreadClosure {
  private:
-  objArrayOop _names;
+  objArrayHandle _names_strings;
+  char **_names_chars;
   typeArrayOop _times;
   int _names_len;
   int _times_len;
   int _count;
 
  public:
-  ThreadTimesClosure(objArrayOop names, typeArrayOop times);
+  ThreadTimesClosure(objArrayHandle names, typeArrayOop times);
+  ~ThreadTimesClosure();
   virtual void do_thread(Thread* thread);
+  void do_unlocked();
   int count() { return _count; }
 };
 
-ThreadTimesClosure::ThreadTimesClosure(objArrayOop names,
+ThreadTimesClosure::ThreadTimesClosure(objArrayHandle names,
                                        typeArrayOop times) {
-  assert(names != NULL, "names was NULL");
+  assert(names() != NULL, "names was NULL");
   assert(times != NULL, "times was NULL");
-  _names = names;
+  _names_strings = names;
   _names_len = names->length();
+  _names_chars = NEW_C_HEAP_ARRAY(char*, _names_len, mtInternal);
   _times = times;
   _times_len = times->length();
   _count = 0;
 }
 
+//
+// Called with Threads_lock held
+//
 void ThreadTimesClosure::do_thread(Thread* thread) {
-  Handle s;
   assert(thread != NULL, "thread was NULL");
 
   // exclude externally visible JavaThreads
@@ -1842,16 +1848,32 @@
   }
 
   EXCEPTION_MARK;
+  ResourceMark rm(THREAD); // thread->name() uses ResourceArea
 
   assert(thread->name() != NULL, "All threads should have a name");
-  s = java_lang_String::create_from_str(thread->name(), CHECK);
-  _names->obj_at_put(_count, s());
-
+  _names_chars[_count] = strdup(thread->name());
   _times->long_at_put(_count, os::is_thread_cpu_time_supported() ?
                         os::thread_cpu_time(thread) : -1);
   _count++;
 }
 
+// Called without Threads_lock, we can allocate String objects.
+void ThreadTimesClosure::do_unlocked() {
+
+  EXCEPTION_MARK;
+  for (int i = 0; i < _count; i++) {
+    Handle s = java_lang_String::create_from_str(_names_chars[i],  CHECK);
+    _names_strings->obj_at_put(i, s());
+  }
+}
+
+ThreadTimesClosure::~ThreadTimesClosure() {
+  for (int i = 0; i < _count; i++) {
+    free(_names_chars[i]);
+  }
+  FREE_C_HEAP_ARRAY(char *, _names_chars, mtInternal);
+}
+
 // Fills names with VM internal thread names and times with the corresponding
 // CPU times.  If names or times is NULL, a NullPointerException is thrown.
 // If the element type of names is not String, an IllegalArgumentException is
@@ -1878,12 +1900,12 @@
   typeArrayOop ta = typeArrayOop(JNIHandles::resolve_non_null(times));
   typeArrayHandle times_ah(THREAD, ta);
 
-  ThreadTimesClosure ttc(names_ah(), times_ah());
+  ThreadTimesClosure ttc(names_ah, times_ah());
   {
     MutexLockerEx ml(Threads_lock);
     Threads::threads_do(&ttc);
   }
-
+  ttc.do_unlocked();
   return ttc.count();
 JVM_END
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/7196045/Test7196045.java	Wed Sep 19 15:24:32 2012 +0100
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/*
+ * @test
+ * @bug 7196045
+ * @summary Possible JVM deadlock in ThreadTimesClosure when using HotspotInternal non-public API.
+ * @run main/othervm
+ */
+
+import java.lang.management.ManagementFactory;
+import javax.management.JMException;
+import javax.management.MBeanServer;
+import javax.management.MalformedObjectNameException;
+import javax.management.ObjectName;
+
+public class Test7196045 {
+
+    public static long duration = 1000 * 60 * 2;
+    private static final String HOTSPOT_INTERNAL = "sun.management:type=HotspotInternal";
+
+    public static void main(String[] args) {
+
+        MBeanServer server = ManagementFactory.getPlatformMBeanServer();
+        ObjectName objName= null;
+        try {
+            ObjectName hotspotInternal = new ObjectName(HOTSPOT_INTERNAL);
+            try {
+                server.registerMBean(new sun.management.HotspotInternal(), hotspotInternal);
+            } catch (JMException e) {
+                throw new RuntimeException("HotSpotWatcher: Failed to register the HotspotInternal MBean" + e);
+            }
+            objName= new ObjectName("sun.management:type=HotspotThreading");
+
+        } catch (MalformedObjectNameException e1) {
+            throw new RuntimeException("Bad object name" + e1);
+        }
+
+        long endTime = System.currentTimeMillis() + duration;
+        long i = 0;
+        while (true) {
+            try {
+                server.getAttribute(objName, "InternalThreadCpuTimes");
+            } catch (Exception ex) {
+                System.err.println("Exception while getting attribute: " + ex);
+            }
+            i++;
+            if (i % 10000 == 0) {
+                System.out.println("Successful iterations: " + i);
+            }
+            if (System.currentTimeMillis() > endTime) {
+                break;
+            }
+        }
+        System.out.println("PASSED.");
+    }
+}