< prev index next >
src/hotspot/share/runtime/handshake.cpp
Print this page
rev 52283 : [mq]: 8212933
rev 52284 : 8212933: Thread-SMR: requesting a VM operation whilst holding a ThreadsListHandle can cause deadlocks
Reviewed-by: eosterlund, dcubed, sspitsyn
*** 39,60 ****
#include "utilities/preserveException.hpp"
class HandshakeOperation: public StackObj {
public:
virtual void do_handshake(JavaThread* thread) = 0;
- virtual void cancel_handshake(JavaThread* thread) = 0;
};
class HandshakeThreadsOperation: public HandshakeOperation {
static Semaphore _done;
ThreadClosure* _thread_cl;
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
void check_state() {
assert(!_done.trywait(), "Must be zero");
--- 39,57 ----
*** 119,137 ****
void doit() {
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) {
return;
}
if (!UseMembar) {
os::serialize_thread_states();
--- 116,130 ----
void doit() {
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;
! } else {
return;
}
if (!UseMembar) {
os::serialize_thread_states();
*** 145,168 ****
}
// 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.
_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();)
}
--- 138,150 ----
}
// 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);
_target->handshake_process_by_vmthread();
}
} while (!poll_for_completed_thread());
DEBUG_ONLY(_op->check_state();)
}
*** 177,188 ****
void doit() {
DEBUG_ONLY(_op->check_state();)
TraceTime timer("Performing operation (vmoperation doit)", TRACETIME_LOG(Info, handshake));
int number_of_threads_issued = 0;
! for (JavaThreadIteratorWithHandle jtiwh; JavaThread *thr = jtiwh.next(); ) {
set_handshake(thr);
number_of_threads_issued++;
}
if (number_of_threads_issued < 1) {
--- 159,171 ----
void doit() {
DEBUG_ONLY(_op->check_state();)
TraceTime timer("Performing operation (vmoperation doit)", TRACETIME_LOG(Info, handshake));
+ JavaThreadIteratorWithHandle jtiwh;
int number_of_threads_issued = 0;
! for ( ; JavaThread *thr = jtiwh.next(); ) {
set_handshake(thr);
number_of_threads_issued++;
}
if (number_of_threads_issued < 1) {
*** 208,219 ****
// by semaphores and we optimistically begin by working on the blocked threads
{
// 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);
! for (JavaThreadIteratorWithHandle jtiwh; 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();
}
}
--- 191,203 ----
// by semaphores and we optimistically begin by working on the blocked threads
{
// 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 ( ; 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();
}
}
*** 260,270 ****
--- 244,258 ----
void HandshakeThreadsOperation::do_handshake(JavaThread* thread) {
ResourceMark rm;
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));
+
+ // 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();
}
*** 304,319 ****
SafepointMechanism::disarm_local_poll_release(target);
}
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;
! }
CautiouslyPreserveExceptionMark pem(thread);
ThreadInVMForHandshake tivm(thread);
if (!_semaphore.trywait()) {
_semaphore.wait_with_safepoint_check(thread);
--- 292,302 ----
SafepointMechanism::disarm_local_poll_release(target);
}
void HandshakeState::process_self_inner(JavaThread* thread) {
assert(Thread::current() == thread, "should call from thread");
! assert(!thread->is_terminated(), "should not be a terminated thread");
CautiouslyPreserveExceptionMark pem(thread);
ThreadInVMForHandshake tivm(thread);
if (!_semaphore.trywait()) {
_semaphore.wait_with_safepoint_check(thread);
*** 325,362 ****
op->do_handshake(thread);
}
_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
// the Threads_lock held so an externally suspended thread cannot be
// 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();
}
static bool possibly_vmthread_can_process_handshake(JavaThread* target) {
// An externally suspended thread cannot be resumed while the
// Threads_lock is held so it is safe.
// Note that this method is allowed to produce false positives.
assert(Threads_lock->owned_by_self(), "Not holding Threads_lock.");
if (target->is_ext_suspended()) {
return true;
}
switch (target->thread_state()) {
case _thread_in_native:
// native threads are safe if they have no java stack or have walkable stack
return !target->has_last_Java_frame() || target->frame_anchor()->walkable();
--- 308,338 ----
op->do_handshake(thread);
}
_semaphore.signal();
}
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
// the Threads_lock held so an externally suspended thread cannot be
// 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_terminated();
}
static bool possibly_vmthread_can_process_handshake(JavaThread* target) {
// An externally suspended thread cannot be resumed while the
// Threads_lock is held so it is safe.
// Note that this method is allowed to produce false positives.
assert(Threads_lock->owned_by_self(), "Not holding Threads_lock.");
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
return !target->has_last_Java_frame() || target->frame_anchor()->walkable();
*** 379,388 ****
--- 355,366 ----
return false;
}
void HandshakeState::process_by_vmthread(JavaThread* target) {
assert(Thread::current()->is_VM_thread(), "should call from vm thread");
+ // Threads_lock must be held here, but that is assert()ed in
+ // possibly_vmthread_can_process_handshake().
if (!has_operation()) {
// JT has already cleared its handshake
return;
}
*** 400,410 ****
// If we own the semaphore at this point and while owning the semaphore
// can observe a safe state the thread cannot possibly continue without
// 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);
// Release the thread
}
--- 378,387 ----
< prev index next >