OpenJDK / jdk / jdk
changeset 57495:26bb0fe2270a
8235829: graal crashes with Zombie.java test
Summary: Start ServiceThread before compiler threads, and run nmethod barriers for zgc before adding to the service thread queues, or posting events from the java thread.
Reviewed-by: pliden, dholmes, rehn
author | coleenp |
---|---|
date | Wed, 18 Dec 2019 11:51:22 -0500 |
parents | 1c844398e041 |
children | 2069b4bfd23b |
files | src/hotspot/share/code/compiledMethod.cpp src/hotspot/share/code/compiledMethod.hpp src/hotspot/share/code/nmethod.cpp src/hotspot/share/prims/jvmtiCodeBlobEvents.cpp src/hotspot/share/prims/jvmtiImpl.cpp src/hotspot/share/prims/jvmtiImpl.hpp src/hotspot/share/prims/jvmtiThreadState.cpp src/hotspot/share/prims/jvmtiThreadState.hpp src/hotspot/share/runtime/serviceThread.cpp src/hotspot/share/runtime/thread.cpp src/hotspot/share/services/management.cpp test/hotspot/jtreg/serviceability/jvmti/CompiledMethodLoad/Zombie.java |
diffstat | 12 files changed, 91 insertions(+), 32 deletions(-) [+] |
line wrap: on
line diff
--- a/src/hotspot/share/code/compiledMethod.cpp Wed Dec 18 14:36:49 2019 +0300 +++ b/src/hotspot/share/code/compiledMethod.cpp Wed Dec 18 11:51:22 2019 -0500 @@ -549,6 +549,21 @@ return true; } +void CompiledMethod::run_nmethod_entry_barrier() { + BarrierSetNMethod* bs_nm = BarrierSet::barrier_set()->barrier_set_nmethod(); + if (bs_nm != NULL) { + // We want to keep an invariant that nmethods found through iterations of a Thread's + // nmethods found in safepoints have gone through an entry barrier and are not armed. + // By calling this nmethod entry barrier, it plays along and acts + // like any other nmethod found on the stack of a thread (fewer surprises). + nmethod* nm = as_nmethod_or_null(); + if (nm != NULL) { + bool alive = bs_nm->nmethod_entry_barrier(nm); + assert(alive, "should be alive"); + } + } +} + void CompiledMethod::cleanup_inline_caches(bool clean_all) { for (;;) { ICRefillVerifier ic_refill_verifier; @@ -557,18 +572,8 @@ return; } } - BarrierSetNMethod* bs_nm = BarrierSet::barrier_set()->barrier_set_nmethod(); - if (bs_nm != NULL) { - // We want to keep an invariant that nmethods found through iterations of a Thread's - // nmethods found in safepoints have gone through an entry barrier and are not armed. - // By calling this nmethod entry barrier from the sweeper, it plays along and acts - // like any other nmethod found on the stack of a thread (fewer surprises). - nmethod* nm = as_nmethod_or_null(); - if (nm != NULL) { - bool alive = bs_nm->nmethod_entry_barrier(nm); - assert(alive, "should be alive"); - } - } + // Call this nmethod entry barrier from the sweeper. + run_nmethod_entry_barrier(); InlineCacheBuffer::refill_ic_stubs(); } }
--- a/src/hotspot/share/code/compiledMethod.hpp Wed Dec 18 14:36:49 2019 +0300 +++ b/src/hotspot/share/code/compiledMethod.hpp Wed Dec 18 11:51:22 2019 -0500 @@ -366,6 +366,9 @@ virtual void clear_inline_caches(); void clear_ic_callsites(); + // Execute nmethod barrier code, as if entering through nmethod call. + void run_nmethod_entry_barrier(); + // Verify and count cached icholder relocations. int verify_icholder_relocations(); void verify_oop_relocations();
--- a/src/hotspot/share/code/nmethod.cpp Wed Dec 18 14:36:49 2019 +0300 +++ b/src/hotspot/share/code/nmethod.cpp Wed Dec 18 11:51:22 2019 -0500 @@ -1567,6 +1567,12 @@ // Transfer information from compilation to jvmti void nmethod::post_compiled_method_load_event(JvmtiThreadState* state) { + // Don't post this nmethod load event if it is already dying + // because the sweeper might already be deleting this nmethod. + if (is_not_entrant() && can_convert_to_zombie()) { + return; + } + // This is a bad time for a safepoint. We don't want // this nmethod to get unloaded while we're queueing the event. NoSafepointVerifier nsv; @@ -1585,15 +1591,16 @@ if (JvmtiExport::should_post_compiled_method_load()) { // Only post unload events if load events are found. set_load_reported(); - // Keep sweeper from turning this into zombie until it is posted. - mark_as_seen_on_stack(); - // If a JavaThread hasn't been passed in, let the Service thread // (which is a real Java thread) post the event JvmtiDeferredEvent event = JvmtiDeferredEvent::compiled_method_load_event(this); if (state == NULL) { + // Execute any barrier code for this nmethod as if it's called, since + // keeping it alive looks like stack walking. + run_nmethod_entry_barrier(); ServiceThread::enqueue_deferred_event(&event); } else { + // This enters the nmethod barrier outside in the caller. state->enqueue_event(&event); } }
--- a/src/hotspot/share/prims/jvmtiCodeBlobEvents.cpp Wed Dec 18 14:36:49 2019 +0300 +++ b/src/hotspot/share/prims/jvmtiCodeBlobEvents.cpp Wed Dec 18 11:51:22 2019 -0500 @@ -34,6 +34,7 @@ #include "prims/jvmtiExport.hpp" #include "prims/jvmtiThreadState.inline.hpp" #include "runtime/handles.inline.hpp" +#include "runtime/safepointVerifiers.hpp" #include "runtime/vmThread.hpp" // Support class to collect a list of the non-nmethod CodeBlobs in @@ -222,16 +223,22 @@ jvmtiError JvmtiCodeBlobEvents::generate_compiled_method_load_events(JvmtiEnv* env) { JvmtiThreadState* state = JvmtiThreadState::state_for(JavaThread::current()); { - // Walk the CodeCache notifying for live nmethods, don't release the CodeCache_lock - // because the sweeper may be running concurrently. - // Save events to the queue for posting outside the CodeCache_lock. - MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag); - // Iterate over non-profiled and profiled nmethods - NMethodIterator iter(NMethodIterator::only_alive_and_not_unloading); - while(iter.next()) { - nmethod* current = iter.method(); - current->post_compiled_method_load_event(state); + NoSafepointVerifier nsv; // safepoints are not safe while collecting methods to post. + { + // Walk the CodeCache notifying for live nmethods, don't release the CodeCache_lock + // because the sweeper may be running concurrently. + // Save events to the queue for posting outside the CodeCache_lock. + MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag); + // Iterate over non-profiled and profiled nmethods + NMethodIterator iter(NMethodIterator::only_alive_and_not_unloading); + while(iter.next()) { + nmethod* current = iter.method(); + current->post_compiled_method_load_event(state); + } } + + // Enter nmethod barrier code if present outside CodeCache_lock + state->run_nmethod_entry_barriers(); } // Now post all the events outside the CodeCache_lock.
--- a/src/hotspot/share/prims/jvmtiImpl.cpp Wed Dec 18 14:36:49 2019 +0300 +++ b/src/hotspot/share/prims/jvmtiImpl.cpp Wed Dec 18 11:51:22 2019 -0500 @@ -25,6 +25,7 @@ #include "precompiled.hpp" #include "classfile/symbolTable.hpp" #include "classfile/systemDictionary.hpp" +#include "code/nmethod.hpp" #include "interpreter/interpreter.hpp" #include "interpreter/oopMapCache.hpp" #include "jvmtifiles/jvmtiEnv.hpp" @@ -992,6 +993,12 @@ JvmtiExport::post_compiled_method_load(env, nm); } +void JvmtiDeferredEvent::run_nmethod_entry_barriers() { + if (_type == TYPE_COMPILED_METHOD_LOAD) { + _event_data.compiled_method_load->run_nmethod_entry_barrier(); + } +} + // Keep the nmethod for compiled_method_load from being unloaded. void JvmtiDeferredEvent::oops_do(OopClosure* f, CodeBlobClosure* cf) { @@ -1008,8 +1015,16 @@ } } + bool JvmtiDeferredEventQueue::has_events() { - return _queue_head != NULL; + // We save the queued events before the live phase and post them when it starts. + // This code could skip saving the events on the queue before the live + // phase and ignore them, but this would change how we do things now. + // Starting the service thread earlier causes this to be called before the live phase begins. + // The events on the queue should all be posted after the live phase so this is an + // ok check. Before the live phase, DynamicCodeGenerated events are posted directly. + // If we add other types of events to the deferred queue, this could get ugly. + return JvmtiEnvBase::get_phase() == JVMTI_PHASE_LIVE && _queue_head != NULL; } void JvmtiDeferredEventQueue::enqueue(JvmtiDeferredEvent event) { @@ -1057,6 +1072,13 @@ } } +void JvmtiDeferredEventQueue::run_nmethod_entry_barriers() { + for(QueueNode* node = _queue_head; node != NULL; node = node->next()) { + node->event().run_nmethod_entry_barriers(); + } +} + + void JvmtiDeferredEventQueue::oops_do(OopClosure* f, CodeBlobClosure* cf) { for(QueueNode* node = _queue_head; node != NULL; node = node->next()) { node->event().oops_do(f, cf);
--- a/src/hotspot/share/prims/jvmtiImpl.hpp Wed Dec 18 14:36:49 2019 +0300 +++ b/src/hotspot/share/prims/jvmtiImpl.hpp Wed Dec 18 11:51:22 2019 -0500 @@ -481,6 +481,7 @@ // Actually posts the event. void post() NOT_JVMTI_RETURN; void post_compiled_method_load_event(JvmtiEnv* env) NOT_JVMTI_RETURN; + void run_nmethod_entry_barriers() NOT_JVMTI_RETURN; // Sweeper support to keep nmethods from being zombied while in the queue. void nmethods_do(CodeBlobClosure* cf) NOT_JVMTI_RETURN; // GC support to keep nmethod from being unloaded while in the queue. @@ -522,6 +523,7 @@ // Post all events in the queue for the current Jvmti environment void post(JvmtiEnv* env) NOT_JVMTI_RETURN; void enqueue(JvmtiDeferredEvent event) NOT_JVMTI_RETURN; + void run_nmethod_entry_barriers(); // Sweeper support to keep nmethods from being zombied while in the queue. void nmethods_do(CodeBlobClosure* cf) NOT_JVMTI_RETURN;
--- a/src/hotspot/share/prims/jvmtiThreadState.cpp Wed Dec 18 14:36:49 2019 +0300 +++ b/src/hotspot/share/prims/jvmtiThreadState.cpp Wed Dec 18 11:51:22 2019 -0500 @@ -432,3 +432,8 @@ } } +void JvmtiThreadState::run_nmethod_entry_barriers() { + if (_jvmti_event_queue != NULL) { + _jvmti_event_queue->run_nmethod_entry_barriers(); + } +}
--- a/src/hotspot/share/prims/jvmtiThreadState.hpp Wed Dec 18 14:36:49 2019 +0300 +++ b/src/hotspot/share/prims/jvmtiThreadState.hpp Wed Dec 18 11:51:22 2019 -0500 @@ -398,6 +398,7 @@ // Thread local event queue, which doesn't require taking the Service_lock. void enqueue_event(JvmtiDeferredEvent* event); void post_events(JvmtiEnv* env); + void run_nmethod_entry_barriers(); }; class RedefineVerifyMark : public StackObj {
--- a/src/hotspot/share/runtime/serviceThread.cpp Wed Dec 18 14:36:49 2019 +0300 +++ b/src/hotspot/share/runtime/serviceThread.cpp Wed Dec 18 11:51:22 2019 -0500 @@ -48,7 +48,8 @@ ServiceThread* ServiceThread::_instance = NULL; JvmtiDeferredEvent* ServiceThread::_jvmti_event = NULL; // The service thread has it's own static deferred event queue. -// Events can be posted before the service thread is created. +// Events can be posted before JVMTI vm_start, so it's too early to call JvmtiThreadState::state_for +// to add this field to the per-JavaThread event queue. TODO: fix this sometime later JvmtiDeferredEventQueue ServiceThread::_jvmti_service_queue; void ServiceThread::initialize() { @@ -195,6 +196,10 @@ void ServiceThread::enqueue_deferred_event(JvmtiDeferredEvent* event) { MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag); + // If you enqueue events before the service thread runs, gc and the sweeper + // cannot keep the nmethod alive. This could be restricted to compiled method + // load and unload events, if we wanted to be picky. + assert(_instance != NULL, "cannot enqueue events before the service thread runs"); _jvmti_service_queue.enqueue(*event); Service_lock->notify_all(); }
--- a/src/hotspot/share/runtime/thread.cpp Wed Dec 18 14:36:49 2019 +0300 +++ b/src/hotspot/share/runtime/thread.cpp Wed Dec 18 11:51:22 2019 -0500 @@ -88,6 +88,7 @@ #include "runtime/safepoint.hpp" #include "runtime/safepointMechanism.inline.hpp" #include "runtime/safepointVerifiers.hpp" +#include "runtime/serviceThread.hpp" #include "runtime/sharedRuntime.hpp" #include "runtime/statSampler.hpp" #include "runtime/stubRoutines.hpp" @@ -3995,6 +3996,10 @@ Chunk::start_chunk_pool_cleaner_task(); } + // Start the service thread + // The service thread enqueues JVMTI deferred events and does various hashtable + // and other cleanups. Needs to start before the compilers start posting events. + ServiceThread::initialize(); // initialize compiler(s) #if defined(COMPILER1) || COMPILER2_OR_JVMCI
--- a/src/hotspot/share/services/management.cpp Wed Dec 18 14:36:49 2019 +0300 +++ b/src/hotspot/share/services/management.cpp Wed Dec 18 11:51:22 2019 -0500 @@ -46,7 +46,6 @@ #include "runtime/jniHandles.inline.hpp" #include "runtime/notificationThread.hpp" #include "runtime/os.hpp" -#include "runtime/serviceThread.hpp" #include "runtime/thread.inline.hpp" #include "runtime/threadSMR.hpp" #include "services/classLoadingService.hpp" @@ -147,8 +146,6 @@ } void Management::initialize(TRAPS) { - // Start the service thread - ServiceThread::initialize(); if (UseNotificationThread) { NotificationThread::initialize(); }
--- a/test/hotspot/jtreg/serviceability/jvmti/CompiledMethodLoad/Zombie.java Wed Dec 18 14:36:49 2019 +0300 +++ b/test/hotspot/jtreg/serviceability/jvmti/CompiledMethodLoad/Zombie.java Wed Dec 18 11:51:22 2019 -0500 @@ -26,9 +26,9 @@ * @bug 8212159 * @summary Generate compiled method load events without crashing * @run main/othervm/native -agentlib:CompiledZombie -Xcomp -XX:ReservedCodeCacheSize=20m Zombie - * - * The stress test that made this fail was -jar SwingSet2.jar from demos (without DISPLAY set so it exits) - */ + **/ + + // The stress test that made this fail was -jar SwingSet2.jar from demos (without DISPLAY set so it exits) public class Zombie { public static void main(java.lang.String[] unused) {