8173361: various crashes in JvmtiExport::post_compiled_method_load
Don't post information that uses metadata from unloaded nmethods
Reviewed-by: eosterlund, dholmes, sspitsyn
diff --git a/src/hotspot/share/code/nmethod.cpp b/src/hotspot/share/code/nmethod.cpp
index a351158..4a4d406 100644
--- a/src/hotspot/share/code/nmethod.cpp
+++ b/src/hotspot/share/code/nmethod.cpp
@@ -1565,14 +1565,18 @@
// Transfer information from compilation to jvmti
void nmethod::post_compiled_method_load_event() {
- Method* moop = method();
+ // 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;
+
+ Method* m = method();
HOTSPOT_COMPILED_METHOD_LOAD(
- (char *) moop->klass_name()->bytes(),
- moop->klass_name()->utf8_length(),
- (char *) moop->name()->bytes(),
- moop->name()->utf8_length(),
- (char *) moop->signature()->bytes(),
- moop->signature()->utf8_length(),
+ (char *) m->klass_name()->bytes(),
+ m->klass_name()->utf8_length(),
+ (char *) m->name()->bytes(),
+ m->name()->utf8_length(),
+ (char *) m->signature()->bytes(),
+ m->signature()->utf8_length(),
insts_begin(), insts_size());
if (JvmtiExport::should_post_compiled_method_load() ||
diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp
index c19b13d..003121e 100644
--- a/src/hotspot/share/oops/instanceKlass.cpp
+++ b/src/hotspot/share/oops/instanceKlass.cpp
@@ -1980,7 +1980,7 @@
// we're single threaded or at a safepoint - no locking needed
get_jmethod_id_length_value(jmeths, idnum, &length, &id);
} else {
- MutexLocker ml(JmethodIdCreation_lock);
+ MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
get_jmethod_id_length_value(jmeths, idnum, &length, &id);
}
}
@@ -2030,7 +2030,7 @@
id = get_jmethod_id_fetch_or_update(idnum, new_id, new_jmeths,
&to_dealloc_id, &to_dealloc_jmeths);
} else {
- MutexLocker ml(JmethodIdCreation_lock);
+ MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
id = get_jmethod_id_fetch_or_update(idnum, new_id, new_jmeths,
&to_dealloc_id, &to_dealloc_jmeths);
}
diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp
index f34c368..e9ca287 100644
--- a/src/hotspot/share/prims/jvmtiExport.cpp
+++ b/src/hotspot/share/prims/jvmtiExport.cpp
@@ -2155,7 +2155,7 @@
int stackframe = 0;
for(ScopeDesc* sd = nm->scope_desc_at(p->real_pc(nm));sd != NULL;sd = sd->sender()) {
// sd->method() can be NULL for stubs but not for nmethods. To be completely robust, include an assert that we should never see a null sd->method()
- assert(sd->method() != NULL, "sd->method() cannot be null.");
+ guarantee(sd->method() != NULL, "sd->method() cannot be null.");
record->pcinfo[scope].methods[stackframe] = sd->method()->jmethod_id();
record->pcinfo[scope].bcis[stackframe] = sd->bci();
stackframe++;
@@ -2166,6 +2166,7 @@
}
void JvmtiExport::post_compiled_method_load(nmethod *nm) {
+ guarantee(!nm->is_unloading(), "nmethod isn't unloaded or unloading");
if (JvmtiEnv::get_phase() < JVMTI_PHASE_PRIMORDIAL) {
return;
}
diff --git a/src/hotspot/share/prims/jvmtiImpl.cpp b/src/hotspot/share/prims/jvmtiImpl.cpp
index fa6353e..5a07e0a 100644
--- a/src/hotspot/share/prims/jvmtiImpl.cpp
+++ b/src/hotspot/share/prims/jvmtiImpl.cpp
@@ -908,9 +908,6 @@
nmethod* nm) {
JvmtiDeferredEvent event = JvmtiDeferredEvent(TYPE_COMPILED_METHOD_LOAD);
event._event_data.compiled_method_load = nm;
- // Keep the nmethod alive until the ServiceThread can process
- // this deferred event.
- nmethodLocker::lock_nmethod(nm);
return event;
}
@@ -943,14 +940,12 @@
}
void JvmtiDeferredEvent::post() {
- assert(ServiceThread::is_service_thread(Thread::current()),
+ assert(Thread::current()->is_service_thread(),
"Service thread must post enqueued events");
switch(_type) {
case TYPE_COMPILED_METHOD_LOAD: {
nmethod* nm = _event_data.compiled_method_load;
JvmtiExport::post_compiled_method_load(nm);
- // done with the deferred event so unlock the nmethod
- nmethodLocker::unlock_nmethod(nm);
break;
}
case TYPE_COMPILED_METHOD_UNLOAD: {
@@ -980,6 +975,21 @@
}
}
+// Keep the nmethod for compiled_method_load from being unloaded.
+void JvmtiDeferredEvent::oops_do(OopClosure* f, CodeBlobClosure* cf) {
+ if (cf != NULL && _type == TYPE_COMPILED_METHOD_LOAD) {
+ cf->do_code_blob(_event_data.compiled_method_load);
+ }
+}
+
+// The sweeper calls this and marks the nmethods here on the stack so that
+// they cannot be turned into zombies while in the queue.
+void JvmtiDeferredEvent::nmethods_do(CodeBlobClosure* cf) {
+ if (cf != NULL && _type == TYPE_COMPILED_METHOD_LOAD) {
+ cf->do_code_blob(_event_data.compiled_method_load);
+ } // May add UNLOAD event but it doesn't work yet.
+}
+
JvmtiDeferredEventQueue::QueueNode* JvmtiDeferredEventQueue::_queue_tail = NULL;
JvmtiDeferredEventQueue::QueueNode* JvmtiDeferredEventQueue::_queue_head = NULL;
@@ -1029,3 +1039,15 @@
delete node;
return event;
}
+
+void JvmtiDeferredEventQueue::oops_do(OopClosure* f, CodeBlobClosure* cf) {
+ for(QueueNode* node = _queue_head; node != NULL; node = node->next()) {
+ node->event().oops_do(f, cf);
+ }
+}
+
+void JvmtiDeferredEventQueue::nmethods_do(CodeBlobClosure* cf) {
+ for(QueueNode* node = _queue_head; node != NULL; node = node->next()) {
+ node->event().nmethods_do(cf);
+ }
+}
diff --git a/src/hotspot/share/prims/jvmtiImpl.hpp b/src/hotspot/share/prims/jvmtiImpl.hpp
index 4734ae4..93d20cd 100644
--- a/src/hotspot/share/prims/jvmtiImpl.hpp
+++ b/src/hotspot/share/prims/jvmtiImpl.hpp
@@ -475,6 +475,10 @@
// Actually posts the event.
void post() NOT_JVMTI_RETURN;
+ // Sweeper support to keep nmethods from being zombied while in the queue.
+ void nmethods_do(CodeBlobClosure* cf);
+ // GC support to keep nmethod from being unloaded while in the queue.
+ void oops_do(OopClosure* f, CodeBlobClosure* cf);
};
/**
@@ -494,7 +498,7 @@
QueueNode(const JvmtiDeferredEvent& event)
: _event(event), _next(NULL) {}
- const JvmtiDeferredEvent& event() const { return _event; }
+ JvmtiDeferredEvent& event() { return _event; }
QueueNode* next() const { return _next; }
void set_next(QueueNode* next) { _next = next; }
@@ -508,6 +512,10 @@
static bool has_events() NOT_JVMTI_RETURN_(false);
static void enqueue(const JvmtiDeferredEvent& event) NOT_JVMTI_RETURN;
static JvmtiDeferredEvent dequeue() NOT_JVMTI_RETURN_(JvmtiDeferredEvent());
+ // Sweeper support to keep nmethods from being zombied while in the queue.
+ static void nmethods_do(CodeBlobClosure* cf);
+ // GC support to keep nmethod from being unloaded while in the queue.
+ static void oops_do(OopClosure* f, CodeBlobClosure* cf);
};
// Utility macro that checks for NULL pointers:
diff --git a/src/hotspot/share/runtime/mutexLocker.cpp b/src/hotspot/share/runtime/mutexLocker.cpp
index 5b5bc26..d33713a 100644
--- a/src/hotspot/share/runtime/mutexLocker.cpp
+++ b/src/hotspot/share/runtime/mutexLocker.cpp
@@ -244,7 +244,7 @@
Notification_lock = Service_lock;
}
- def(JmethodIdCreation_lock , PaddedMutex , leaf, true, _safepoint_check_always); // used for creating jmethodIDs.
+ def(JmethodIdCreation_lock , PaddedMutex , leaf, true, _safepoint_check_never); // used for creating jmethodIDs.
def(SystemDictionary_lock , PaddedMonitor, leaf, true, _safepoint_check_always);
def(ProtectionDomainSet_lock , PaddedMutex , leaf-1, true, _safepoint_check_never);
diff --git a/src/hotspot/share/runtime/serviceThread.cpp b/src/hotspot/share/runtime/serviceThread.cpp
index 6c92a84..ed599ac 100644
--- a/src/hotspot/share/runtime/serviceThread.cpp
+++ b/src/hotspot/share/runtime/serviceThread.cpp
@@ -46,6 +46,7 @@
#include "services/threadIdTable.hpp"
ServiceThread* ServiceThread::_instance = NULL;
+JvmtiDeferredEvent* ServiceThread::_jvmti_event = NULL;
void ServiceThread::initialize() {
EXCEPTION_MARK;
@@ -138,7 +139,9 @@
}
if (has_jvmti_events) {
+ // Get the event under the Service_lock
jvmti_event = JvmtiDeferredEventQueue::dequeue();
+ _jvmti_event = &jvmti_event;
}
}
@@ -151,7 +154,8 @@
}
if (has_jvmti_events) {
- jvmti_event.post();
+ _jvmti_event->post();
+ _jvmti_event = NULL; // reset
}
if (!UseNotificationThread) {
@@ -186,6 +190,26 @@
}
}
-bool ServiceThread::is_service_thread(Thread* thread) {
- return thread == _instance;
+void ServiceThread::oops_do(OopClosure* f, CodeBlobClosure* cf) {
+ JavaThread::oops_do(f, cf);
+ // The ServiceThread "owns" the JVMTI Deferred events, scan them here
+ // to keep them alive until they are processed.
+ if (cf != NULL) {
+ if (_jvmti_event != NULL) {
+ _jvmti_event->oops_do(f, cf);
+ }
+ MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
+ JvmtiDeferredEventQueue::oops_do(f, cf);
+ }
+}
+
+void ServiceThread::nmethods_do(CodeBlobClosure* cf) {
+ JavaThread::nmethods_do(cf);
+ if (cf != NULL) {
+ if (_jvmti_event != NULL) {
+ _jvmti_event->nmethods_do(cf);
+ }
+ MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
+ JvmtiDeferredEventQueue::nmethods_do(cf);
+ }
}
diff --git a/src/hotspot/share/runtime/serviceThread.hpp b/src/hotspot/share/runtime/serviceThread.hpp
index dacd203..5ce9a03 100644
--- a/src/hotspot/share/runtime/serviceThread.hpp
+++ b/src/hotspot/share/runtime/serviceThread.hpp
@@ -30,12 +30,13 @@
// A hidden from external view JavaThread for JVMTI compiled-method-load
// events, oop storage cleanup, and the maintainance of string, symbol,
// protection domain, and resolved method tables.
+class JvmtiDeferredEvent;
class ServiceThread : public JavaThread {
friend class VMStructs;
private:
-
static ServiceThread* _instance;
+ static JvmtiDeferredEvent* _jvmti_event;
static void service_thread_entry(JavaThread* thread, TRAPS);
ServiceThread(ThreadFunction entry_point) : JavaThread(entry_point) {};
@@ -45,9 +46,11 @@
// Hide this thread from external view.
bool is_hidden_from_external_view() const { return true; }
+ bool is_service_thread() const { return true; }
- // Returns true if the passed thread is the service thread.
- static bool is_service_thread(Thread* thread);
+ // GC support
+ void oops_do(OopClosure* f, CodeBlobClosure* cf);
+ void nmethods_do(CodeBlobClosure* cf);
};
#endif // SHARE_RUNTIME_SERVICETHREAD_HPP
diff --git a/src/hotspot/share/runtime/thread.hpp b/src/hotspot/share/runtime/thread.hpp
index 72c524a..c74993e 100644
--- a/src/hotspot/share/runtime/thread.hpp
+++ b/src/hotspot/share/runtime/thread.hpp
@@ -480,6 +480,7 @@
virtual bool is_Java_thread() const { return false; }
virtual bool is_Compiler_thread() const { return false; }
virtual bool is_Code_cache_sweeper_thread() const { return false; }
+ virtual bool is_service_thread() const { return false; }
virtual bool is_hidden_from_external_view() const { return false; }
virtual bool is_jvmti_agent_thread() const { return false; }
// True iff the thread can perform GC operations at a safepoint.
