From 41f9c77a469b5af8ad9a04cc9081af77f519b45b Mon Sep 17 00:00:00 2001
From: ysr <unknown>
Date: Thu, 21 Oct 2010 17:29:24 -0700
Subject: [PATCH] 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

---
 .../concurrentMarkSweepThread.cpp                 | 15 ++++++++++-----
 .../concurrentMarkSweepThread.hpp                 |  6 ++++--
 src/share/vm/runtime/globals.hpp                  |  4 ++--
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.cpp b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.cpp
index f0033815b..f9823af67 100644
--- a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.cpp
+++ b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.cpp
@@ -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 @@ void ConcurrentMarkSweepThread::desynchronize(bool is_cms_thread) {
   }
 }
 
-// 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 @@ void ConcurrentMarkSweepThread::sleepBeforeNextCycle() {
       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 --git a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.hpp b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.hpp
index 26eac7c37..1390b20a1 100644
--- a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.hpp
+++ b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.hpp
@@ -120,8 +120,10 @@ class ConcurrentMarkSweepThread: public ConcurrentGCThread {
   }
 
   // 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 --git a/src/share/vm/runtime/globals.hpp b/src/share/vm/runtime/globals.hpp
index 600586554..1fa26ce7f 100644
--- a/src/share/vm/runtime/globals.hpp
+++ b/src/share/vm/runtime/globals.hpp
@@ -1585,7 +1585,7 @@ class CommandLineFlags {
           "(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 @@ class CommandLineFlags {
   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,                                             \
-- 
GitLab