8231666: ThreadIdTable::grow() invokes invalid thread transition
Reviewed-by: dholmes, rehn
--- a/src/hotspot/share/runtime/serviceThread.cpp Tue Oct 08 09:13:08 2019 -0700
+++ b/src/hotspot/share/runtime/serviceThread.cpp Tue Oct 08 09:39:10 2019 -0700
@@ -43,6 +43,7 @@
#include "services/diagnosticFramework.hpp"
#include "services/gcNotifier.hpp"
#include "services/lowMemoryDetector.hpp"
+#include "services/threadIdTable.hpp"
ServiceThread* ServiceThread::_instance = NULL;
@@ -101,6 +102,7 @@
bool stringtable_work = false;
bool symboltable_work = false;
bool resolved_method_table_work = false;
+ bool thread_id_table_work = false;
bool protection_domain_table_work = false;
bool oopstorage_work = false;
JvmtiDeferredEvent jvmti_event;
@@ -127,6 +129,7 @@
(stringtable_work = StringTable::has_work()) |
(symboltable_work = SymbolTable::has_work()) |
(resolved_method_table_work = ResolvedMethodTable::has_work()) |
+ (thread_id_table_work = ThreadIdTable::has_work()) |
(protection_domain_table_work = SystemDictionary::pd_cache_table()->has_work()) |
(oopstorage_work = OopStorage::has_cleanup_work_and_reset())
) == 0) {
@@ -169,6 +172,10 @@
ResolvedMethodTable::do_concurrent_work(jt);
}
+ if (thread_id_table_work) {
+ ThreadIdTable::do_concurrent_work(jt);
+ }
+
if (protection_domain_table_work) {
SystemDictionary::pd_cache_table()->unlink();
}
--- a/src/hotspot/share/services/threadIdTable.cpp Tue Oct 08 09:13:08 2019 -0700
+++ b/src/hotspot/share/services/threadIdTable.cpp Tue Oct 08 09:39:10 2019 -0700
@@ -46,6 +46,7 @@
static volatile size_t _items_count = 0;
volatile bool ThreadIdTable::_is_initialized = false;
+volatile bool ThreadIdTable::_has_work = false;
class ThreadIdTableEntry : public CHeapObj<mtInternal> {
private:
@@ -141,6 +142,26 @@
return (size_t)1 << _local_table->get_size_log2(Thread::current());
}
+void ThreadIdTable::check_concurrent_work() {
+ if (_has_work) {
+ return;
+ }
+
+ double load_factor = get_load_factor();
+ // Resize if we have more items than preferred load factor
+ if ( load_factor > PREF_AVG_LIST_LEN && !_local_table->is_max_size_reached()) {
+ log_debug(thread, table)("Concurrent work triggered, load factor: %g",
+ load_factor);
+ trigger_concurrent_work();
+ }
+}
+
+void ThreadIdTable::trigger_concurrent_work() {
+ MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
+ _has_work = true;
+ Service_lock->notify_all();
+}
+
void ThreadIdTable::grow(JavaThread* jt) {
ThreadIdTableHash::GrowTask gt(_local_table);
if (!gt.prepare(jt)) {
@@ -192,13 +213,13 @@
}
};
-void ThreadIdTable::grow_if_required() {
- assert(Thread::current()->is_Java_thread(),"Must be Java thread");
+void ThreadIdTable::do_concurrent_work(JavaThread* jt) {
assert(_is_initialized, "Thread table is not initialized");
+ _has_work = false;
double load_factor = get_load_factor();
log_debug(thread, table)("Concurrent work, load factor: %g", load_factor);
if (load_factor > PREF_AVG_LIST_LEN && !_local_table->is_max_size_reached()) {
- grow(JavaThread::current());
+ grow(jt);
}
}
@@ -215,7 +236,7 @@
// The hash table takes ownership of the ThreadTableEntry,
// even if it's not inserted.
if (_local_table->insert(thread, lookup, entry)) {
- grow_if_required();
+ check_concurrent_work();
return java_thread;
}
}
--- a/src/hotspot/share/services/threadIdTable.hpp Tue Oct 08 09:13:08 2019 -0700
+++ b/src/hotspot/share/services/threadIdTable.hpp Tue Oct 08 09:39:10 2019 -0700
@@ -36,6 +36,7 @@
friend class ThreadIdTableConfig;
static volatile bool _is_initialized;
+ static volatile bool _has_work;
public:
// Initialization
@@ -47,12 +48,17 @@
static JavaThread* add_thread(jlong tid, JavaThread* thread);
static bool remove_thread(jlong tid);
+ // Growing
+ static bool has_work() { return _has_work; }
+ static void do_concurrent_work(JavaThread* jt);
+
private:
static void create_table(size_t size);
static size_t table_size();
static double get_load_factor();
- static void grow_if_required();
+ static void check_concurrent_work();
+ static void trigger_concurrent_work();
static void grow(JavaThread* jt);
static void item_added();
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/javax/management/mxbean/ThreadStartTest.java Tue Oct 08 09:39:10 2019 -0700
@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2019, 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 8231666
+ * @summary Test checks that new threads could be successfully started when the thread
+ * table introduced in 8185005 is growing. The test enables the thread table by calling
+ * ThreadMXBean.getThreadInfo() and then creates a number of threads to force the thread
+ * table to grow.
+ *
+ * @run main ThreadStartTest
+ */
+
+import java.lang.management.ManagementFactory;
+import java.lang.management.ThreadMXBean;
+
+public class ThreadStartTest {
+ public static void main(String[] args) {
+
+ ThreadMXBean mbean = ManagementFactory.getThreadMXBean();
+ // Enable thread table
+ mbean.getThreadInfo(Thread.currentThread().getId());
+
+ // Create a large number of threads to make the thread table grow
+ for (int i = 0; i < 1000; i++) {
+ Thread t = new Thread(() -> {
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException ex) {
+ }
+ });
+ t.start();
+ }
+ }
+}