# HG changeset patch
# User ysr
# Date 1287707364 25200
# Node ID cd3ef3fd20dd421ee2f1469d2cb0801bac314d2e
# Parent  72a161e62cc456e8190c77de580b10b7767fb806
6992998: CMSWaitDuration=0 causes hangs with +ExplicitGCInvokesConcurrent
Summary: Closed a timing hole during which concurrent full gc requests can be missed. The hole can increase the latency of the response to a full gc request by up to the value of CMSWaitDuration. If CMSWaitDuration=0 is, as currently, interpreted as an unbounded wait, suitable in certain tuning scenarios, the application can potentially hang. Made two obscure tunables, including CMSWaitDuration, manageable.
Reviewed-by: jcoomes, tonyp

diff -r 72a161e62cc4 -r cd3ef3fd20dd src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.cpp
--- a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.cpp	Sat Oct 16 17:12:19 2010 -0400
+++ b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.cpp	Thu Oct 21 17:29:24 2010 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2010, 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
@@ -272,12 +272,16 @@
   }
 }
 
-// Wait until the next synchronous GC or a timeout, whichever is earlier.
-void ConcurrentMarkSweepThread::wait_on_cms_lock(long t) {
+// Wait until the next synchronous GC, a concurrent full gc request,
+// or a timeout, whichever is earlier.
+void ConcurrentMarkSweepThread::wait_on_cms_lock(long t_millis) {
   MutexLockerEx x(CGC_lock,
                   Mutex::_no_safepoint_check_flag);
+  if (_should_terminate || _collector->_full_gc_requested) {
+    return;
+  }
   set_CMS_flag(CMS_cms_wants_token);   // to provoke notifies
-  CGC_lock->wait(Mutex::_no_safepoint_check_flag, t);
+  CGC_lock->wait(Mutex::_no_safepoint_check_flag, t_millis);
   clear_CMS_flag(CMS_cms_wants_token);
   assert(!CMS_flag_is_set(CMS_cms_has_token | CMS_cms_wants_token),
          "Should not be set");
@@ -289,7 +293,8 @@
       icms_wait();
       return;
     } else {
-      // Wait until the next synchronous GC or a timeout, whichever is earlier
+      // Wait until the next synchronous GC, a concurrent full gc
+      // request or a timeout, whichever is earlier.
       wait_on_cms_lock(CMSWaitDuration);
     }
     // Check if we should start a CMS collection cycle
diff -r 72a161e62cc4 -r cd3ef3fd20dd src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.hpp
--- a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.hpp	Sat Oct 16 17:12:19 2010 -0400
+++ b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.hpp	Thu Oct 21 17:29:24 2010 -0700
@@ -120,8 +120,10 @@
   }
 
   // Wait on CMS lock until the next synchronous GC
-  // or given timeout, whichever is earlier.
-  void    wait_on_cms_lock(long t); // milliseconds
+  // or given timeout, whichever is earlier. A timeout value
+  // of 0 indicates that there is no upper bound on the wait time.
+  // A concurrent full gc request terminates the wait.
+  void wait_on_cms_lock(long t_millis);
 
   // The CMS thread will yield during the work portion of its cycle
   // only when requested to.  Both synchronous and asychronous requests
diff -r 72a161e62cc4 -r cd3ef3fd20dd src/share/vm/runtime/globals.hpp
--- a/src/share/vm/runtime/globals.hpp	Sat Oct 16 17:12:19 2010 -0400
+++ b/src/share/vm/runtime/globals.hpp	Thu Oct 21 17:29:24 2010 -0700
@@ -1585,7 +1585,7 @@
           "(Temporary, subject to experimentation)"                         \
           "Nominal minimum work per abortable preclean iteration")          \
                                                                             \
-  product(intx, CMSAbortablePrecleanWaitMillis, 100,                        \
+  manageable(intx, CMSAbortablePrecleanWaitMillis, 100,                     \
           "(Temporary, subject to experimentation)"                         \
           " Time that we sleep between iterations when not given"           \
           " enough work per iteration")                                     \
@@ -1677,7 +1677,7 @@
   product(uintx, CMSWorkQueueDrainThreshold, 10,                            \
           "Don't drain below this size per parallel worker/thief")          \
                                                                             \
-  product(intx, CMSWaitDuration, 2000,                                      \
+  manageable(intx, CMSWaitDuration, 2000,                                   \
           "Time in milliseconds that CMS thread waits for young GC")        \
                                                                             \
   product(bool, CMSYield, true,                                             \