review (S) for 6965671: fatal error: acquiring lock JNIGlobalHandle_lock/16 out of order with lock CodeCache_lock/1

[Leela Mohan]

This is super late reply to this review mail but i have couple of questions
about this change.

Looking at the diff here :
http://hg.openjdk.java.net/jdk7/jdk7/hotspot/rev/65b0c03b165d

I think, Calling JvmtiExport::post_compiled_method_load(JvmtiEnv* env,..)
to JvmtiExport::post_compiled_method_load(nmethod*) versio is possibly
incorrect since, this would do callbacks to multiple jvmti environments
(jvmti agents) which is unexpected. Callbacks are expected to be called to
current jvmti env which called GenerateEvents.






On Thu, Jul 1, 2010 at 4:07 PM Tom Rodriguez <tom.rodriguez at oracle.com>
wrote:

> Thanks.  Dan also reviewed this as I was developing it.
>
> tom
>
> On Jul 1, 2010, at 2:55 PM, Vladimir Kozlov wrote:
>
> > Thanks, Tom
> >
> > Changes are good to go.
> >
> > Vladimir
> >
> > On 7/1/10 2:41 PM, Tom Rodriguez wrote:
> >>
> >> On Jul 1, 2010, at 2:36 PM, Vladimir Kozlov wrote:
> >>
> >>> You did not explain your changes for cb->name().
> >>
> >> That was just a clean up I've been wanting to do.  There was a time
> then only some of the CodeBlob subclasses had name() methods but now it's
> part of CodeBlob so that logic is unnecessary.
> >>
> >>> Why we don't lock nmethod in sweeper during CodeCache walk but lock
> here?
> >>
> >> nmethodLocker protects you from the sweeper so the sweeper doesn't need
> protection.
> >>
> >> tom
> >>
> >>>
> >>> Thanks,
> >>> Vladimir
> >>>
> >>> On 7/1/10 1:58 PM, Tom Rodriguez wrote:
> >>>> http://cr.openjdk.java.net/~never/6965671/
> >>>>
> >>>> 6965671: fatal error: acquiring lock JNIGlobalHandle_lock/16 out of
> order with lock CodeCache_lock/1
> >>>> Reviewed-by:
> >>>>
> >>>> The jmethodID fix created a deadlock because of lock ordering when
> >>>> walking the CodeCache to generate CompiledMethodLoad events.  The walk
> >>>> is performed with the CodeCache_lock held but we need to acquire the
> >>>> JNIGlobalHandle_lock to make a jmethodID.  I considered switching back
> >>>> to collecting the methodOop instead of the jmethodID under the lock
> >>>> but inspection of the code made it clear that there's a preexisting
> >>>> problem with GC of the methodOops stored into the nmethodDesc.
> >>>> Instead I switched to a simpler implementation that holds the
> >>>> CodeCache_lock while walking the CodeCache but releases the lock to
> >>>> actually perform the notify.  This simplifies the code greatly as well
> >>>> as fixing the bug.
> >>>>
> >>>> Tested with NSK jvmti,jdi,jdb,hprof,jit,regression and JDI_REGRESSION
> >>>> tests.
> >>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20190607/c8ab8829/attachment.html>