RFR (S) 8173361: various crashes in JvmtiExport::post_compiled_method_load

[serguei.spitsyn at oracle.com]

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, coleen.phillimore at oracle.com 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20191115/daa95b1a/attachment.html>