8309637: runtime/handshake/HandshakeTimeoutTest.java fails with "has not cleared handshake op" and SIGILL

Reviewed-by: dholmes, coleenp

src/hotspot/share/runtime/java.cpp

@@ -471,9 +471,7 @@ void before_exit(JavaThread* thread, bool halt) {
 
   // Stop the WatcherThread. We do this before disenrolling various
   // PeriodicTasks to reduce the likelihood of races.
-  if (PeriodicTask::num_tasks() > 0) {
-    WatcherThread::stop();
-  }
+  WatcherThread::stop();
 
   // shut down the StatSampler task
   StatSampler::disengage();

src/hotspot/share/runtime/nonJavaThread.cpp

@@ -156,7 +156,7 @@ void NamedThread::print_on(outputStream* st) const {
 // timer interrupts exists on the platform.
 
 WatcherThread* WatcherThread::_watcher_thread   = nullptr;
-bool WatcherThread::_startable = false;
+bool WatcherThread::_run_all_tasks = false;
 volatile bool  WatcherThread::_should_terminate = false;
 
 WatcherThread::WatcherThread() : NonJavaThread() {
@@ -185,6 +185,11 @@ int WatcherThread::sleep() const {
     return 0;  // we did not sleep.
   }
 
+  if (!_run_all_tasks) {
+    ml.wait(100);
+    return 0;
+  }
+
   // remaining will be zero if there are no tasks,
   // causing the WatcherThread to sleep until a task is
   // enrolled
@@ -280,7 +285,10 @@ void WatcherThread::run() {
       break;
     }
 
-    PeriodicTask::real_time_tick(time_waited);
+    // Don't process enrolled tasks until VM is fully initialized.
+    if (_run_all_tasks) {
+      PeriodicTask::real_time_tick(time_waited);
+    }
   }
 
   // Signal that it is terminated
@@ -293,18 +301,16 @@ void WatcherThread::run() {
 }
 
 void WatcherThread::start() {
-  assert(PeriodicTask_lock->owned_by_self(), "PeriodicTask_lock required");
-
-  if (watcher_thread() == nullptr && _startable) {
-    _should_terminate = false;
-    // Create the single instance of WatcherThread
-    new WatcherThread();
-  }
+  MonitorLocker ml(PeriodicTask_lock);
+  _should_terminate = false;
+  // Create the single instance of WatcherThread
+  new WatcherThread();
 }
 
-void WatcherThread::make_startable() {
-  assert(PeriodicTask_lock->owned_by_self(), "PeriodicTask_lock required");
-  _startable = true;
+void WatcherThread::run_all_tasks() {
+  MonitorLocker ml(PeriodicTask_lock);
+  _run_all_tasks = true;
+  ml.notify();
 }
 
 void WatcherThread::stop() {

src/hotspot/share/runtime/nonJavaThread.hpp

@@ -110,7 +110,7 @@ class WatcherThread: public NonJavaThread {
  private:
   static WatcherThread* _watcher_thread;
 
-  static bool _startable;
+  static bool _run_all_tasks;
   // volatile due to at least one lock-free read
   volatile static bool _should_terminate;
  public:
@@ -137,9 +137,9 @@ class WatcherThread: public NonJavaThread {
   // Create and start the single instance of WatcherThread, or stop it on shutdown
   static void start();
   static void stop();
-  // Only allow start once the VM is sufficiently initialized
-  // Otherwise the first task to enroll will trigger the start
-  static void make_startable();
+  // Allow executing registered tasks once the VM is sufficiently
+  // initialized. Meanwhile only error reporting will be checked.
+  static void run_all_tasks();
  private:
   int sleep() const;
 };

src/hotspot/share/runtime/task.cpp

@@ -29,6 +29,7 @@
 #include "runtime/mutexLocker.hpp"
 #include "runtime/nonJavaThread.hpp"
 #include "runtime/task.hpp"
+#include "runtime/threads.hpp"
 #include "runtime/timer.hpp"
 
 int PeriodicTask::_num_tasks = 0;
@@ -95,10 +96,9 @@ void PeriodicTask::enroll() {
   }
 
   WatcherThread* thread = WatcherThread::watcher_thread();
+  assert(thread != nullptr || !Threads::is_vm_complete(), "vm created but no WatcherThread");
   if (thread != nullptr) {
     thread->unpark();
-  } else {
-    WatcherThread::start();
   }
 }
 

src/hotspot/share/runtime/threads.cpp

@@ -553,6 +553,10 @@ jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) {
     return status;
   }
 
+  // Create WatcherThread as soon as we can since we need it in case
+  // of hangs during error reporting.
+  WatcherThread::start();
+
   // Add main_thread to threads list to finish barrier setup with
   // on_thread_attach.  Should be before starting to build Java objects in
   // init_globals2, which invokes barriers.
@@ -792,19 +796,11 @@ jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) {
     CLEAR_PENDING_EXCEPTION;
   }
 
-  {
-    MutexLocker ml(PeriodicTask_lock);
-    // Make sure the WatcherThread can be started by WatcherThread::start()
-    // or by dynamic enrollment.
-    WatcherThread::make_startable();
-    // Start up the WatcherThread if there are any periodic tasks
-    // NOTE:  All PeriodicTasks should be registered by now. If they
-    //   aren't, late joiners might appear to start slowly (we might
-    //   take a while to process their first tick).
-    if (PeriodicTask::num_tasks() > 0) {
-      WatcherThread::start();
-    }
-  }
+  // Let WatcherThread run all registered periodic tasks now.
+  // NOTE:  All PeriodicTasks should be registered by now. If they
+  //   aren't, late joiners might appear to start slowly (we might
+  //   take a while to process their first tick).
+  WatcherThread::run_all_tasks();
 
   create_vm_timer.end();
 #ifdef ASSERT