8371990: Remove two second delayed OOME after GC shutdown

Reviewed-by: kbarrett, tschatzl, ayang, iwalulya

Author: stefank

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

@@ -478,11 +478,6 @@ HeapWord* G1CollectedHeap::attempt_allocation_slow(uint node_index, size_t word_
     log_trace(gc, alloc)("%s: Unsuccessfully scheduled collection allocating %zu words",
                          Thread::current()->name(), word_size);
 
-    if (is_shutting_down()) {
-      stall_for_vm_shutdown();
-      return nullptr;
-    }
-
     // Has the gc overhead limit been reached in the meantime? If so, this mutator
     // should receive null even when unsuccessfully scheduling a collection as well
     // for global consistency.
@@ -738,11 +733,6 @@ HeapWord* G1CollectedHeap::attempt_allocation_humongous(size_t word_size) {
     log_trace(gc, alloc)("%s: Unsuccessfully scheduled collection allocating %zu",
                          Thread::current()->name(), word_size);
 
-    if (is_shutting_down()) {
-      stall_for_vm_shutdown();
-      return nullptr;
-    }
-
     // Has the gc overhead limit been reached in the meantime? If so, this mutator
     // should receive null even when unsuccessfully scheduling a collection as well
     // for global consistency.
@@ -1645,6 +1635,10 @@ jint G1CollectedHeap::initialize() {
   return JNI_OK;
 }
 
+bool G1CollectedHeap::concurrent_mark_is_terminating() const {
+  return _cm_thread->should_terminate();
+}
+
 void G1CollectedHeap::stop() {
   // Stop all concurrent threads. We do this to make sure these threads
   // do not continue to execute and access resources (e.g. logging)
@@ -1965,8 +1959,8 @@ bool G1CollectedHeap::try_collect_concurrently(size_t allocation_word_size,
     }
 
     // If VMOp skipped initiating concurrent marking cycle because
-    // we're terminating, then we're done.
-    if (is_shutting_down()) {
+    // we're shutting down, then we're done.
+    if (op.is_shutting_down()) {
       LOG_COLLECT_CONCURRENTLY(cause, "skipped: terminating");
       return false;
     }

src/hotspot/share/gc/g1/g1CollectedHeap.hpp

@@ -917,6 +917,9 @@ class G1CollectedHeap : public CollectedHeap {
   // specified by the policy object.
   jint initialize() override;
 
+  // Returns whether concurrent mark threads (and the VM) are about to terminate.
+  bool concurrent_mark_is_terminating() const;
+
   void safepoint_synchronize_begin() override;
   void safepoint_synchronize_end() override;
 

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp

@@ -1883,7 +1883,7 @@ bool G1ConcurrentMark::concurrent_cycle_abort() {
   // nothing, but this situation should be extremely rare (a full gc after shutdown
   // has been signalled is already rare), and this work should be negligible compared
   // to actual full gc work.
-  if (!cm_thread()->in_progress() && !_g1h->is_shutting_down()) {
+  if (!cm_thread()->in_progress() && !_g1h->concurrent_mark_is_terminating()) {
     return false;
   }
 

src/hotspot/share/gc/g1/g1Policy.cpp

@@ -669,7 +669,6 @@ bool G1Policy::should_retain_evac_failed_region(uint index) const {
 }
 
 void G1Policy::record_pause_start_time() {
-  assert(!_g1h->is_shutting_down(), "Invariant!");
   Ticks now = Ticks::now();
   _cur_pause_start_sec = now.seconds();
 
@@ -1280,12 +1279,6 @@ void G1Policy::decide_on_concurrent_start_pause() {
   // concurrent start pause).
   assert(!collector_state()->in_concurrent_start_gc(), "pre-condition");
 
-  // We should not be starting a concurrent start pause if the concurrent mark
-  // thread is terminating.
-  if (_g1h->is_shutting_down()) {
-    return;
-  }
-
   if (collector_state()->initiate_conc_mark_if_possible()) {
     // We had noticed on a previous pause that the heap occupancy has
     // gone over the initiating threshold and we should start a

src/hotspot/share/gc/g1/g1RemSet.cpp

@@ -1018,14 +1018,8 @@ class G1MergeHeapRootsTask : public WorkerTask {
       // the pause occurs during the Concurrent Cleanup for Next Mark phase.
       // Only at that point the region's bitmap may contain marks while being in the collection
       // set at the same time.
-      //
-      // There is one exception: shutdown might have aborted the Concurrent Cleanup for Next
-      // Mark phase midway, which might have also left stale marks in old generation regions.
-      // There might actually have been scheduled multiple collections, but at that point we do
-      // not care that much about performance and just do the work multiple times if needed.
-      return (_g1h->collector_state()->clear_bitmap_in_progress() ||
-              _g1h->is_shutting_down()) &&
-              hr->is_old();
+      return _g1h->collector_state()->clear_bitmap_in_progress() &&
+             hr->is_old();
     }
 
   public:

src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp

@@ -344,11 +344,6 @@ HeapWord* ParallelScavengeHeap::mem_allocate_work(size_t size, bool is_tlab) {
         assert(is_in_or_null(op.result()), "result not in heap");
         return op.result();
       }
-
-      if (is_shutting_down()) {
-        stall_for_vm_shutdown();
-        return nullptr;
-      }
     }
 
     // Was the gc-overhead reached inside the safepoint? If so, this mutator

src/hotspot/share/gc/serial/serialHeap.cpp

@@ -337,11 +337,6 @@ HeapWord* SerialHeap::mem_allocate_work(size_t size, bool is_tlab) {
       break;
     }
 
-    if (is_shutting_down()) {
-      stall_for_vm_shutdown();
-      return nullptr;
-    }
-
     // Give a warning if we seem to be looping forever.
     if ((QueuedAllocationWarningCount > 0) &&
         (try_count % QueuedAllocationWarningCount == 0)) {

src/hotspot/share/gc/shared/collectedHeap.cpp

@@ -62,6 +62,8 @@
 
 class ClassLoaderData;
 
+bool CollectedHeap::_is_shutting_down = false;
+
 size_t CollectedHeap::_lab_alignment_reserve = SIZE_MAX;
 Klass* CollectedHeap::_filler_object_klass = nullptr;
 size_t CollectedHeap::_filler_array_max_size = 0;
@@ -386,11 +388,6 @@ MetaWord* CollectedHeap::satisfy_failed_metadata_allocation(ClassLoaderData* loa
       return op.result();
     }
 
-    if (is_shutting_down()) {
-      stall_for_vm_shutdown();
-      return nullptr;
-    }
-
     loop_count++;
     if ((QueuedAllocationWarningCount > 0) &&
         (loop_count % QueuedAllocationWarningCount == 0)) {
@@ -605,30 +602,20 @@ void CollectedHeap::post_initialize() {
   initialize_serviceability();
 }
 
-bool CollectedHeap::is_shutting_down() const {
-  return Universe::is_shutting_down();
+bool CollectedHeap::is_shutting_down() {
+  assert(Heap_lock->owned_by_self(), "Protected by this lock");
+  return _is_shutting_down;
 }
 
-void CollectedHeap::stall_for_vm_shutdown() {
-  assert(is_shutting_down(), "Precondition");
-  // Stall the thread (2 seconds) instead of an indefinite wait to avoid deadlock
-  // if the VM shutdown triggers a GC.
-  // The 2-seconds sleep is:
-  //   - long enough to keep daemon threads stalled, while the shutdown
-  //     sequence completes in the common case.
-  //   - short enough to avoid excessive stall time if the shutdown itself
-  //     triggers a GC.
-  JavaThread::current()->sleep(2 * MILLIUNITS);
-
-  ResourceMark rm;
-  log_warning(gc, alloc)("%s: Stall for VM-Shutdown timed out; allocation may fail with OOME", Thread::current()->name());
-}
+void CollectedHeap::initiate_shutdown() {
+  {
+    // Acquire the Heap_lock to synchronize with VM_Heap_Sync_Operations,
+    // which may depend on the value of _is_shutting_down flag.
+    MutexLocker hl(Heap_lock);
+    _is_shutting_down = true;
+  }
 
-void CollectedHeap::before_exit() {
   print_tracing_info();
-
-  // Stop any on-going concurrent work and prepare for exit.
-  stop();
 }
 
 size_t CollectedHeap::bootstrap_max_memory() const {

src/hotspot/share/gc/shared/collectedHeap.hpp

@@ -96,6 +96,8 @@ class CollectedHeap : public CHeapObj<mtGC> {
   friend class MemAllocator;
 
  private:
+  static bool _is_shutting_down;
+
   GCHeapLog*      _heap_log;
   GCMetaspaceLog* _metaspace_log;
 
@@ -209,11 +211,10 @@ class CollectedHeap : public CHeapObj<mtGC> {
   // Default implementation does nothing.
   virtual void print_tracing_info() const = 0;
 
+ public:
   // Stop any onging concurrent work and prepare for exit.
   virtual void stop() = 0;
 
- public:
-
   static inline size_t filler_array_max_size() {
     return _filler_array_max_size;
   }
@@ -245,14 +246,9 @@ class CollectedHeap : public CHeapObj<mtGC> {
   // This is the correct place to place such initialization methods.
   virtual void post_initialize();
 
-  bool is_shutting_down() const;
-
-  // If the VM is shutting down, we may have skipped VM_CollectForAllocation.
-  // In this case, stall the allocation request briefly in the hope that
-  // the VM shutdown completes before the allocation request returns.
-  void stall_for_vm_shutdown();
+  static bool is_shutting_down();
 
-  void before_exit();
+  void initiate_shutdown();
 
   // Stop and resume concurrent GC threads interfering with safepoint operations
   virtual void safepoint_synchronize_begin() {}

src/hotspot/share/gc/shared/gcVMOperations.cpp

@@ -92,6 +92,22 @@ static bool should_use_gclocker() {
   return UseSerialGC || UseParallelGC;
 }
 
+static void block_if_java_thread() {
+  Thread* thread = Thread::current();
+  if (thread->is_Java_thread()) {
+    // Block here and allow the shutdown to complete
+    while (true) {
+      // The call to wait has a few important effects:
+      // 1) Block forever (minus spurious wake-ups, hence the loop)
+      // 2) Release the Heap_lock, which is taken by the shutdown code
+      // 3) Transition to blocked state so that the final VM_Exit operation can be scheduled
+      Heap_lock->wait();
+    }
+  } else {
+    assert(thread->is_ConcurrentGC_thread(), "Unexpected thread type");
+  }
+}
+
 bool VM_GC_Operation::doit_prologue() {
   assert(_gc_cause != GCCause::_no_gc, "Illegal GCCause");
 
@@ -110,8 +126,15 @@ bool VM_GC_Operation::doit_prologue() {
   }
   VM_Heap_Sync_Operation::doit_prologue();
 
+  _is_shutting_down = CollectedHeap::is_shutting_down();
+  if (_is_shutting_down) {
+    // Block forever if a Java thread is triggering a GC after
+    // the GC has started to shut down.
+    block_if_java_thread();
+  }
+
   // Check invocations
-  if (skip_operation() || Universe::is_shutting_down()) {
+  if (skip_operation() || _is_shutting_down) {
     // skip collection
     Heap_lock->unlock();
     if (should_use_gclocker()) {