diff --git a/src/hotspot/share/runtime/threadSMR.cpp b/src/hotspot/share/runtime/threadSMR.cpp index 418f7707118..4c68648fec8 100644 --- a/src/hotspot/share/runtime/threadSMR.cpp +++ b/src/hotspot/share/runtime/threadSMR.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2026, 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 @@ -726,7 +726,8 @@ JavaThread* ThreadsList::find_JavaThread_from_java_tid(jlong java_tid) const { } } } - } else if (!thread->is_exiting()) { + } else if (includes(thread) && !thread->is_exiting()) { + // The thread is protected by this list and has not yet exited return thread; } return nullptr; @@ -883,7 +884,7 @@ void ThreadsSMRSupport::add_thread(JavaThread *thread){ ThreadsList *old_list = xchg_java_thread_list(new_list); free_list(old_list); - if (ThreadIdTable::is_initialized()) { + if (ThreadIdTable::is_initialized_acquire()) { jlong tid = SharedRuntime::get_java_tid(thread); ThreadIdTable::add_thread(tid, thread); } diff --git a/src/hotspot/share/runtime/threads.cpp b/src/hotspot/share/runtime/threads.cpp index f7f755a37b3..442b68e596a 100644 --- a/src/hotspot/share/runtime/threads.cpp +++ b/src/hotspot/share/runtime/threads.cpp @@ -1127,7 +1127,7 @@ void Threads::remove(JavaThread* p, bool is_daemon) { ConditionalMutexLocker throttle_ml(ThreadsLockThrottle_lock, UseThreadsLockThrottleLock); MonitorLocker ml(Threads_lock); - if (ThreadIdTable::is_initialized()) { + if (ThreadIdTable::is_initialized_acquire()) { // This cleanup must be done before the current thread's GC barrier // is detached since we need to touch the threadObj oop. jlong tid = SharedRuntime::get_java_tid(p); diff --git a/src/hotspot/share/services/management.cpp b/src/hotspot/share/services/management.cpp index 09277e16479..664fb5a8ef3 100644 --- a/src/hotspot/share/services/management.cpp +++ b/src/hotspot/share/services/management.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2026, 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 @@ -1146,6 +1146,7 @@ JVM_ENTRY(jint, jmm_GetThreadInfo(JNIEnv *env, jlongArray ids, jint maxDepth, jo // create dummy snapshot dump_result.add_thread_snapshot(); } else { + assert(dump_result.t_list()->includes(jt), "Must be protected"); dump_result.add_thread_snapshot(jt); } } diff --git a/src/hotspot/share/services/threadIdTable.cpp b/src/hotspot/share/services/threadIdTable.cpp index 1604927a0ac..d5141441ccd 100644 --- a/src/hotspot/share/services/threadIdTable.cpp +++ b/src/hotspot/share/services/threadIdTable.cpp @@ -1,5 +1,5 @@ /* -* Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. +* Copyright (c) 2019, 2026, 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 @@ -23,7 +23,6 @@ */ #include "classfile/javaClasses.inline.hpp" -#include "runtime/atomicAccess.hpp" #include "runtime/interfaceSupport.inline.hpp" #include "runtime/javaThread.inline.hpp" #include "runtime/threadSMR.hpp" @@ -44,7 +43,7 @@ static ThreadIdTableHash* volatile _local_table = nullptr; static volatile size_t _current_size = 0; static volatile size_t _items_count = 0; -volatile bool ThreadIdTable::_is_initialized = false; +Atomic ThreadIdTable::_is_initialized {false}; volatile bool ThreadIdTable::_has_work = false; class ThreadIdTableEntry : public CHeapObj { @@ -81,24 +80,25 @@ class ThreadIdTableConfig : public AllStatic { // Lazily creates the table and populates it with the given // thread list void ThreadIdTable::lazy_initialize(const ThreadsList *threads) { - if (!_is_initialized) { + if (!_is_initialized.load_acquire()) { { // There is no obvious benefit in allowing the thread table // to be concurrently populated during initialization. MutexLocker ml(ThreadIdTableCreate_lock); - if (_is_initialized) { + if (_is_initialized.load_relaxed()) { return; } create_table(threads->length()); - _is_initialized = true; + _is_initialized.release_store(true); } + for (uint i = 0; i < threads->length(); i++) { JavaThread* thread = threads->thread_at(i); oop tobj = thread->threadObj(); if (tobj != nullptr) { - jlong java_tid = java_lang_Thread::thread_id(tobj); MutexLocker ml(Threads_lock); if (!thread->is_exiting()) { + jlong java_tid = java_lang_Thread::thread_id(tobj); // Must be inside the lock to ensure that we don't add a thread to the table // that has just passed the removal point in Threads::remove(). add_thread(java_tid, thread); @@ -211,7 +211,7 @@ public: }; void ThreadIdTable::do_concurrent_work(JavaThread* jt) { - assert(_is_initialized, "Thread table is not initialized"); + assert(_is_initialized.load_relaxed(), "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); @@ -221,7 +221,8 @@ void ThreadIdTable::do_concurrent_work(JavaThread* jt) { } JavaThread* ThreadIdTable::add_thread(jlong tid, JavaThread* java_thread) { - assert(_is_initialized, "Thread table is not initialized"); + assert(Threads_lock->owned_by_self(), "Must hold Threads_lock"); + assert(_is_initialized.load_relaxed(), "Thread table is not initialized"); Thread* thread = Thread::current(); ThreadIdTableLookup lookup(tid); ThreadGet tg; @@ -240,7 +241,7 @@ JavaThread* ThreadIdTable::add_thread(jlong tid, JavaThread* java_thread) { } JavaThread* ThreadIdTable::find_thread_by_tid(jlong tid) { - assert(_is_initialized, "Thread table is not initialized"); + assert(_is_initialized.load_relaxed(), "Thread table is not initialized"); Thread* thread = Thread::current(); ThreadIdTableLookup lookup(tid); ThreadGet tg; @@ -249,7 +250,8 @@ JavaThread* ThreadIdTable::find_thread_by_tid(jlong tid) { } bool ThreadIdTable::remove_thread(jlong tid) { - assert(_is_initialized, "Thread table is not initialized"); + assert(Threads_lock->owned_by_self(), "Must hold Threads_lock"); + assert(_is_initialized.load_relaxed(), "Thread table is not initialized"); Thread* thread = Thread::current(); ThreadIdTableLookup lookup(tid); return _local_table->remove(thread, lookup); diff --git a/src/hotspot/share/services/threadIdTable.hpp b/src/hotspot/share/services/threadIdTable.hpp index 15dfb89d670..256484cce2d 100644 --- a/src/hotspot/share/services/threadIdTable.hpp +++ b/src/hotspot/share/services/threadIdTable.hpp @@ -1,5 +1,5 @@ /* -* Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. +* Copyright (c) 2019, 2026, 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 @@ -26,6 +26,7 @@ #define SHARE_SERVICES_THREADIDTABLE_HPP #include "memory/allStatic.hpp" +#include "runtime/atomic.hpp" class JavaThread; class ThreadsList; @@ -34,13 +35,15 @@ class ThreadIdTableConfig; class ThreadIdTable : public AllStatic { friend class ThreadIdTableConfig; - static volatile bool _is_initialized; + static Atomic _is_initialized; static volatile bool _has_work; public: // Initialization static void lazy_initialize(const ThreadsList* threads); - static bool is_initialized() { return _is_initialized; } + static bool is_initialized_acquire() { + return _is_initialized.load_acquire(); + } // Lookup and list management static JavaThread* find_thread_by_tid(jlong tid); diff --git a/test/hotspot/jtreg/serviceability/threads/ThreadInfoTest.java b/test/hotspot/jtreg/serviceability/threads/ThreadInfoTest.java new file mode 100644 index 00000000000..b5db455cc20 --- /dev/null +++ b/test/hotspot/jtreg/serviceability/threads/ThreadInfoTest.java @@ -0,0 +1,187 @@ +/* + * Copyright (c) 2026, 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. + */ + +import jdk.test.lib.Utils; + +import java.lang.management.ManagementFactory; +import java.lang.management.ThreadInfo; +import java.lang.management.ThreadMXBean; + +import java.util.ArrayList; +import java.util.List; + +/* + * @test + * @bug 8323792 + * @summary Make sure that jmm_GetThreadInfo() call does not crash JVM + * @library /test/lib + * @modules java.management + * @run main/othervm ThreadInfoTest + * + * @comment Exercise getThreadInfo(ids, 0). Depth parameter of zero means + * no VM operation, which could crash with Threads starting and ending. + */ + +public class ThreadInfoTest { + private static com.sun.management.ThreadMXBean mbean = + (com.sun.management.ThreadMXBean)ManagementFactory.getThreadMXBean(); + + private static final int NUM_THREADS = 2; + static long[] ids = new long[NUM_THREADS]; + static ThreadInfo[] infos = new ThreadInfo[NUM_THREADS]; + static volatile int count = 0; + static int ITERATIONS = 4; + + public static void main(String[] argv) throws Exception { + boolean replacing = false; + + startThreads(ids, NUM_THREADS); + new MyGetThreadInfoThread(ids).start(); + new MyReplacerThread(ids).start(); + for (int i = 0; i < ITERATIONS; i++) { + do { + count = countInfo(infos); + System.out.println("Iteration " + i + ": ThreadInfos found (Threads alive): " + count); + goSleep(100); + } while (count > 0); + } + } + + private static Thread newThread(int i) { + Thread thread = new MyThread(i); + thread.setDaemon(true); + return thread; + } + + private static void startThreads(long[] ids, int count) { + System.out.println("Starting " + count + " Threads..."); + Thread[] threads = new Thread[count]; + for (int i = 0; i < count; i++) { + threads[i] = newThread(i); + threads[i].start(); + ids[i] = threads[i].getId(); + } + System.out.println(ids); + } + + // Count ThreadInfo from array, return how many are non-null. + private static int countInfo(ThreadInfo[] info) { + int count = 0; + if (info != null) { + int i = 0; + for (ThreadInfo ti: info) { + if (ti != null) { + count++; + } + i++; + } + } + return count; + } + + private static int replaceThreads(long[] ids, ThreadInfo[] info) { + int replaced = 0; + if (info != null) { + for (int i = 0; i < info.length; i++) { + ThreadInfo ti = info[i]; + if (ti == null) { + Thread thread = newThread(i); + thread.start(); + ids[i] = thread.getId(); + replaced++; + } + } + } + return replaced; + } + + private static void goSleep(long ms) { + try { + Thread.sleep(ms); + } catch (InterruptedException e) { + System.out.println("Unexpected exception is thrown: " + e); + } + } + + // A Thread which replaces Threads in the shared array of threads. + static class MyReplacerThread extends Thread { + long[] ids; + + public MyReplacerThread(long[] ids) { + this.ids = ids; + this.setDaemon(true); + } + + public void run() { + boolean replacing = false; + while (true) { + if (replacing) { + replaceThreads(ids, infos); + } + if (count < 10) { + replacing = true; + } + if (count > 20) { + replacing = false; + } + goSleep(1); + } + } + } + + // A Thread which lives for a short while. + static class MyThread extends Thread { + long endTimeMs; + + public MyThread(long n) { + super("MyThread-" + n); + endTimeMs = (n * n * 10) + System.currentTimeMillis(); + } + + public void run() { + try { + long sleep = Math.max(1, endTimeMs - System.currentTimeMillis()); + goSleep(sleep); + } catch (Exception e) { + System.out.println(Thread.currentThread().getName() + ": " + e); + } + } + } + + // A Thread to continually call getThreadInfo on a shared array of thread ids. + static class MyGetThreadInfoThread extends Thread { + long[] ids; + + public MyGetThreadInfoThread(long[] ids) { + this.ids = ids; + this.setDaemon(true); + } + + public void run() { + while (true) { + infos = mbean.getThreadInfo(ids, 0); + goSleep(10); + } + } + } +}