diff --git a/src/hotspot/share/runtime/handshake.cpp b/src/hotspot/share/runtime/handshake.cpp index f2498aa38dd2b..5641f3f9a9bab 100644 --- a/src/hotspot/share/runtime/handshake.cpp +++ b/src/hotspot/share/runtime/handshake.cpp @@ -502,6 +502,16 @@ bool HandshakeState::has_async_exception_operation(bool ThreadDeath_only) { } } +void HandshakeState::clean_async_exception_operation() { + while (has_async_exception_operation(/* ThreadDeath_only */ false)) { + MutexLocker ml(&_lock, Mutex::_no_safepoint_check_flag); + HandshakeOperation* op; + op = _queue.peek(async_exception_filter); + remove_op(op); + delete op; + } +} + bool HandshakeState::have_non_self_executable_operation() { assert(_handshakee != Thread::current(), "Must not be called by self"); assert(_lock.owned_by_self(), "Lock must be held"); diff --git a/src/hotspot/share/runtime/handshake.hpp b/src/hotspot/share/runtime/handshake.hpp index 321e691651b73..8ac5b7bc4742b 100644 --- a/src/hotspot/share/runtime/handshake.hpp +++ b/src/hotspot/share/runtime/handshake.hpp @@ -133,6 +133,7 @@ class HandshakeState { bool has_operation() { return !_queue.is_empty(); } bool has_operation(bool allow_suspend, bool check_async_exception); bool has_async_exception_operation(bool ThreadDeath_only); + void clean_async_exception_operation(); bool operation_pending(HandshakeOperation* op); diff --git a/src/hotspot/share/runtime/thread.cpp b/src/hotspot/share/runtime/thread.cpp index 768cf93ef6ccf..f8f55a567a392 100644 --- a/src/hotspot/share/runtime/thread.cpp +++ b/src/hotspot/share/runtime/thread.cpp @@ -1365,6 +1365,7 @@ static bool is_daemon(oop threadObj) { // cleanup_failed_attach_current_thread as well. void JavaThread::exit(bool destroy_vm, ExitType exit_type) { assert(this == JavaThread::current(), "thread consistency check"); + assert(!is_exiting(), "should not be exiting or terminated already"); elapsedTimer _timer_exit_phase1; elapsedTimer _timer_exit_phase2; @@ -1426,18 +1427,23 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) { if (JvmtiExport::should_post_thread_life()) { JvmtiExport::post_thread_end(this); } - - // The careful dance between thread suspension and exit is handled here. - // Since we are in thread_in_vm state and suspension is done with handshakes, - // we can just put in the exiting state and it will be correctly handled. - set_terminated(_thread_exiting); - - ThreadService::current_thread_exiting(this, is_daemon(threadObj())); } else { - assert(!is_terminated() && !is_exiting(), "must not be exiting"); // before_exit() has already posted JVMTI THREAD_END events } + // Cleanup any pending async exception now since we cannot access oops after + // BarrierSet::barrier_set()->on_thread_detach() has been executed. + if (has_async_exception_condition()) { + handshake_state()->clean_async_exception_operation(); + } + + // The careful dance between thread suspension and exit is handled here. + // Since we are in thread_in_vm state and suspension is done with handshakes, + // we can just put in the exiting state and it will be correctly handled. + // Also, no more async exceptions will be added to the queue after this point. + set_terminated(_thread_exiting); + ThreadService::current_thread_exiting(this, is_daemon(threadObj())); + if (log_is_enabled(Debug, os, thread, timer)) { _timer_exit_phase1.stop(); _timer_exit_phase2.start(); @@ -1529,7 +1535,8 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) { } #endif // INCLUDE_JVMCI - // Remove from list of active threads list, and notify VM thread if we are the last non-daemon thread + // Remove from list of active threads list, and notify VM thread if we are the last non-daemon thread. + // We call BarrierSet::barrier_set()->on_thread_detach() here so no touching of oops after this point. Threads::remove(this, daemon); if (log_is_enabled(Debug, os, thread, timer)) { @@ -1699,8 +1706,9 @@ void JavaThread::handle_async_exception(oop java_throwable) { } void JavaThread::install_async_exception(AsyncExceptionHandshake* aeh) { - // Do not throw asynchronous exceptions against the compiler thread. - if (!can_call_java()) { + // Do not throw asynchronous exceptions against the compiler thread + // or if the thread is already exiting. + if (!can_call_java() || is_exiting()) { delete aeh; return; } diff --git a/test/hotspot/jtreg/runtime/Thread/StopAtExit.java b/test/hotspot/jtreg/runtime/Thread/StopAtExit.java index a9febfa0b55b5..39511446796e5 100644 --- a/test/hotspot/jtreg/runtime/Thread/StopAtExit.java +++ b/test/hotspot/jtreg/runtime/Thread/StopAtExit.java @@ -72,7 +72,30 @@ public static void main(String[] args) { usage(); } } + test(timeMax); + + // Fire-up deamon that just creates new threads. This generates contention on + // Threads_lock while worker tries to exit, creating more places where target + // can be seen as handshake safe. + Thread threadCreator = new Thread() { + @Override + public void run() { + while (true) { + Thread dummyThread = new Thread(() -> {}); + dummyThread.start(); + try { + dummyThread.join(); + } catch(InterruptedException ie) { + } + } + } + }; + threadCreator.setDaemon(true); + threadCreator.start(); + test(timeMax); + } + public static void test(int timeMax) { System.out.println("About to execute for " + timeMax + " seconds."); long count = 0; diff --git a/test/hotspot/jtreg/runtime/Thread/SuspendAtExit.java b/test/hotspot/jtreg/runtime/Thread/SuspendAtExit.java index 95e89010d9b1e..f693f23b1f8b6 100644 --- a/test/hotspot/jtreg/runtime/Thread/SuspendAtExit.java +++ b/test/hotspot/jtreg/runtime/Thread/SuspendAtExit.java @@ -78,7 +78,30 @@ public static void main(String[] args) { usage(); } } + test(timeMax); + + // Fire-up deamon that just creates new threads. This generates contention on + // Threads_lock while worker tries to exit, creating more places where target + // can be seen as handshake safe. + Thread threadCreator = new Thread() { + @Override + public void run() { + while (true) { + Thread dummyThread = new Thread(() -> {}); + dummyThread.start(); + try { + dummyThread.join(); + } catch(InterruptedException ie) { + } + } + } + }; + threadCreator.setDaemon(true); + threadCreator.start(); + test(timeMax); + } + public static void test(int timeMax) { System.out.println("About to execute for " + timeMax + " seconds."); long count = 0;