From 57e7e82fa1cfb82bee6ff242d9a730c42bde663f Mon Sep 17 00:00:00 2001 From: Patricio Chilano Mateo Date: Mon, 10 Jul 2023 19:09:27 +0000 Subject: [PATCH] 8309637: runtime/handshake/HandshakeTimeoutTest.java fails with "has not cleared handshake op" and SIGILL Reviewed-by: dholmes, coleenp --- src/hotspot/share/runtime/java.cpp | 4 +-- src/hotspot/share/runtime/nonJavaThread.cpp | 30 ++++++++++++--------- src/hotspot/share/runtime/nonJavaThread.hpp | 8 +++--- src/hotspot/share/runtime/task.cpp | 4 +-- src/hotspot/share/runtime/threads.cpp | 22 +++++++-------- 5 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/hotspot/share/runtime/java.cpp b/src/hotspot/share/runtime/java.cpp index 140d0281e2f..8746efebc21 100644 --- a/src/hotspot/share/runtime/java.cpp +++ b/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(); diff --git a/src/hotspot/share/runtime/nonJavaThread.cpp b/src/hotspot/share/runtime/nonJavaThread.cpp index 3fbfa080e1f..f537afcaa06 100644 --- a/src/hotspot/share/runtime/nonJavaThread.cpp +++ b/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() { diff --git a/src/hotspot/share/runtime/nonJavaThread.hpp b/src/hotspot/share/runtime/nonJavaThread.hpp index 7807af6e228..6d9095924d9 100644 --- a/src/hotspot/share/runtime/nonJavaThread.hpp +++ b/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; }; diff --git a/src/hotspot/share/runtime/task.cpp b/src/hotspot/share/runtime/task.cpp index d38c1d145d0..9eccd368d25 100644 --- a/src/hotspot/share/runtime/task.cpp +++ b/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(); } } diff --git a/src/hotspot/share/runtime/threads.cpp b/src/hotspot/share/runtime/threads.cpp index 4f6fcf76db1..302c3672ce0 100644 --- a/src/hotspot/share/runtime/threads.cpp +++ b/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