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

[Vladimir Kozlov]

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.
>