8273695: Safepoint deadlock on VMOperation_lock

Reviewed-by: dcubed, pchilanomate, eosterlund

src/hotspot/share/runtime/safepointMechanism.hpp

@@ -45,15 +45,12 @@ class SafepointMechanism : public AllStatic {
 
   static address _polling_page;
 
-
   static inline void disarm_local_poll(JavaThread* thread);
 
   static inline bool global_poll();
 
   static void process(JavaThread *thread, bool allow_suspend);
 
-  static inline bool should_process_no_suspend(JavaThread* thread);
-
   static void default_initialize();
 
   static void pd_initialize() NOT_AIX({ default_initialize(); });

src/hotspot/share/runtime/safepointMechanism.inline.hpp

@@ -30,6 +30,7 @@
 #include "runtime/atomic.hpp"
 #include "runtime/handshake.hpp"
 #include "runtime/safepoint.hpp"
+#include "runtime/stackWatermarkSet.hpp"
 #include "runtime/thread.inline.hpp"
 
 // Caller is responsible for using a memory barrier if needed.
@@ -62,26 +63,29 @@ bool SafepointMechanism::global_poll() {
   return (SafepointSynchronize::_state != SafepointSynchronize::_not_synchronized);
 }
 
-bool SafepointMechanism::should_process_no_suspend(JavaThread* thread) {
-  if (global_poll() || thread->handshake_state()->has_a_non_suspend_operation()) {
-    return true;
-  } else {
-    // We ignore suspend requests if any and just check before returning if we need
-    // to fix the thread's oops and first few frames due to a possible safepoint.
-    StackWatermarkSet::on_safepoint(thread);
-    update_poll_values(thread);
-    OrderAccess::cross_modify_fence();
-    return false;
-  }
-}
-
 bool SafepointMechanism::should_process(JavaThread* thread, bool allow_suspend) {
   if (!local_poll_armed(thread)) {
     return false;
   } else if (allow_suspend) {
     return true;
   }
-  return should_process_no_suspend(thread);
+  //  We are armed but we should ignore suspend operations.
+  if (global_poll() || // Safepoint
+      thread->handshake_state()->has_a_non_suspend_operation() || // Non-suspend handshake
+      !StackWatermarkSet::processing_started(thread)) { // StackWatermark processing is not started
+    return true;
+  }
+
+  // It has boiled down to two possibilities:
+  // 1: We have nothing to process, this just a disarm poll.
+  // 2: We have a suspend handshake, which cannot be processed.
+  // We update the poll value in case of a disarm, to reduce false positives.
+  update_poll_values(thread);
+
+  // We are now about to avoid processing and thus no cross modify fence will be executed.
+  // In case a safepoint happened, while being blocked, we execute it here.
+  OrderAccess::cross_modify_fence();
+  return false;
 }
 
 void SafepointMechanism::process_if_requested(JavaThread* thread, bool allow_suspend) {

src/hotspot/share/runtime/stackWatermarkSet.cpp

@@ -128,6 +128,15 @@ void StackWatermarkSet::start_processing(JavaThread* jt, StackWatermarkKind kind
   // will always update the poll values when waking up from a safepoint.
 }
 
+bool StackWatermarkSet::processing_started(JavaThread* jt) {
+  for (StackWatermark* current = head(jt); current != NULL; current = current->next()) {
+    if (!current->processing_started()) {
+      return false;
+    }
+  }
+  return true;
+}
+
 void StackWatermarkSet::finish_processing(JavaThread* jt, void* context, StackWatermarkKind kind) {
   StackWatermark* watermark = get(jt, kind);
   if (watermark != NULL) {

src/hotspot/share/runtime/stackWatermarkSet.hpp

@@ -79,6 +79,9 @@ public:
   // Called to ensure that processing of the thread is started
   static void start_processing(JavaThread* jt, StackWatermarkKind kind);
 
+  // Returns true if all StackWatermarks have been started.
+  static bool processing_started(JavaThread* jt);
+
   // Called to finish the processing of a thread
   static void finish_processing(JavaThread* jt, void* context, StackWatermarkKind kind);
 

test/hotspot/jtreg/ProblemList.txt

@@ -85,11 +85,6 @@ gc/stress/gclocker/TestGCLockerWithParallel.java 8180622 generic-all
 gc/stress/gclocker/TestGCLockerWithG1.java 8180622 generic-all
 gc/stress/TestJNIBlockFullGC/TestJNIBlockFullGC.java 8192647 generic-all
 gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java 8241293 macosx-x64
-gc/stringdedup/TestStringDeduplicationAgeThreshold.java#Z 8273695 generic-all
-gc/stringdedup/TestStringDeduplicationPrintOptions.java#Z 8273695 generic-all
-gc/stringdedup/TestStringDeduplicationInterned.java#Z 8273695 generic-all
-gc/stringdedup/TestStringDeduplicationTableResize.java#Z 8273695 generic-all
-gc/stringdedup/TestStringDeduplicationYoungGC.java#Z 8273695 generic-all
 
 #############################################################################