RFR(s): 8213574: Deadlock in string table expansion when dumping lots of CDS classes

[Robbin Ehn]

Hi David,

On 11/12/18 11:32 PM, David Holmes wrote:
> Hi Robbin,
> 
> On 12/11/2018 11:59 PM, Robbin Ehn wrote:
>> Hi all, please review.
>>
>> The re-sizing operation is run by the ServiceThread (JavaThread). To be 
>> safepoint polite it pauses the operation and do safepoint checks. Operations on
>> the hashtable that visit multiple bucket are mutual exclusive. This means you
>> can't iterate over the table during a safepoint if there is paused resize.
>>
>> The hashtable uses a Mutex, which means if there were other threads using it
>> during the safepoint the VM thread sneaking would break it. Since there are no 
>> such users it is safe to access it without locks in side the safepoint. That is
>> how rehash works today.
> 
> Okay - so you are saying that if the existing code is not actually broken, then 
> the new code is also not actually broken. I agree. But I'd be happier if there 
> was somewhere we could assert this fact. :)
> 
> I wonder whether as part of the Mutex rewrite we should add the ability to 
> indicate what types of thread can acquire a given mutex, and thus ensure 
> non-JavaThreads can't access things they should not ?

That would be nice.
ErikÖ have added some thread behavior which might be used for fine grain control.

> 
>> Until we sorted this out better in 8213742,  I'm adding a safepoint scanning
>> operation that can handle a paused resize and which skips the lock.
>>
>> CR: https://bugs.openjdk.java.net/browse/JDK-8213574
>> Webrev: http://cr.openjdk.java.net/~rehn/8213574/webrev/
> 
> The Thread argument to do_safepoint_scan seems unused and unnecessary - the 
> current thread must be the VMThread (which should be asserted)

Replying to original mail with an small update.

> 
> Otherwise looks good.

Thanks, Robbin

> 
> Thanks,
> David
> -----
> 
>> Passes t1-3 and 8213587 new test which triggered the issue.
>>
>> Thanks, Robbin