changeset 1035:87770dcf831b

6876794: 4/4 sp07t002 hangs very intermittently Summary: remove over locking by VMThread on "is thread suspended?" check Reviewed-by: dholmes, acorn, andrew
author dcubed
date Tue, 22 Sep 2009 21:12:37 -0600
parents d72ba3205918
children 1af62b6ca0f9 528d98fe1037
files src/share/vm/runtime/safepoint.cpp src/share/vm/runtime/thread.cpp src/share/vm/runtime/thread.hpp
diffstat 3 files changed, 18 insertions(+), 13 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/runtime/safepoint.cpp	Mon Sep 21 18:10:22 2009 -0400
+++ b/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;
--- a/src/share/vm/runtime/thread.cpp	Mon Sep 21 18:10:22 2009 -0400
+++ b/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()) {
--- a/src/share/vm/runtime/thread.hpp	Mon Sep 21 18:10:22 2009 -0400
+++ b/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);