# HG changeset patch # User rehn # Date 1540472770 -7200 # Thu Oct 25 15:06:10 2018 +0200 # Node ID 1043db3551a0ef50ac0a97b1e339bd434b03ee4e # Parent 003c062e16ea8a3d8a9af5691e908006b1862cf8 [mq]: 8212933 diff --git a/src/hotspot/share/runtime/handshake.cpp b/src/hotspot/share/runtime/handshake.cpp --- a/src/hotspot/share/runtime/handshake.cpp +++ b/src/hotspot/share/runtime/handshake.cpp @@ -41,7 +41,6 @@ class HandshakeOperation: public StackObj { public: virtual void do_handshake(JavaThread* thread) = 0; - virtual void cancel_handshake(JavaThread* thread) = 0; }; class HandshakeThreadsOperation: public HandshakeOperation { @@ -51,8 +50,6 @@ public: HandshakeThreadsOperation(ThreadClosure* cl) : _thread_cl(cl) {} void do_handshake(JavaThread* thread); - void cancel_handshake(JavaThread* thread) { _done.signal(); }; - bool thread_has_completed() { return _done.trywait(); } #ifdef ASSERT @@ -121,15 +118,11 @@ DEBUG_ONLY(_op->check_state();) TraceTime timer("Performing single-target operation (vmoperation doit)", TRACETIME_LOG(Info, handshake)); - { - ThreadsListHandle tlh; - if (tlh.includes(_target)) { - set_handshake(_target); - _thread_alive = true; - } - } - - if (!_thread_alive) { + ThreadsListHandle tlh; + if (tlh.includes(_target)) { + set_handshake(_target); + _thread_alive = true; + } else { return; } @@ -147,20 +140,9 @@ // We need to re-think this with SMR ThreadsList. // There is an assumption in the code that the Threads_lock should be // locked during certain phases. - MutexLockerEx ml(Threads_lock, Mutex::_no_safepoint_check_flag); - ThreadsListHandle tlh; - if (tlh.includes(_target)) { - // Warning _target's address might be re-used. - // handshake_process_by_vmthread will check the semaphore for us again. - // Since we can't have more then one handshake in flight a reuse of - // _target's address should be okay since the new thread will not have - // an operation. + { + MutexLockerEx ml(Threads_lock, Mutex::_no_safepoint_check_flag); _target->handshake_process_by_vmthread(); - } else { - // We can't warn here since the thread does cancel_handshake after - // it has been removed from the ThreadsList. So we should just keep - // looping here until while below returns false. If we have a bug, - // then we hang here, which is good for debugging. } } while (!poll_for_completed_thread()); DEBUG_ONLY(_op->check_state();) @@ -179,8 +161,9 @@ DEBUG_ONLY(_op->check_state();) TraceTime timer("Performing operation (vmoperation doit)", TRACETIME_LOG(Info, handshake)); + JavaThreadIteratorWithHandle jtiwh; int number_of_threads_issued = 0; - for (JavaThreadIteratorWithHandle jtiwh; JavaThread *thr = jtiwh.next(); ) { + for ( ; JavaThread *thr = jtiwh.next(); ) { set_handshake(thr); number_of_threads_issued++; } @@ -210,8 +193,9 @@ // We need to re-think this with SMR ThreadsList. // There is an assumption in the code that the Threads_lock should // be locked during certain phases. + jtiwh.rewind(); MutexLockerEx ml(Threads_lock, Mutex::_no_safepoint_check_flag); - for (JavaThreadIteratorWithHandle jtiwh; JavaThread *thr = jtiwh.next(); ) { + for ( ; JavaThread *thr = jtiwh.next(); ) { // A new thread on the ThreadsList will not have an operation, // hence it is skipped in handshake_process_by_vmthread. thr->handshake_process_by_vmthread(); @@ -262,7 +246,11 @@ FormatBufferResource message("Operation for thread " PTR_FORMAT ", is_vm_thread: %s", p2i(thread), BOOL_TO_STR(Thread::current()->is_VM_thread())); TraceTime timer(message, TRACETIME_LOG(Debug, handshake, task)); - _thread_cl->do_thread(thread); + + // Only actually execute the operation for non terminated threads. + if (!thread->is_terminated()) { + _thread_cl->do_thread(thread); + } // Use the semaphore to inform the VM thread that we have completed the operation _done.signal(); @@ -306,12 +294,7 @@ void HandshakeState::process_self_inner(JavaThread* thread) { assert(Thread::current() == thread, "should call from thread"); - - if (thread->is_terminated()) { - // If thread is not on threads list but armed, cancel. - thread->cancel_handshake(); - return; - } + assert(!thread->is_terminated(), "should not be a terminated thread"); CautiouslyPreserveExceptionMark pem(thread); ThreadInVMForHandshake tivm(thread); @@ -327,16 +310,6 @@ _semaphore.signal(); } -void HandshakeState::cancel_inner(JavaThread* thread) { - assert(Thread::current() == thread, "should call from thread"); - assert(thread->thread_state() == _thread_in_vm, "must be in vm state"); - HandshakeOperation* op = _operation; - clear_handshake(thread); - if (op != NULL) { - op->cancel_handshake(thread); - } -} - bool HandshakeState::vmthread_can_process_handshake(JavaThread* target) { // SafepointSynchronize::safepoint_safe() does not consider an externally // suspended thread to be safe. However, this function must be called with @@ -344,7 +317,7 @@ // resumed thus it is safe. assert(Threads_lock->owned_by_self(), "Not holding Threads_lock."); return SafepointSynchronize::safepoint_safe(target, target->thread_state()) || - target->is_ext_suspended(); + target->is_ext_suspended() || target->is_terminated(); } static bool possibly_vmthread_can_process_handshake(JavaThread* target) { @@ -355,6 +328,9 @@ if (target->is_ext_suspended()) { return true; } + if (target->is_terminated()) { + return true; + } switch (target->thread_state()) { case _thread_in_native: // native threads are safe if they have no java stack or have walkable stack @@ -402,7 +378,6 @@ // getting caught by the semaphore. if (vmthread_can_process_handshake(target)) { guarantee(!_semaphore.trywait(), "we should already own the semaphore"); - _operation->do_handshake(target); // Disarm after VM thread have executed the operation. clear_handshake(target); diff --git a/src/hotspot/share/runtime/handshake.hpp b/src/hotspot/share/runtime/handshake.hpp --- a/src/hotspot/share/runtime/handshake.hpp +++ b/src/hotspot/share/runtime/handshake.hpp @@ -72,19 +72,13 @@ return _operation != NULL; } - void cancel(JavaThread* thread) { - if (!_thread_in_process_handshake) { - FlagSetting fs(_thread_in_process_handshake, true); - cancel_inner(thread); - } - } - void process_by_self(JavaThread* thread) { if (!_thread_in_process_handshake) { FlagSetting fs(_thread_in_process_handshake, true); process_self_inner(thread); } } + void process_by_vmthread(JavaThread* target); }; diff --git a/src/hotspot/share/runtime/thread.cpp b/src/hotspot/share/runtime/thread.cpp --- a/src/hotspot/share/runtime/thread.cpp +++ b/src/hotspot/share/runtime/thread.cpp @@ -4251,9 +4251,6 @@ before_exit(thread); thread->exit(true); - // thread will never call smr_delete, instead of implicit cancel - // in wait_for_vm_thread_exit we do it explicit. - thread->cancel_handshake(); // Stop VM thread. { diff --git a/src/hotspot/share/runtime/thread.hpp b/src/hotspot/share/runtime/thread.hpp --- a/src/hotspot/share/runtime/thread.hpp +++ b/src/hotspot/share/runtime/thread.hpp @@ -1266,10 +1266,6 @@ return _handshake.has_operation(); } - void cancel_handshake() { - _handshake.cancel(this); - } - void handshake_process_by_self() { _handshake.process_by_self(this); } diff --git a/src/hotspot/share/runtime/threadSMR.cpp b/src/hotspot/share/runtime/threadSMR.cpp --- a/src/hotspot/share/runtime/threadSMR.cpp +++ b/src/hotspot/share/runtime/threadSMR.cpp @@ -989,11 +989,6 @@ // Retry the whole scenario. } - if (ThreadLocalHandshakes) { - // The thread is about to be deleted so cancel any handshake. - thread->cancel_handshake(); - } - delete thread; if (EnableThreadSMRStatistics) { timer.stop();