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

[dean.long at oracle.com]

On 10/24/18 1:52 AM, Erik Österlund wrote:
> 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.
>

OK, I forgot that mutex's can't be locked recursively.

I found a clue to the reason for original 1998 change.  This was also 
added at the same time, to klassVtable::initialize_vtable():

   // We need the compile lock in order to force the super vtables and
   // methodOop code pointers to stay constant.
   // (Race scenario: compiler thread updates a vtable entry while we're 
copying
   // down entries from the superclass vtable.)
   assert_lock(Compile_lock);

By 1999, the vtable code changed a lot and the assert above went away, 
so it was probably just an oversight that the extra uses of Compile_lock 
weren't removed at the same time.

dl

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