8240588: _threadObj cannot be used on an exiting JavaThread

Reviewed-by: rehn, dcubed, kbarrett
This commit is contained in:
David Holmes 2020-05-13 22:29:54 -04:00
parent be7771b2b9
commit 17dd7dc38c
8 changed files with 266 additions and 3 deletions
src/hotspot/share
test
hotspot/jtreg/runtime/Thread
lib/sun/hotspot

@ -205,6 +205,7 @@ void Universe::oops_do(OopClosure* f) {
f->do_oop((oop*)&_vm_exception);
f->do_oop((oop*)&_reference_pending_list);
debug_only(f->do_oop((oop*)&_fullgc_alot_dummy_array);)
ThreadsSMRSupport::exiting_threads_oops_do(f);
}
void LatestMethodCache::metaspace_pointers_do(MetaspaceClosure* it) {

@ -2222,6 +2222,60 @@ WB_ENTRY(jint, WB_GetKlassMetadataSize(JNIEnv* env, jobject wb, jclass mirror))
return k->size() * wordSize;
WB_END
// See test/hotspot/jtreg/runtime/Thread/ThreadObjAccessAtExit.java.
// It explains how the thread's priority field is used for test state coordination.
//
WB_ENTRY(void, WB_CheckThreadObjOfTerminatingThread(JNIEnv* env, jobject wb, jobject target_handle))
oop target_oop = JNIHandles::resolve_non_null(target_handle);
jlong tid = java_lang_Thread::thread_id(target_oop);
JavaThread* target = java_lang_Thread::thread(target_oop);
// Grab a ThreadsListHandle to protect the target thread whilst terminating
ThreadsListHandle tlh;
// Look up the target thread by tid to ensure it is present
JavaThread* t = tlh.list()->find_JavaThread_from_java_tid(tid);
if (t == NULL) {
THROW_MSG(vmSymbols::java_lang_RuntimeException(), "Target thread not found in ThreadsList!");
}
tty->print_cr("WB_CheckThreadObjOfTerminatingThread: target thread is protected");
// Allow target to terminate by boosting priority
java_lang_Thread::set_priority(t->threadObj(), ThreadPriority(NormPriority + 1));
// Now wait for the target to terminate
while (!target->is_terminated()) {
ThreadBlockInVM tbivm(thread); // just in case target is involved in a safepoint
os::naked_short_sleep(0);
}
tty->print_cr("WB_CheckThreadObjOfTerminatingThread: target thread is terminated");
// Now release the GC inducing thread - we have to re-resolve the external oop that
// was passed in as GC may have occurred and we don't know if we can trust t->threadObj() now.
oop original = JNIHandles::resolve_non_null(target_handle);
java_lang_Thread::set_priority(original, ThreadPriority(NormPriority + 2));
tty->print_cr("WB_CheckThreadObjOfTerminatingThread: GC has been initiated - checking threadObj:");
// The Java code should be creating garbage and triggering GC, which would potentially move
// the threadObj oop. If the exiting thread is properly protected then its threadObj should
// remain valid and equal to our initial target_handle. Loop a few times to give GC a chance to
// kick in.
for (int i = 0; i < 5; i++) {
oop original = JNIHandles::resolve_non_null(target_handle);
oop current = t->threadObj();
if (original != current) {
tty->print_cr("WB_CheckThreadObjOfTerminatingThread: failed comparison on iteration %d", i);
THROW_MSG(vmSymbols::java_lang_RuntimeException(), "Target thread oop has changed!");
} else {
tty->print_cr("WB_CheckThreadObjOfTerminatingThread: successful comparison on iteration %d", i);
ThreadBlockInVM tbivm(thread);
os::naked_short_sleep(50);
}
}
WB_END
#define CC (char*)
static JNINativeMethod methods[] = {
@ -2447,6 +2501,7 @@ static JNINativeMethod methods[] = {
{CC"clearInlineCaches0", CC"(Z)V", (void*)&WB_ClearInlineCaches },
{CC"handshakeWalkStack", CC"(Ljava/lang/Thread;Z)I", (void*)&WB_HandshakeWalkStack },
{CC"checkThreadObjOfTerminatingThread", CC"(Ljava/lang/Thread;)V", (void*)&WB_CheckThreadObjOfTerminatingThread },
{CC"addCompilerDirective", CC"(Ljava/lang/String;)I",
(void*)&WB_AddCompilerDirective },
{CC"removeCompilerDirective", CC"(I)V", (void*)&WB_RemoveCompilerDirective },

@ -2169,6 +2169,14 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
JvmtiExport::cleanup_thread(this);
}
// We need to cache the thread name for logging purposes below as once
// we have called on_thread_detach this thread must not access any oops.
char* thread_name = NULL;
if (log_is_enabled(Debug, os, thread, timer)) {
ResourceMark rm(this);
thread_name = os::strdup(get_thread_name());
}
// We must flush any deferred card marks and other various GC barrier
// related buffers (e.g. G1 SATB buffer and G1 dirty card queue buffer)
// before removing a thread from the list of active threads.
@ -2187,17 +2195,17 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
if (log_is_enabled(Debug, os, thread, timer)) {
_timer_exit_phase4.stop();
ResourceMark rm(this);
log_debug(os, thread, timer)("name='%s'"
", exit-phase1=" JLONG_FORMAT
", exit-phase2=" JLONG_FORMAT
", exit-phase3=" JLONG_FORMAT
", exit-phase4=" JLONG_FORMAT,
get_thread_name(),
thread_name,
_timer_exit_phase1.milliseconds(),
_timer_exit_phase2.milliseconds(),
_timer_exit_phase3.milliseconds(),
_timer_exit_phase4.milliseconds());
os::free(thread_name);
}
}

@ -1017,6 +1017,7 @@ class JavaThread: public Thread {
friend class VMStructs;
friend class JVMCIVMStructs;
friend class WhiteBox;
friend class ThreadsSMRSupport; // to access _threadObj for exiting_threads_oops_do
private:
bool _on_thread_list; // Is set when this JavaThread is added to the Threads list
oop _threadObj; // The Java level thread object

@ -41,6 +41,9 @@
#include "utilities/resourceHash.hpp"
#include "utilities/vmError.hpp"
// List of exiting threads
ThreadsSMRSupport::Holder* ThreadsSMRSupport::_exiting_threads = NULL;
// The '_cnt', '_max' and '_times" fields are enabled via
// -XX:+EnableThreadSMRStatistics:
@ -923,10 +926,14 @@ void ThreadsSMRSupport::release_stable_list_wake_up(bool is_nested) {
}
void ThreadsSMRSupport::remove_thread(JavaThread *thread) {
ThreadsSMRSupport::add_exiting_thread(thread);
if (ThreadIdTable::is_initialized()) {
jlong tid = SharedRuntime::get_java_tid(thread);
ThreadIdTable::remove_thread(tid);
}
ThreadsList *new_list = ThreadsList::remove_thread(ThreadsSMRSupport::get_java_thread_list(), thread);
if (EnableThreadSMRStatistics) {
ThreadsSMRSupport::inc_java_thread_list_alloc_cnt();
@ -991,6 +998,7 @@ void ThreadsSMRSupport::wait_until_not_protected(JavaThread *thread) {
// This is the common case.
ThreadsSMRSupport::clear_delete_notify();
ThreadsSMRSupport::delete_lock()->unlock();
ThreadsSMRSupport::remove_exiting_thread(thread);
break;
}
if (!has_logged_once) {
@ -1180,3 +1188,47 @@ void ThreadsSMRSupport::print_info_elements_on(outputStream* st, ThreadsList* t_
cnt++;
}
}
void ThreadsSMRSupport::add_exiting_thread(JavaThread* thread) {
assert(thread == JavaThread::current(), "invariant");
assert(Threads_lock->owned_by_self(), "invariant");
assert(!contains_exiting_thread(thread), "invariant");
Holder* h = new Holder(thread, _exiting_threads);
_exiting_threads = h;
}
void ThreadsSMRSupport::remove_exiting_thread(JavaThread* thread) {
assert(thread == JavaThread::current(), "invariant");
assert(Threads_lock->owned_by_self(), "invariant");
// If a thread fails to initialize fully it can be deleted immediately
// so we won't remove it from the ThreadsList and so never add it to the
// exiting thread list - so we can't assert(contains_exiting_thread(p)) here.
for (Holder* current = _exiting_threads, **prev_next = &_exiting_threads;
current != NULL;
prev_next = &current->_next, current = current->_next) {
if (current->_thread == thread) {
*prev_next = current->_next;
delete current;
break;
}
}
}
#ifdef ASSERT
bool ThreadsSMRSupport::contains_exiting_thread(JavaThread* thread) {
for (Holder* current = _exiting_threads; current != NULL; current = current->_next) {
if (current->_thread == thread) {
return true;
}
}
return false;
}
#endif
void ThreadsSMRSupport::exiting_threads_oops_do(OopClosure* f) {
assert_locked_or_safepoint(Threads_lock);
for (Holder* current = _exiting_threads; current != NULL; current = current->_next) {
f->do_oop((oop*) &current->_thread->_threadObj);
}
}

@ -81,7 +81,16 @@ class ThreadClosure;
// remains in scope. The target JavaThread * may have logically exited,
// but that target JavaThread * will not be deleted until it is no
// longer protected by a ThreadsListHandle.
//
// Once a JavaThread has removed itself from the main ThreadsList it is
// no longer visited by GC. To ensure that thread's threadObj() oop remains
// valid while the thread is still accessible from a ThreadsListHandle we
// maintain a special list of exiting threads:
// - In remove() we add the exiting thread to the list (under the Threads_lock).
// - In wait_until_not_protected() we remove it from the list (again under the
// Threads_lock).
// - Universe::oops_do walks the list (at a safepoint so VMThread holds
// Threads_lock) and visits the _threadObj oop of each JavaThread.
// SMR Support for the Threads class.
//
@ -89,6 +98,17 @@ class ThreadsSMRSupport : AllStatic {
friend class VMStructs;
friend class SafeThreadsListPtr; // for _nested_thread_list_max, delete_notify(), release_stable_list_wake_up() access
// Helper class for the exiting thread list
class Holder : public CHeapObj<mtInternal> {
public:
JavaThread* _thread;
Holder* _next;
Holder(JavaThread* thread, Holder* next) : _thread(thread), _next(next) {}
};
// The list of exiting threads
static Holder* _exiting_threads;
// The coordination between ThreadsSMRSupport::release_stable_list() and
// ThreadsSMRSupport::smr_delete() uses the delete_lock in order to
// reduce the traffic on the Threads_lock.
@ -150,6 +170,12 @@ class ThreadsSMRSupport : AllStatic {
static void smr_delete(JavaThread *thread);
static void update_tlh_stats(uint millis);
// Exiting thread list maintenance
static void add_exiting_thread(JavaThread* thread);
static void remove_exiting_thread(JavaThread* thread);
DEBUG_ONLY(static bool contains_exiting_thread(JavaThread* thread);)
static void exiting_threads_oops_do(OopClosure* f);
// Logging and printing support:
static void log_statistics();
static void print_info_elements_on(outputStream* st, ThreadsList* t_list);

@ -0,0 +1,117 @@
/*
* Copyright (c) 2020, 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 8240588
* @summary Use the WhiteBox API to ensure that we can safely access the
* threadObj oop of a JavaThread during termination, after it has
* removed itself from the main ThreadsList.
* @library /testlibrary /test/lib
* @build sun.hotspot.WhiteBox
* @run driver ClassFileInstaller sun.hotspot.WhiteBox
* @comment run with a small heap, but we need at least 7M for ZGC
* @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xmx7m -XX:-DisableExplicitGC ThreadObjAccessAtExit
*/
import sun.hotspot.WhiteBox;
// We need to coordinate the actions of the target thread, the GC thread
// and the WB main test method. To simplify things we use the target
// thread's priority field to indicate the state of the test as follows:
// - Start the target thread with priority 5 (NORM_PRIORITY), it will run
// until its priority is boosted by 1
// - Start the GC thread, which will spin until the target thread's priority
// has been boosted by 2
// - Call into the WB test method and:
// - Grab a ThreadsListHandle and ensure it contains the target
// - Increase the target priority by one so it will terminate
// - Wait until we see the JavaThread has terminated
// - Increase the target thread priority by one again to release the GC thread
// - Check the original Thread oop with the target->threadObj to see if they
// are the same (looping a few times to improve the chances of GC having
// time to move the underlying object).
// - If the oop has changed throw an exception
public class ThreadObjAccessAtExit {
static class GCThread extends Thread {
// Allocate a moderate-size array
static Object[] arr = new Object[64*1024];
// Wait till we see the main thread is ready then clear the storage
// we consumed at class initialization and run an explicit GC cycle.
// This is sufficient (via experimentation) to cause the oop to be
// relocated.
public void run() {
System.out.println("GCThread waiting ... ");
try {
while (target.getPriority() != Thread.NORM_PRIORITY + 2) {
Thread.sleep(10);
}
}
catch(InterruptedException ie) {
throw new RuntimeException(ie);
}
System.out.println("GCThread running ... ");
arr = null;
System.gc();
}
}
static Thread target; // for easy access from GCThread
public static void main(String[] args) throws Throwable {
WhiteBox wb = WhiteBox.getWhiteBox();
// Create the GCThread, which performs the initial large
// allocation that will later be released when it runs.
GCThread g = new GCThread();
g.setName("GCThread");
// Create the target thread (hopefully in a region that will cause
// it to move when GC happens later).
target = new Thread("Target") {
public void run() {
Thread current = Thread.currentThread();
// Wait until we are told to terminate by the main thread
try {
while (current.getPriority() != Thread.NORM_PRIORITY + 1) {
Thread.sleep(10);
}
}
catch(InterruptedException ie) {
throw new RuntimeException(ie);
}
System.out.println("Target is terminating");
}
};
g.start();
target.setPriority(Thread.NORM_PRIORITY); // just to be explicit
target.start();
wb.checkThreadObjOfTerminatingThread(target);
}
}

@ -613,4 +613,7 @@ public class WhiteBox {
public native int aotLibrariesCount();
public native int getKlassMetadataSize(Class<?> c);
// ThreadSMR GC safety check for threadObj
public native void checkThreadObjOfTerminatingThread(Thread target);
}