RFR (S/M) JDK-8010196 NPG: Internal Error: Metaspace allocation lock -- possible deadlock

[Mikael Gerdin]

On 2013-04-10 13:14, Stefan Karlsson wrote:
> On 04/08/2013 03:49 PM, Mikael Gerdin wrote:
>> Hi
>>
>> The problem is that when running the G1 garbage collector a call to
>> objArrayOopDesc::obj_at_put can in rare cases cause the G1 dirty card
>> queue buffer to fill up and this will cause G1 to take
>> DirtyCardQ_CBL_mon/16 to return the full buffer and get a new one.
>>
>> Adding dependencies to a ClassLoaderData is currently protected by the
>> per-metaspace "Metaspace allocation lock"/5 (which protects more than
>> just allocations).
>>
>> Because I want to avoid messing around with the lock ordering I
>> suggest that we use an ObjectLocker on the _dependencies oop in
>> ClassLoaderData.
>> _dependencies is a 2-element objArrayOop, in effect it's an ad-hoc
>> linked list of ClassLoaders/Classes which must be kept alive by this CLD.
>>
>> Using an ObjectLocker on the head element of the linked list should be
>> at least as good as using the metaspace_lock(). There should not be
>> any new deadlock issues with VM mutexes since any application thread
>> could use a similar synchronized linked list construct.
>>
>> As a bonus I suggest that we factor out the dependency handling from
>> ClassLoaderData into a inner class: ClassLoaderData::Dependencies and
>> let Dependencies manage the head of the linked list by itself. This
>> should make it more clear that the dependency list is not guarded by
>> the metaspace lock anymore.
>>
>> Buglinks:
>> https://jbs.oracle.com/bugs/browse/JDK-8010196
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8010196
>>
>> Testing:
>> default jprt run
>> jdk/test/java/lang/invoke/BigArityTest.java (which could reproduce the
>> issue with -XX:+UseG1GC -XX:G1UpdateBufferSize=1)
>>
>> Webrevs:
>> Incremental:
>> http://cr.openjdk.java.net/~mgerdin/8010196/webrev.0-fix/webrev
>> http://cr.openjdk.java.net/~mgerdin/8010196/webrev.0-factor-out/webrev
>> http://cr.openjdk.java.net/~mgerdin/8010196/webrev.0-rename/webrev
>> Everything:
>> http://cr.openjdk.java.net/~mgerdin/8010196/webrev.0/webrev
>
> Looks good.

Thanks Stefan and Coleen for reviewing and David for "not reviewing but 
still looking at the code" :)

Coleen has asked me to look more closely at the error handling in the 
CLD initialization code. I'll do that as a separate CR.

/Mikael

>
> StefanK
>
>>
>> Thanks
>> /Mikael
>