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

[David Holmes]

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?

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

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
>