--- old/src/hotspot/share/runtime/thread.cpp 2018-12-10 01:36:27.384865433 -0500 +++ new/src/hotspot/share/runtime/thread.cpp 2018-12-10 01:36:25.764772763 -0500 @@ -213,8 +213,12 @@ // Base class for all threads: VMThread, WatcherThread, ConcurrentMarkSweepThread, // JavaThread +DEBUG_ONLY(Thread* Thread::_starting_thread = NULL;) Thread::Thread() { + + DEBUG_ONLY(_run_state = PRE_CALL_RUN;) + // stack and get_thread set_stack_base(NULL); set_stack_size(0); @@ -363,9 +367,16 @@ #endif // INCLUDE_NMT void Thread::call_run() { + DEBUG_ONLY(_run_state = CALL_RUN;) + // At this point, Thread object should be fully initialized and // Thread::current() should be set. + assert(Thread::current_or_null() != NULL, "current thread is unset"); + assert(Thread::current_or_null() == this, "current thread is wrong"); + + // Perform common initialization actions + register_thread_stack_with_NMT(); JFR_ONLY(Jfr::on_thread_start(this);) @@ -375,25 +386,41 @@ os::current_thread_id(), p2i(stack_base() - stack_size()), p2i(stack_base()), stack_size()/1024); + // Perform initialization actions + DEBUG_ONLY(_run_state = PRE_RUN;) + this->pre_run(); + // Invoke ::run() + DEBUG_ONLY(_run_state = RUN;) this->run(); // Returned from ::run(). Thread finished. - // Note: at this point the thread object may already have deleted itself. - // So from here on do not dereference *this*. + // Perform common tear-down actions - // If a thread has not deleted itself ("delete this") as part of its - // termination sequence, we have to ensure thread-local-storage is - // cleared before we actually terminate. No threads should ever be - // deleted asynchronously with respect to their termination. - if (Thread::current_or_null_safe() != NULL) { - assert(Thread::current_or_null_safe() == this, "current thread is wrong"); - Thread::clear_thread_current(); - } + assert(Thread::current_or_null() != NULL, "current thread is unset"); + assert(Thread::current_or_null() == this, "current thread is wrong"); + // Perform tear-down actions + DEBUG_ONLY(_run_state = POST_RUN;) + this->post_run(); + + // Note: at this point the thread object may already have deleted itself, + // so from here on do not dereference *this*. Not all thread types currently + // delete themselves when they terminate. But no thread should ever be deleted + // asynchronously with respect to its termination - that is what _run_state can + // be used to check. + + assert(Thread::current_or_null() == NULL, "current thread still present"); } Thread::~Thread() { + + // Attached threads will remain in PRE_CALL_RUN, as will threads that don't actually + // get started due to errors etc. Any active thread should at least reach post_run + // before it is deleted (usually in post_run()). + assert(_run_state == PRE_CALL_RUN || + _run_state == POST_RUN, "Active Thread deleted before post_run()"); + // Notify the barrier set that a thread is being destroyed. Note that a barrier // set might not be available if we encountered errors during bootstrapping. BarrierSet* const barrier_set = BarrierSet::barrier_set(); @@ -440,9 +467,10 @@ // osthread() can be NULL, if creation of thread failed. if (osthread() != NULL) os::free_thread(osthread()); - // clear Thread::current if thread is deleting itself. + // Clear Thread::current if thread is deleting itself and it has not + // already been done. This must be done before the memory is deallocated. // Needed to ensure JNI correctly detects non-attached threads. - if (this == Thread::current()) { + if (this == Thread::current_or_null()) { Thread::clear_thread_current(); } @@ -1046,7 +1074,9 @@ } bool Thread::set_as_starting_thread() { + assert(_starting_thread == NULL, "already initialized"); // NOTE: this must be called inside the main thread. + DEBUG_ONLY(_starting_thread = this;) return os::create_main_thread((JavaThread*)this); } @@ -1248,28 +1278,59 @@ _current = OrderAccess::load_acquire(&_current->_next); } -NonJavaThread::NonJavaThread() : Thread(), _next(NULL) { - // Add this thread to _the_list. +// _next == this is used to indicate not yet in list. +NonJavaThread::NonJavaThread() : Thread(), _next(this) { + // During bootstrap, before barrier set creation, record threads in + // the list so barrier set creation can find them. + if (BarrierSet::barrier_set() == NULL) { + assert(Thread::current_or_null() == _starting_thread, + "only the main thread should create other threads during startup"); + add_to_the_list(); + } +} + +NonJavaThread::~NonJavaThread() { } + +void NonJavaThread::add_to_the_list() { MutexLockerEx lock(NonJavaThreadsList_lock, Mutex::_no_safepoint_check_flag); _next = _the_list._head; OrderAccess::release_store(&_the_list._head, this); } -NonJavaThread::~NonJavaThread() { - JFR_ONLY(Jfr::on_thread_exit(this);) - // Remove this thread from _the_list. +void NonJavaThread::remove_from_the_list() { MutexLockerEx lock(NonJavaThreadsList_lock, Mutex::_no_safepoint_check_flag); NonJavaThread* volatile* p = &_the_list._head; for (NonJavaThread* t = *p; t != NULL; p = &t->_next, t = *p) { if (t == this) { *p = this->_next; - // Wait for any in-progress iterators. + // Wait for any in-progress iterators. Concurrent synchronize is + // not allowed, so do it while holding the list lock. _the_list._protect.synchronize(); break; } } } +void NonJavaThread::pre_run() { + // Add to the list if not already present. During startup + // threads will be added at construction time. + if (_next == this) { + assert(BarrierSet::barrier_set() != NULL, "invariant"); + add_to_the_list(); + } + // This is slightly odd in that NamedThread is a subclass, but + // in fact name() is defined in Thread + assert(this->name() != NULL, "thread name was not set before it was started"); + this->set_native_thread_name(this->name()); +} + +void NonJavaThread::post_run() { + JFR_ONLY(Jfr::on_thread_exit(this);) + remove_from_the_list(); + // Ensure thread-local-storage is cleared before termination. + Thread::clear_thread_current(); +} + // NamedThread -- non-JavaThread subclasses with multiple // uniquely named instances should derive from this. NamedThread::NamedThread() : @@ -1296,10 +1357,6 @@ va_end(ap); } -void NamedThread::initialize_named_thread() { - set_native_thread_name(name()); -} - void NamedThread::print_on(outputStream* st) const { st->print("\"%s\" ", name()); Thread::print_on(st); @@ -1396,7 +1453,6 @@ void WatcherThread::run() { assert(this == watcher_thread(), "just checking"); - this->set_native_thread_name(this->name()); this->set_active_handles(JNIHandleBlock::allocate_block()); while (true) { assert(watcher_thread() == Thread::current(), "thread consistency check"); @@ -1752,19 +1808,30 @@ } -// The first routine called by a new Java thread +// First JavaThread specific code executed by a new Java thread. +void JavaThread::pre_run() { + // empty - see comments in run() +} + +// The main routine called by a new Java thread. This isn't overridden +// by subclasses, instead different subclasses define a different "entry_point" +// which defines the actual logic for that kind of thread. void JavaThread::run() { // initialize thread-local alloc buffer related fields this->initialize_tlab(); - // used to test validity of stack trace backs + // Used to test validity of stack trace backs. + // This can't be moved into pre_run() else we invalidate + // the requirement that thread_main_inner is lower on + // the stack. Consequently all the initialization logic + // stays here in run() rather than pre_run(). this->record_base_of_stack_pointer(); this->create_stack_guard_pages(); this->cache_global_variables(); - // Thread is now sufficient initialized to be handled by the safepoint code as being + // Thread is now sufficiently initialized to be handled by the safepoint code as being // in the VM. Change thread state from _thread_new to _thread_in_vm ThreadStateTransition::transition_and_fence(this, _thread_new, _thread_in_vm); @@ -1783,13 +1850,10 @@ } // We call another function to do the rest so we are sure that the stack addresses used - // from there will be lower than the stack base just computed + // from there will be lower than the stack base just computed. thread_main_inner(); - - // Note, thread is no longer valid at this point! } - void JavaThread::thread_main_inner() { assert(JavaThread::current() == this, "sanity check"); assert(this->threadObj() != NULL, "just checking"); @@ -1810,9 +1874,14 @@ DTRACE_THREAD_PROBE(stop, this); this->exit(false); - this->smr_delete(); } +// Shared teardown for all JavaThreads +void JavaThread::post_run() { + // Defer deletion to here to ensure 'this' is still referenceable in call_run + // for any shared tear-down. + this->smr_delete(); +} static void ensure_join(JavaThread* thread) { // We do not need to grab the Threads_lock, since we are operating on ourself.