# HG changeset patch
# User pliden
# Date 1553798639 -3600
# Node ID ddd60ad787d48c33f4c067795520fb95d935f9f0
# Parent  37648a9c4a6a87b9dfb6ffe4697b36453e837dc0
8221394: Clean up ConcurrentGCThread
Reviewed-by: kbarrett, eosterlund

diff -r 37648a9c4a6a -r ddd60ad787d4 src/hotspot/share/gc/shared/concurrentGCThread.cpp
--- a/src/hotspot/share/gc/shared/concurrentGCThread.cpp	Thu Mar 28 19:39:14 2019 +0100
+++ b/src/hotspot/share/gc/shared/concurrentGCThread.cpp	Thu Mar 28 19:43:59 2019 +0100
@@ -23,72 +23,58 @@
  */
 
 #include "precompiled.hpp"
-#include "classfile/systemDictionary.hpp"
 #include "gc/shared/concurrentGCThread.hpp"
-#include "oops/instanceRefKlass.hpp"
-#include "oops/oop.inline.hpp"
 #include "runtime/init.hpp"
-#include "runtime/java.hpp"
-#include "runtime/javaCalls.hpp"
+#include "runtime/mutexLocker.hpp"
+#include "runtime/orderAccess.hpp"
 #include "runtime/os.hpp"
 
 ConcurrentGCThread::ConcurrentGCThread() :
-  _should_terminate(false), _has_terminated(false) {
-};
+    _should_terminate(false),
+    _has_terminated(false) {}
 
 void ConcurrentGCThread::create_and_start(ThreadPriority prio) {
   if (os::create_thread(this, os::cgc_thread)) {
-    // XXX: need to set this to low priority
-    // unless "aggressive mode" set; priority
-    // should be just less than that of VMThread.
     os::set_priority(this, prio);
-    if (!_should_terminate) {
-      os::start_thread(this);
-    }
-  }
-}
-
-void ConcurrentGCThread::initialize_in_thread() {
-  this->set_active_handles(JNIHandleBlock::allocate_block());
-  // From this time Thread::current() should be working.
-  assert(this == Thread::current(), "just checking");
-}
-
-void ConcurrentGCThread::terminate() {
-  assert(_should_terminate, "Should only be called on terminate request.");
-  // Signal that it is terminated
-  {
-    MutexLockerEx mu(Terminator_lock,
-                     Mutex::_no_safepoint_check_flag);
-    _has_terminated = true;
-    Terminator_lock->notify();
+    os::start_thread(this);
   }
 }
 
 void ConcurrentGCThread::run() {
-  initialize_in_thread();
+  // Setup handle area
+  set_active_handles(JNIHandleBlock::allocate_block());
+
+  // Wait for initialization to complete
   wait_init_completed();
 
   run_service();
 
-  terminate();
+  // Signal thread has terminated
+  MonitorLockerEx ml(Terminator_lock);
+  OrderAccess::release_store(&_has_terminated, true);
+  ml.notify_all();
 }
 
 void ConcurrentGCThread::stop() {
-  // it is ok to take late safepoints here, if needed
-  {
-    MutexLockerEx mu(Terminator_lock);
-    assert(!_has_terminated,   "stop should only be called once");
-    assert(!_should_terminate, "stop should only be called once");
-    _should_terminate = true;
-  }
+  assert(!should_terminate(), "Invalid state");
+  assert(!has_terminated(), "Invalid state");
+
+  // Signal thread to terminate
+  OrderAccess::release_store_fence(&_should_terminate, true);
 
   stop_service();
 
-  {
-    MutexLockerEx mu(Terminator_lock);
-    while (!_has_terminated) {
-      Terminator_lock->wait();
-    }
+  // Wait for thread to terminate
+  MonitorLockerEx ml(Terminator_lock);
+  while (!_has_terminated) {
+    ml.wait();
   }
 }
+
+bool ConcurrentGCThread::should_terminate() const {
+  return OrderAccess::load_acquire(&_should_terminate);
+}
+
+bool ConcurrentGCThread::has_terminated() const {
+  return OrderAccess::load_acquire(&_has_terminated);
+}
diff -r 37648a9c4a6a -r ddd60ad787d4 src/hotspot/share/gc/shared/concurrentGCThread.hpp
--- a/src/hotspot/share/gc/shared/concurrentGCThread.hpp	Thu Mar 28 19:39:14 2019 +0100
+++ b/src/hotspot/share/gc/shared/concurrentGCThread.hpp	Thu Mar 28 19:43:59 2019 +0100
@@ -26,48 +26,28 @@
 #define SHARE_GC_SHARED_CONCURRENTGCTHREAD_HPP
 
 #include "runtime/thread.hpp"
-#include "utilities/macros.hpp"
 
 class ConcurrentGCThread: public NamedThread {
-  friend class VMStructs;
-
-  bool volatile _should_terminate;
-  bool _has_terminated;
-
-  // Do initialization steps in the thread: record stack base and size,
-  // init thread local storage, set JNI handle block.
-  void initialize_in_thread();
-
-  // Wait until Universe::is_fully_initialized();
-  void wait_for_universe_init();
-
-  // Record that the current thread is terminating, and will do more
-  // concurrent work.
-  void terminate();
+private:
+  volatile bool _should_terminate;
+  volatile bool _has_terminated;
 
 protected:
-  // Create and start the thread (setting it's priority.)
   void create_and_start(ThreadPriority prio = NearMaxPriority);
 
-  // Do the specific GC work. Called by run() after initialization complete.
   virtual void run_service() = 0;
-
-  // Shut down the specific GC work. Called by stop() as part of termination protocol.
-  virtual void stop_service()  = 0;
+  virtual void stop_service() = 0;
 
 public:
   ConcurrentGCThread();
 
-  // Tester
-  bool is_ConcurrentGC_thread() const { return true; }
+  virtual bool is_ConcurrentGC_thread() const { return true; }
 
   virtual void run();
-
-  // shutdown following termination protocol
   virtual void stop();
 
-  bool should_terminate() { return _should_terminate; }
-  bool has_terminated()   { return _has_terminated; }
+  bool should_terminate() const;
+  bool has_terminated() const;
 };
 
 #endif // SHARE_GC_SHARED_CONCURRENTGCTHREAD_HPP
diff -r 37648a9c4a6a -r ddd60ad787d4 src/hotspot/share/runtime/thread.cpp
--- a/src/hotspot/share/runtime/thread.cpp	Thu Mar 28 19:39:14 2019 +0100
+++ b/src/hotspot/share/runtime/thread.cpp	Thu Mar 28 19:43:59 2019 +0100
@@ -1506,7 +1506,7 @@
   {
     MutexLockerEx mu(Terminator_lock, Mutex::_no_safepoint_check_flag);
     _watcher_thread = NULL;
-    Terminator_lock->notify();
+    Terminator_lock->notify_all();
   }
 }