8148992: VM can hang on exit if root region scanning is initiated but not executed
authorbrutisso
Wed, 10 Feb 2016 12:56:55 +0100
changeset 36084 9a3bf78e9a76
parent 35949 02676622ca27
child 36085 222ab7d1a9bf
8148992: VM can hang on exit if root region scanning is initiated but not executed Reviewed-by: tschatzl, pliden, jwilhelm
hotspot/src/share/vm/gc/g1/concurrentMarkThread.cpp
hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp
hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp
hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp
--- a/hotspot/src/share/vm/gc/g1/concurrentMarkThread.cpp	Mon Feb 08 18:26:27 2016 +0300
+++ b/hotspot/src/share/vm/gc/g1/concurrentMarkThread.cpp	Wed Feb 10 12:56:55 2016 +0100
@@ -23,6 +23,7 @@
  */
 
 #include "precompiled.hpp"
+#include "classfile/classLoaderData.hpp"
 #include "gc/g1/concurrentMarkThread.inline.hpp"
 #include "gc/g1/g1CollectedHeap.inline.hpp"
 #include "gc/g1/g1CollectorPolicy.hpp"
@@ -123,6 +124,7 @@
     // wait until started is set.
     sleepBeforeNextCycle();
     if (_should_terminate) {
+      _cm->root_regions()->cancel_scan();
       break;
     }
 
@@ -132,6 +134,11 @@
       HandleMark   hm;
       double cycle_start = os::elapsedVTime();
 
+      {
+        GCConcPhaseTimer(_cm, "Concurrent Clearing of Claimed Marks");
+        ClassLoaderDataGraph::clear_claimed_marks();
+      }
+
       // We have to ensure that we finish scanning the root regions
       // before the next GC takes place. To ensure this we have to
       // make sure that we do not join the STS until the root regions
@@ -140,7 +147,7 @@
       // without the root regions have been scanned which would be a
       // correctness issue.
 
-      if (!cm()->has_aborted()) {
+      {
         GCConcPhaseTimer(_cm, "Concurrent Root Region Scanning");
         _cm->scanRootRegions();
       }
--- a/hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp	Mon Feb 08 18:26:27 2016 +0300
+++ b/hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp	Wed Feb 10 12:56:55 2016 +0100
@@ -1290,8 +1290,7 @@
       ref_processor_cm()->verify_no_references_recorded();
 
       // Abandon current iterations of concurrent marking and concurrent
-      // refinement, if any are in progress. We have to do this before
-      // wait_until_scan_finished() below.
+      // refinement, if any are in progress.
       concurrent_mark()->abort();
 
       // Make sure we'll choose a new allocation region afterwards.
--- a/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp	Mon Feb 08 18:26:27 2016 +0300
+++ b/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp	Wed Feb 10 12:56:55 2016 +0100
@@ -372,6 +372,16 @@
   return res;
 }
 
+void G1CMRootRegions::notify_scan_done() {
+  MutexLockerEx x(RootRegionScan_lock, Mutex::_no_safepoint_check_flag);
+  _scan_in_progress = false;
+  RootRegionScan_lock->notify_all();
+}
+
+void G1CMRootRegions::cancel_scan() {
+  notify_scan_done();
+}
+
 void G1CMRootRegions::scan_finished() {
   assert(scan_in_progress(), "pre-condition");
 
@@ -381,11 +391,7 @@
   }
   _next_survivor = NULL;
 
-  {
-    MutexLockerEx x(RootRegionScan_lock, Mutex::_no_safepoint_check_flag);
-    _scan_in_progress = false;
-    RootRegionScan_lock->notify_all();
-  }
+  notify_scan_done();
 }
 
 bool G1CMRootRegions::wait_until_scan_finished() {
@@ -978,13 +984,11 @@
 };
 
 void G1ConcurrentMark::scanRootRegions() {
-  // Start of concurrent marking.
-  ClassLoaderDataGraph::clear_claimed_marks();
-
   // scan_in_progress() will have been set to true only if there was
   // at least one root region to scan. So, if it's false, we
   // should not attempt to do any further work.
   if (root_regions()->scan_in_progress()) {
+    assert(!has_aborted(), "Aborting before root region scanning is finished not supported.");
     GCTraceConcTime(Info, gc) tt("Concurrent Root Region Scan");
 
     _parallel_marking_threads = calc_parallel_marking_threads();
--- a/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp	Mon Feb 08 18:26:27 2016 +0300
+++ b/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp	Wed Feb 10 12:56:55 2016 +0100
@@ -229,6 +229,8 @@
   volatile bool        _should_abort;
   HeapRegion* volatile _next_survivor;
 
+  void notify_scan_done();
+
 public:
   G1CMRootRegions();
   // We actually do most of the initialization in this method.
@@ -248,6 +250,8 @@
   // all have been claimed.
   HeapRegion* claim_next();
 
+  void cancel_scan();
+
   // Flag that we're done with root region scanning and notify anyone
   // who's waiting on it. If aborted is false, assume that all regions
   // have been claimed.