8215047: Task terminators do not complete termination in consistent state

Reviewed-by: tschatzl, minqi

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Red Hat, Inc. All rights reserved.
+ * Copyright (c) 2018, 2019, Red Hat, Inc. All rights reserved.
  *
  * This code is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License version 2 only, as
@@ -38,15 +38,17 @@ bool OWSTTaskTerminator::offer_termination(TerminatorTerminator* terminator) {
   // Single worker, done
   if (_n_threads == 1) {
     _offered_termination = 1;
+    assert(!peek_in_queue_set(), "Precondition");
     return true;
   }
 
   _blocker->lock_without_safepoint_check();
-  // All arrived, done
   _offered_termination++;
+  // All arrived, done
   if (_offered_termination == _n_threads) {
     _blocker->notify_all();
     _blocker->unlock();
+    assert(!peek_in_queue_set(), "Precondition");
     return true;
   }
 
@@ -59,6 +61,7 @@ bool OWSTTaskTerminator::offer_termination(TerminatorTerminator* terminator) {
 
       if (do_spin_master_work(terminator)) {
         assert(_offered_termination == _n_threads, "termination condition");
+        assert(!peek_in_queue_set(), "Precondition");
         return true;
       } else {
         _blocker->lock_without_safepoint_check();
@@ -68,12 +71,14 @@ bool OWSTTaskTerminator::offer_termination(TerminatorTerminator* terminator) {
 
       if (_offered_termination == _n_threads) {
         _blocker->unlock();
+        assert(!peek_in_queue_set(), "Precondition");
         return true;
       }
     }
 
     size_t tasks = tasks_in_queue_set();
     if (exit_termination(tasks, terminator)) {
+      assert_lock_strong(_blocker);
       _offered_termination--;
       _blocker->unlock();
       return false;
@@ -153,19 +158,24 @@ bool OWSTTaskTerminator::do_spin_master_work(TerminatorTerminator* terminator) {
       _total_peeks++;
 #endif
     size_t tasks = tasks_in_queue_set();
-    if (exit_termination(tasks, terminator)) {
+    bool exit = exit_termination(tasks, terminator);
+    {
       MonitorLockerEx locker(_blocker, Mutex::_no_safepoint_check_flag);
-      if (tasks >= _offered_termination - 1) {
-        locker.notify_all();
-      } else {
-        for (; tasks > 1; tasks--) {
-          locker.notify();
+      // Termination condition reached
+      if (_offered_termination == _n_threads) {
+        _spin_master = NULL;
+        return true;
+      } else if (exit) {
+        if (tasks >= _offered_termination - 1) {
+          locker.notify_all();
+        } else {
+          for (; tasks > 1; tasks--) {
+            locker.notify();
+          }
         }
+        _spin_master = NULL;
+        return false;
       }
-      _spin_master = NULL;
-      return false;
-    } else if (_offered_termination == _n_threads) {
-      return true;
     }
   }
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Red Hat, Inc. All rights reserved.
+ * Copyright (c) 2018, 2019, Red Hat, Inc. All rights reserved.
  *
  * This code is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License version 2 only, as
@@ -55,6 +55,7 @@ public:
   }
 
   virtual ~OWSTTaskTerminator() {
+    assert(_spin_master == NULL, "Should have been reset");
     assert(_blocker != NULL, "Can not be NULL");
     delete _blocker;
   }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2019, 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
@@ -118,6 +118,11 @@ ParallelTaskTerminator(uint n_threads, TaskQueueSetSuper* queue_set) :
   _queue_set(queue_set),
   _offered_termination(0) {}
 
+ParallelTaskTerminator::~ParallelTaskTerminator() {
+  assert(_offered_termination == 0 || !peek_in_queue_set(), "Precondition");
+  assert(_offered_termination == 0 || _offered_termination == _n_threads, "Terminated or aborted" );
+}
+
 bool ParallelTaskTerminator::peek_in_queue_set() {
   return _queue_set->peek();
 }
@@ -162,6 +167,7 @@ ParallelTaskTerminator::offer_termination(TerminatorTerminator* terminator) {
     assert(_offered_termination <= _n_threads, "Invariant");
     // Are all threads offering termination?
     if (_offered_termination == _n_threads) {
+      assert(!peek_in_queue_set(), "Precondition");
       return true;
     } else {
       // Look for more work.
@@ -211,9 +217,7 @@ ParallelTaskTerminator::offer_termination(TerminatorTerminator* terminator) {
 #endif
       if (peek_in_queue_set() ||
           (terminator != NULL && terminator->should_exit_termination())) {
-        Atomic::dec(&_offered_termination);
-        assert(_offered_termination < _n_threads, "Invariant");
-        return false;
+        return complete_or_exit_termination();
       }
     }
   }
@@ -229,6 +233,23 @@ void ParallelTaskTerminator::print_termination_counts() {
 }
 #endif
 
+bool ParallelTaskTerminator::complete_or_exit_termination() {
+  // If termination is ever reached, terminator should stay in such state,
+  // so that all threads see the same state
+  uint current_offered = _offered_termination;
+  uint expected_value;
+  do {
+    if (current_offered == _n_threads) {
+      assert(!peek_in_queue_set(), "Precondition");
+      return true;
+    }
+    expected_value = current_offered;
+  } while ((current_offered = Atomic::cmpxchg(current_offered - 1, &_offered_termination, current_offered)) != expected_value);
+
+  assert(_offered_termination < _n_threads, "Invariant");
+  return false;
+}
+
 void ParallelTaskTerminator::reset_for_reuse() {
   if (_offered_termination != 0) {
     assert(_offered_termination == _n_threads,

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

@@ -467,11 +467,18 @@ protected:
   virtual void yield();
   void sleep(uint millis);
 
+  // Called when exiting termination is requested.
+  // When the request is made, terminator may have already terminated
+  // (e.g. all threads are arrived and offered termination). In this case,
+  // it should ignore the request and complete the termination.
+  // Return true if termination is completed. Otherwise, return false.
+  bool complete_or_exit_termination();
 public:
 
   // "n_threads" is the number of threads to be terminated.  "queue_set" is a
   // queue sets of work queues of other threads.
   ParallelTaskTerminator(uint n_threads, TaskQueueSetSuper* queue_set);
+  virtual ~ParallelTaskTerminator();
 
   // The current thread has no work, and is ready to terminate if everyone
   // else is.  If returns "true", all threads are terminated.  If returns