6987703: iCMS: Intermittent hang with gc/gctests/CallGC/CallGC01 and +ExplicitGCInvokesConcurrent

Count enable_icms() and disable_icms() events so as to prevent inteference between concurrent calls, which can cause the iCMS thread to be left stranded in icms_wait() with an unserviced request and no young allocations to unwedge it.

Reviewed-by: jcoomes, poonam

hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2011, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1689,6 +1689,8 @@ void CMSCollector::request_full_gc(unsigned int full_gc_count) {
     MutexLockerEx y(CGC_lock, Mutex::_no_safepoint_check_flag);
     _full_gc_requested = true;
     CGC_lock->notify();   // nudge CMS thread
+  } else {
+    assert(gc_count > full_gc_count, "Error: causal loop");
   }
 }
 

hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2011, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -51,7 +51,7 @@ int  ConcurrentMarkSweepThread::_CMS_flag         = CMS_nil;
 volatile jint ConcurrentMarkSweepThread::_pending_yields      = 0;
 volatile jint ConcurrentMarkSweepThread::_pending_decrements  = 0;
 
-volatile bool ConcurrentMarkSweepThread::_icms_enabled   = false;
+volatile jint ConcurrentMarkSweepThread::_icms_disabled   = 0;
 volatile bool ConcurrentMarkSweepThread::_should_run     = false;
 // When icms is enabled, the icms thread is stopped until explicitly
 // started.
@@ -84,7 +84,7 @@ ConcurrentMarkSweepThread::ConcurrentMarkSweepThread(CMSCollector* collector)
     }
   }
   _sltMonitor = SLT_lock;
-  set_icms_enabled(CMSIncrementalMode);
+  assert(!CMSIncrementalMode || icms_is_enabled(), "Error");
 }
 
 void ConcurrentMarkSweepThread::run() {
@@ -341,11 +341,11 @@ void ConcurrentMarkSweepThread::stop_icms() {
 
 void ConcurrentMarkSweepThread::icms_wait() {
   assert(UseConcMarkSweepGC && CMSIncrementalMode, "just checking");
-  if (_should_stop && icms_enabled()) {
+  if (_should_stop && icms_is_enabled()) {
     MutexLockerEx x(iCMS_lock, Mutex::_no_safepoint_check_flag);
     trace_state("pause_icms");
     _collector->stats().stop_cms_timer();
-    while(!_should_run && icms_enabled()) {
+    while(!_should_run && icms_is_enabled()) {
       iCMS_lock->wait(Mutex::_no_safepoint_check_flag);
     }
     _collector->stats().start_cms_timer();

hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2011, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -40,7 +40,7 @@
 class ConcurrentMarkSweepGeneration;
 class CMSCollector;
 
-// The Concurrent Mark Sweep GC Thread (could be several in the future).
+// The Concurrent Mark Sweep GC Thread
 class ConcurrentMarkSweepThread: public ConcurrentGCThread {
   friend class VMStructs;
   friend class ConcurrentMarkSweepGeneration;   // XXX should remove friendship
@@ -55,8 +55,6 @@ class ConcurrentMarkSweepThread: public ConcurrentGCThread {
   static SurrogateLockerThread::SLT_msg_type _sltBuffer;
   static Monitor*                       _sltMonitor;
 
-  ConcurrentMarkSweepThread*            _next;
-
   static bool _should_terminate;
 
   enum CMS_flag_type {
@@ -84,7 +82,7 @@ class ConcurrentMarkSweepThread: public ConcurrentGCThread {
   // Tracing messages, enabled by CMSTraceThreadState.
   static inline void trace_state(const char* desc);
 
-  static volatile bool _icms_enabled;   // iCMS enabled?
+  static volatile int _icms_disabled;   // a counter to track #iCMS disable & enable
   static volatile bool _should_run;     // iCMS may run
   static volatile bool _should_stop;    // iCMS should stop
 
@@ -214,10 +212,25 @@ class ConcurrentMarkSweepThread: public ConcurrentGCThread {
 
   // Incremental mode is enabled globally by the flag CMSIncrementalMode.  It
   // must also be enabled/disabled dynamically to allow foreground collections.
-  static inline void enable_icms()              { _icms_enabled = true; }
+#define ICMS_ENABLING_ASSERT                                                      \
-  static inline void disable_icms()             { _icms_enabled = false; }
+          assert((CMSIncrementalMode  && _icms_disabled >= 0) ||                  \
-  static inline void set_icms_enabled(bool val) { _icms_enabled = val; }
+                 (!CMSIncrementalMode && _icms_disabled <= 0), "Error")
-  static inline bool icms_enabled()             { return _icms_enabled; }
+
+  static inline void enable_icms() {
+    ICMS_ENABLING_ASSERT;
+    Atomic::dec(&_icms_disabled);
+  }
+  static inline void disable_icms() {
+   ICMS_ENABLING_ASSERT;
+   Atomic::inc(&_icms_disabled);
+  }
+  static inline bool icms_is_disabled() {
+   ICMS_ENABLING_ASSERT;
+   return _icms_disabled > 0;
+  }
+  static inline bool icms_is_enabled() {
+   return !icms_is_disabled();
+  }
 };
 
 inline void ConcurrentMarkSweepThread::trace_state(const char* desc) {

hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/vmCMSOperations.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2011, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -192,14 +192,18 @@ void VM_GenCollectFullConcurrent::doit() {
          "total_collections() should be monotonically increasing");
 
   MutexLockerEx x(FullGCCount_lock, Mutex::_no_safepoint_check_flag);
+  assert(_full_gc_count_before <= gch->total_full_collections(), "Error");
   if (gch->total_full_collections() == _full_gc_count_before) {
-    // Disable iCMS until the full collection is done.
+    // Disable iCMS until the full collection is done, and
+    // remember that we did so.
     CMSCollector::disable_icms();
+    _disabled_icms = true;
     // In case CMS thread was in icms_wait(), wake it up.
     CMSCollector::start_icms();
     // Nudge the CMS thread to start a concurrent collection.
     CMSCollector::request_full_gc(_full_gc_count_before);
   } else {
+    assert(_full_gc_count_before < gch->total_full_collections(), "Error");
     FullGCCount_lock->notify_all();  // Inform the Java thread its work is done
   }
 }
@@ -259,6 +263,8 @@ void VM_GenCollectFullConcurrent::doit_epilogue() {
       FullGCCount_lock->wait(Mutex::_no_safepoint_check_flag);
     }
   }
-  // Enable iCMS back.
+  // Enable iCMS back if we disabled it earlier.
-  CMSCollector::enable_icms();
+  if (_disabled_icms) {
+    CMSCollector::enable_icms();
+  }
 }

hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/vmCMSOperations.hpp

@@ -128,11 +128,14 @@ class VM_CMS_Final_Remark: public VM_CMS_Operation {
 // VM operation to invoke a concurrent collection of the heap as a
 // GenCollectedHeap heap.
 class VM_GenCollectFullConcurrent: public VM_GC_Operation {
+  bool _disabled_icms;
  public:
   VM_GenCollectFullConcurrent(unsigned int gc_count_before,
                               unsigned int full_gc_count_before,
                               GCCause::Cause gc_cause)
-    : VM_GC_Operation(gc_count_before, gc_cause, full_gc_count_before, true /* full */) {
+    : VM_GC_Operation(gc_count_before, gc_cause, full_gc_count_before, true /* full */),
+      _disabled_icms(false)
+  {
     assert(FullGCCount_lock != NULL, "Error");
     assert(UseAsyncConcMarkSweepGC, "Else will hang caller");
   }