RFR (S) 8213092: Add more runtime locks for concurrent class unloading

Doerr, Martin martin.doerr at sap.com
Tue Oct 30 14:09:12 UTC 2018


Hi Coleen,

I think the acquisition of MetaspaceExpand_lock should get moved to VirtualSpaceList::contains because find_enclosing_space is used during error reporting. If the VM crashes while the lock is held, error reporting will hang.

Best regards,
Martin


-----Original Message-----
From: hotspot-runtime-dev <hotspot-runtime-dev-bounces at openjdk.java.net> On Behalf Of coleen.phillimore at oracle.com
Sent: Dienstag, 30. Oktober 2018 14:21
To: hotspot-runtime-dev at openjdk.java.net
Subject: Re: RFR (S) 8213092: Add more runtime locks for concurrent class unloading



On 10/30/18 9:16 AM, coleen.phillimore at oracle.com wrote:
>
>
> On 10/30/18 8:36 AM, David Holmes wrote:
>> On 30/10/2018 10:24 PM, coleen.phillimore at oracle.com wrote:
>>> On 10/30/18 12:17 AM, David Holmes wrote:
>>>> Hi Coleen,
>>>>
>>>> On 30/10/2018 1:45 PM, coleen.phillimore at oracle.com wrote:
>>>>> Summary: Add locks for calling CLDG::purge concurrently as well 
>>>>> and for calling SystemDictionary::do_unloading concurrently.
>>>>>
>>>>> Ran linux-x64 tier1-6 through mach5 and hotspot/jtreg/runtime 
>>>>> tests, which include the module tests.
>>>>>
>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8213092.01/webrev
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8213092
>>>>
>>>> So ... all the locks covered by an assert_locked_or_safepoint, or 
>>>> which are acquired by the VMThread at a safepoint, must never be 
>>>> held by a JavaThread if it could reach a safepoint whilst that lock 
>>>> is held - else we could deadlock. So can we check that with 
>>>> NoSafepointVerifiers?
>>>
>>> Actually I think this is not possible to add NSV.   You can acquire 
>>> the ClassLoaderDataGraph_lock and then the Module_lock. The latter 
>>> would check for a safepoint also for a Java thread. This is 
>>> currently done for Jvmti and JFR, but not in other code that I can 
>>> see.  I don't actually know how to fix this problem.
>>
>> This seems risky. If a JavaThread can hold the CLDG_lock while 
>> blocked at a safepoint (acquiring the Module_lock), then what is to 
>> stop the VMThread from hitting one of these sections of code 
>> protected by locked_or_safepoint and then proceeding into what is 
>> effectively a critical section (by virtue of there being a safepoint) 
>> when the JavaThread is itself in the midst of a critical section? Do 
>> we actively take steps to prevent this somehow, or to make it safe 
>> for the VMThread to proceed?
>

I might not have answered your question about this lock in particular.  
There is only the linking and unlinking (in a safepoint except at ZGC) 
that are protected by CLDG_lock, and these are not interrupted by a 
safepoint.  So this is safe.

Coleen
> No we don't.  I think we have this problem today (not introduced or 
> made worse by this patch).  I'll file an bug to fix it and hopefully 
> add detection for this.  I think we don't need to take CLDG_lock in a 
> safepoint and should prevent doing so, but things like this are more 
> reliable to do with computers than visual inspection. Can you add your 
> suggestions to my RFE? https://bugs.openjdk.java.net/browse/JDK-8213150
>
> The CLDG lock can be shared by non-java threads and java threads, 
> which is the point.  There may be other locks though.
>>
>>>
>>> The locks added in this patch set though are for the NonJavaThreads, 
>>> who do not do safepoint checks.  The NonJavaThreads that acquire 
>>> these locks use the STS joiner mechanism which disallows safepoints 
>>> while being held (and since they are non Java threads, do not check 
>>> for safepoints themselves).
>>>
>>> This is how it's going to look for the ZGC caller:
>>>
>>> {
>>> SuspendibleThreadSetJoiner sts_joiner;
>>> // Unlink the classes.
>>> MutexLockerEx ml(ClassLoaderDataGraph_lock);
>>> unloading_occurred = 
>>> SystemDictionary::do_unloading(ZStatPhase::timer(),
>>> true /* do_cleaning */););
>>> }
>>
>> Somehow I missed the creation/invention of the STS joiner mechanism.
>>
>
> Me too!   It's in gc/shared but it's really runtime code, except 
> people in runtime didn't know about it because it's used by GC threads.
>
> Let me know if you have more questions and can review this code.
>
> Thanks,
> Coleen
>
>> David
>>
>>>
>>>>
>>>> Further, are these locks acquired by non-JavaThreads such that the 
>>>> VMThread may be delayed whilst a safepoint is active?
>>>
>>> Yes, theoretically they could delay the VMThread from getting to a 
>>> safepoint or doing its work while in a safepoint but the threads 
>>> that take out these locks only hold them for short durations.
>>>
>>> Thanks,
>>> Coleen
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Thanks,
>>>>> Coleen
>>>
>



More information about the hotspot-runtime-dev mailing list