8213231: ThreadSnapshot::_threadObj can become stale
Reviewed-by: dcubed, dholmes, rehn
--- a/src/hotspot/share/runtime/vmOperations.cpp Thu Jan 24 12:45:19 2019 +0530
+++ b/src/hotspot/share/runtime/vmOperations.cpp Wed Jan 23 13:40:09 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -333,8 +333,7 @@
if (_with_locked_synchronizers) {
tcl = concurrent_locks.thread_concurrent_locks(jt);
}
- ThreadSnapshot* ts = snapshot_thread(jt, tcl);
- _result->add_thread_snapshot(ts);
+ snapshot_thread(jt, tcl);
}
} else {
// Snapshot threads in the given _threads array
@@ -345,7 +344,7 @@
if (th() == NULL) {
// skip if the thread doesn't exist
// Add a dummy snapshot
- _result->add_thread_snapshot(new ThreadSnapshot());
+ _result->add_thread_snapshot();
continue;
}
@@ -362,24 +361,22 @@
jt->is_exiting() ||
jt->is_hidden_from_external_view()) {
// add a NULL snapshot if skipped
- _result->add_thread_snapshot(new ThreadSnapshot());
+ _result->add_thread_snapshot();
continue;
}
ThreadConcurrentLocks* tcl = NULL;
if (_with_locked_synchronizers) {
tcl = concurrent_locks.thread_concurrent_locks(jt);
}
- ThreadSnapshot* ts = snapshot_thread(jt, tcl);
- _result->add_thread_snapshot(ts);
+ snapshot_thread(jt, tcl);
}
}
}
-ThreadSnapshot* VM_ThreadDump::snapshot_thread(JavaThread* java_thread, ThreadConcurrentLocks* tcl) {
- ThreadSnapshot* snapshot = new ThreadSnapshot(_result->t_list(), java_thread);
+void VM_ThreadDump::snapshot_thread(JavaThread* java_thread, ThreadConcurrentLocks* tcl) {
+ ThreadSnapshot* snapshot = _result->add_thread_snapshot(java_thread);
snapshot->dump_stack_at_safepoint(_max_depth, _with_locked_monitors);
snapshot->set_concurrent_locks(tcl);
- return snapshot;
}
volatile bool VM_Exit::_vm_exited = false;
--- a/src/hotspot/share/runtime/vmOperations.hpp Thu Jan 24 12:45:19 2019 +0530
+++ b/src/hotspot/share/runtime/vmOperations.hpp Wed Jan 23 13:40:09 2019 +0100
@@ -445,7 +445,7 @@
bool _with_locked_monitors;
bool _with_locked_synchronizers;
- ThreadSnapshot* snapshot_thread(JavaThread* java_thread, ThreadConcurrentLocks* tcl);
+ void snapshot_thread(JavaThread* java_thread, ThreadConcurrentLocks* tcl);
public:
VM_ThreadDump(ThreadDumpResult* result,
--- a/src/hotspot/share/services/management.cpp Thu Jan 24 12:45:19 2019 +0530
+++ b/src/hotspot/share/services/management.cpp Wed Jan 23 13:40:09 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -1097,15 +1097,13 @@
for (int i = 0; i < num_threads; i++) {
jlong tid = ids_ah->long_at(i);
JavaThread* jt = dump_result.t_list()->find_JavaThread_from_java_tid(tid);
- ThreadSnapshot* ts;
if (jt == NULL) {
// if the thread does not exist or now it is terminated,
// create dummy snapshot
- ts = new ThreadSnapshot();
+ dump_result.add_thread_snapshot();
} else {
- ts = new ThreadSnapshot(dump_result.t_list(), jt);
+ dump_result.add_thread_snapshot(jt);
}
- dump_result.add_thread_snapshot(ts);
}
} else {
// obtain thread dump with the specific list of threads with stack trace
--- a/src/hotspot/share/services/threadService.cpp Thu Jan 24 12:45:19 2019 +0530
+++ b/src/hotspot/share/services/threadService.cpp Wed Jan 23 13:40:09 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -503,8 +503,25 @@
}
}
+ThreadSnapshot* ThreadDumpResult::add_thread_snapshot() {
+ ThreadSnapshot* ts = new ThreadSnapshot();
+ link_thread_snapshot(ts);
+ return ts;
+}
-void ThreadDumpResult::add_thread_snapshot(ThreadSnapshot* ts) {
+ThreadSnapshot* ThreadDumpResult::add_thread_snapshot(JavaThread* thread) {
+ // Note: it is very important that the ThreadSnapshot* gets linked before
+ // ThreadSnapshot::initialize gets called. This is to ensure that
+ // ThreadSnapshot::oops_do can get called prior to the field
+ // ThreadSnapshot::_threadObj being assigned a value (to prevent a dangling
+ // oop).
+ ThreadSnapshot* ts = new ThreadSnapshot();
+ link_thread_snapshot(ts);
+ ts->initialize(t_list(), thread);
+ return ts;
+}
+
+void ThreadDumpResult::link_thread_snapshot(ThreadSnapshot* ts) {
assert(_num_threads == 0 || _num_snapshots < _num_threads,
"_num_snapshots must be less than _num_threads");
_num_snapshots++;
@@ -831,12 +848,9 @@
memset((void*) _perf_recursion_counts, 0, sizeof(_perf_recursion_counts));
}
-ThreadSnapshot::ThreadSnapshot(ThreadsList * t_list, JavaThread* thread) {
+void ThreadSnapshot::initialize(ThreadsList * t_list, JavaThread* thread) {
_thread = thread;
_threadObj = thread->threadObj();
- _stack_trace = NULL;
- _concurrent_locks = NULL;
- _next = NULL;
ThreadStatistics* stat = thread->get_thread_stat();
_contended_enter_ticks = stat->contended_enter_ticks();
@@ -846,9 +860,6 @@
_sleep_ticks = stat->sleep_ticks();
_sleep_count = stat->sleep_count();
- _blocker_object = NULL;
- _blocker_object_owner = NULL;
-
_thread_status = java_lang_Thread::get_thread_status(_threadObj);
_is_ext_suspended = thread->is_being_ext_suspended();
_is_in_native = (thread->thread_state() == _thread_in_native);
--- a/src/hotspot/share/services/threadService.hpp Thu Jan 24 12:45:19 2019 +0530
+++ b/src/hotspot/share/services/threadService.hpp Wed Jan 23 13:40:09 2019 +0100
@@ -213,12 +213,15 @@
ThreadConcurrentLocks* _concurrent_locks;
ThreadSnapshot* _next;
-public:
- // Dummy snapshot
+ // ThreadSnapshot instances should only be created via
+ // ThreadDumpResult::add_thread_snapshot.
+ friend class ThreadDumpResult;
ThreadSnapshot() : _thread(NULL), _threadObj(NULL),
_blocker_object(NULL), _blocker_object_owner(NULL),
_stack_trace(NULL), _concurrent_locks(NULL), _next(NULL) {};
- ThreadSnapshot(ThreadsList * t_list, JavaThread* thread);
+ void initialize(ThreadsList * t_list, JavaThread* thread);
+
+public:
~ThreadSnapshot();
java_lang_Thread::ThreadStatus thread_status() { return _thread_status; }
@@ -367,12 +370,16 @@
ThreadsListSetter _setter; // Helper to set hazard ptr in the originating thread
// which protects the JavaThreads in _snapshots.
+ void link_thread_snapshot(ThreadSnapshot* ts);
+
public:
ThreadDumpResult();
ThreadDumpResult(int num_threads);
~ThreadDumpResult();
- void add_thread_snapshot(ThreadSnapshot* ts);
+ ThreadSnapshot* add_thread_snapshot();
+ ThreadSnapshot* add_thread_snapshot(JavaThread* thread);
+
void set_next(ThreadDumpResult* next) { _next = next; }
ThreadDumpResult* next() { return _next; }
int num_threads() { return _num_threads; }