Hi, I've been working on answers to these questions, so I'll start with
this one.
The nmethodLocker keeps the nmethod from being reclaimed (made_zombie or
memory released) by the sweeper, but the nmethod could be unloaded.
Unloading the nmethod clears the Method* _method field.
The post_compiled_method_load event needs the _method field to look at
things like inlining and ScopeDesc fields. If the nmethod is unloaded,
some of the oops are dead. There are "holder" oops that correspond to
the metadata in the nmethod. If these oops are dead, causing the
nmethod to get unloaded, then the metadata may not be valid.
So my change 02 looks for a NULL nmethod._method field to tell whether
we can post information about the nmethod.
There's code in nmethod.cpp like:
jmethodID nmethod::get_and_cache_jmethod_id() {
if (_jmethod_id == NULL) {
// Cache the jmethod_id since it can no longer be looked up once the
// method itself has been marked for unloading.
_jmethod_id = method()->jmethod_id();
}
return _jmethod_id;
}
Which was added when post_method_load and unload were turned into
deferred events.
I put more debugging in the bug to show this crash was from an unloaded
nmethod.
Coleen
On 11/15/19 4:45 PM, [email protected] wrote:
Hi Coleen,
I have some questions.
Both the compiler method load and unload are posted as deferred events.
Both events keep the nmethod alive until the ServiceThread processes
the event.
The implementation is:
JvmtiDeferredEvent
JvmtiDeferredEvent::compiled_method_load_event(nmethod* nm) {
. . .
// Keep the nmethod alive until the ServiceThread can process
// this deferred event.
nmethodLocker::lock_nmethod(nm);
return event;
}
JvmtiDeferredEvent
JvmtiDeferredEvent::compiled_method_unload_event(nmethod* nm,
jmethodID id, const void* code) {
. . .
// Keep the nmethod alive until the ServiceThread can process
// this deferred event. This will keep the memory for the
// generated code from being reused too early. We pass
// zombie_ok == true here so that our nmethod that was just
// made into a zombie can be locked.
nmethodLocker::lock_nmethod(nm, true /* zombie_ok */);
return event;
}
void JvmtiDeferredEvent::post() {
assert(ServiceThread::is_service_thread(Thread::current()),
"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: {
nmethod* nm = _event_data.compiled_method_unload.nm;
JvmtiExport::post_compiled_method_unload(
_event_data.compiled_method_unload.method_id,
_event_data.compiled_method_unload.code_begin);
// done with the deferred event so unlock the nmethod
nmethodLocker::unlock_nmethod(nm);
break;
}
. . .
}
}
Then I wonder how is it possible for the nmethod to be not alive here?:
2168 void JvmtiExport::post_compiled_method_load(nmethod *nm) {
. . .
2173 // It's not safe to look at metadata for unloaded methods.
2174 if (!nm->is_alive()) {
2175 return;
2176 }
At least, it lokks like something else is broken.
Do I miss something important here?
Thanks,
Serguei
On 11/14/19 5:15 PM, [email protected] wrote:
Summary: Don't post information which uses metadata from unloaded
nmethods
Tested tier1-3 and 100 times with test that failed (reproduced
failure without the fix).
open webrev at
http://cr.openjdk.java.net/~coleenp/2019/8173361.01/webrev
bug link https://bugs.openjdk.java.net/browse/JDK-8173361
Thanks,
Coleen
