# HG changeset patch # User dcubed # Date 1253675557 21600 # Node ID 67b89f5a5cac0a4c21c3d73ff3fb6116b5819791 # Parent 6734b8b1adf1c796dcaec81382ead48c6a329b0c 6876794: 4/4 sp07t002 hangs very intermittently Summary: remove over locking by VMThread on "is thread suspended?" check Reviewed-by: dholmes, acorn, andrew diff -r 6734b8b1adf1 -r 67b89f5a5cac hotspot/src/share/vm/runtime/safepoint.cpp --- a/hotspot/src/share/vm/runtime/safepoint.cpp Mon Sep 21 18:10:22 2009 -0400 +++ b/hotspot/src/share/vm/runtime/safepoint.cpp Tue Sep 22 21:12:37 2009 -0600 @@ -769,9 +769,23 @@ // to grab the Threads_lock which we own here, so a thread cannot be // resumed during safepoint synchronization. - // We check with locking because another thread that has not yet - // synchronized may be trying to suspend this one. - bool is_suspended = _thread->is_any_suspended_with_lock(); + // We check to see if this thread is suspended without locking to + // avoid deadlocking with a third thread that is waiting for this + // thread to be suspended. The third thread can notice the safepoint + // that we're trying to start at the beginning of its SR_lock->wait() + // call. If that happens, then the third thread will block on the + // safepoint while still holding the underlying SR_lock. We won't be + // able to get the SR_lock and we'll deadlock. + // + // We don't need to grab the SR_lock here for two reasons: + // 1) The suspend flags are both volatile and are set with an + // Atomic::cmpxchg() call so we should see the suspended + // state right away. + // 2) We're being called from the safepoint polling loop; if + // we don't see the suspended state on this iteration, then + // we'll come around again. + // + bool is_suspended = _thread->is_ext_suspended(); if (is_suspended) { roll_forward(_at_safepoint); return; diff -r 6734b8b1adf1 -r 67b89f5a5cac hotspot/src/share/vm/runtime/thread.cpp --- a/hotspot/src/share/vm/runtime/thread.cpp Mon Sep 21 18:10:22 2009 -0400 +++ b/hotspot/src/share/vm/runtime/thread.cpp Tue Sep 22 21:12:37 2009 -0600 @@ -1942,7 +1942,7 @@ MutexLockerEx ml(SR_lock(), Mutex::_no_safepoint_check_flag); - assert(!this->is_any_suspended(), + assert(!this->is_ext_suspended(), "a thread trying to self-suspend should not already be suspended"); if (this->is_suspend_equivalent()) { diff -r 6734b8b1adf1 -r 67b89f5a5cac hotspot/src/share/vm/runtime/thread.hpp --- a/hotspot/src/share/vm/runtime/thread.hpp Mon Sep 21 18:10:22 2009 -0400 +++ b/hotspot/src/share/vm/runtime/thread.hpp Tue Sep 22 21:12:37 2009 -0600 @@ -967,11 +967,6 @@ return (_suspend_flags & _ext_suspended) != 0; } - // legacy method that checked for either external suspension or vm suspension - bool is_any_suspended() const { - return is_ext_suspended(); - } - bool is_external_suspend_with_lock() const { MutexLockerEx ml(SR_lock(), Mutex::_no_safepoint_check_flag); return is_external_suspend(); @@ -997,10 +992,6 @@ return ret; } - bool is_any_suspended_with_lock() const { - MutexLockerEx ml(SR_lock(), Mutex::_no_safepoint_check_flag); - return is_any_suspended(); - } // utility methods to see if we are doing some kind of suspension bool is_being_ext_suspended() const { MutexLockerEx ml(SR_lock(), Mutex::_no_safepoint_check_flag);