# HG changeset patch # User ehelin # Date 1548247209 -3600 # Node ID 391d671f222b94cc6032239d6d34d2565b89105a # Parent 1ffc0fa0e98baeef0d7a29ca9d44ad1696a572e2 8213231: ThreadSnapshot::_threadObj can become stale Reviewed-by: dcubed, dholmes, rehn diff -r 1ffc0fa0e98b -r 391d671f222b src/hotspot/share/runtime/vmOperations.cpp --- a/src/hotspot/share/runtime/vmOperations.cpp Fri Jan 25 10:59:07 2019 +0000 +++ 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; diff -r 1ffc0fa0e98b -r 391d671f222b src/hotspot/share/runtime/vmOperations.hpp --- a/src/hotspot/share/runtime/vmOperations.hpp Fri Jan 25 10:59:07 2019 +0000 +++ 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, diff -r 1ffc0fa0e98b -r 391d671f222b src/hotspot/share/services/management.cpp --- a/src/hotspot/share/services/management.cpp Fri Jan 25 10:59:07 2019 +0000 +++ 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 diff -r 1ffc0fa0e98b -r 391d671f222b src/hotspot/share/services/threadService.cpp --- a/src/hotspot/share/services/threadService.cpp Fri Jan 25 10:59:07 2019 +0000 +++ 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); diff -r 1ffc0fa0e98b -r 391d671f222b src/hotspot/share/services/threadService.hpp --- a/src/hotspot/share/services/threadService.hpp Fri Jan 25 10:59:07 2019 +0000 +++ 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; }