RFR: 8212682: Avoid holding Compile_lock when blocking for GC in ObjArrayKlass::allocate_objArray_klass()
Erik Österlund
erik.osterlund at oracle.com
Mon Nov 19 14:09:08 UTC 2018
Hi Dean,
May I interpret your commentary as Reviewed?
Thanks,
/Erik
On 2018-10-24 22:20, dean.long at oracle.com wrote:
> 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
>>>
>>
>
More information about the hotspot-dev
mailing list