RFR: 8212682: Avoid holding Compile_lock when blocking for GC in ObjArrayKlass::allocate_objArray_klass()

[Erik Österlund]

Hi Dean,

On 2018-10-23 22:27, dean.long at oracle.com wrote:
> Can allocate_objArray_klass() end up calling 
> SystemDictionary::add_to_hierarchy()?  If so, then you'll end up 
> locking Compile_lock anway, but in the wrong order with respect to 
> MultiArray_lock.

No, I can't see that allocate_objArray_klass() ever calls 
SystemDictionary::add_to_hierarchy(). If it did, we would assert that 
the Compile_lock is held; it never re-acquires the lock.

Thanks,
/Erik

> dl
>
> On 10/23/18 7:59 AM, Erik Österlund wrote:
>> Hi,
>>
>> We occasionally blockingly wait for GC in allocations in 
>> ObjArrayKlass::allocate_objArray_klass(), while holding the 
>> Compile_lock.
>> This is problematic for concurrent class unloading that needs to hold 
>> that lock in the unloading path. This introduces a potential deadlock 
>> situation.
>>
>> After staring enough at this code together with help from Coleen, I 
>> have convinced myself that the Compile_lock is in fact not needed in 
>> this path, and there is nothing to my knowledge it actually protects 
>> here. The vague comment next to the lock suggests its purpose is to 
>> protect vtable stubs. But it doesn't. There is a VtableStubs_lock for 
>> that, and I find no traces if the Compile_lock having anything to do 
>> with that.
>>
>> Coleen helped me trying to trace down where this locking code came 
>> from. It takes us back to before anyone was around, and does not seem 
>> to have changed in the past 2 decades, and its origins are a bit 
>> unclear. The theory is that perhaps vtable stubs used to be protected 
>> by the Compile_lock in ancient times, and the locking code is still 
>> around because nobody dared to touch it.
>>
>> Since both code analysis and mach5 suggest there is no reason for 
>> this lock to be held here, I propose to remove it with this patch.
>>
>> If anyone that has been around for more than 2 decades in HotSpot 
>> happens to know the reason why this locking was introduced, any 
>> commentary around that would be very appreciated.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8212682/webrev.00/
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8212682
>>
>> Thanks,
>> /Erik
>