RFR: 8203885: ConcurrentLocksDump::dump_at_safepoint() should not allocate array in resource area

[David Holmes]

On 28/05/2018 11:05 PM, Per Liden wrote:
> On 05/28/2018 02:47 PM, David Holmes wrote:
>> Hi Per,
>>
>> On 28/05/2018 10:16 PM, Per Liden wrote:
>>> ConcurrentLocksDump::dump_at_safepoint() creates a GrowableArray, 
>>> which gets allocated in a resource area. This array is than passed 
>>> down a call chain, where it can't control that another ResourceMark 
>>> isn't created. In the leaf of this call chain, a closure 
>>> (FindInstanceClosure) is executed, which appends to the array, which 
>>> means it might need to be resized. This doesn't work if a new 
>>> ResourceMark has been created, since the array resize will happen in 
>>> a nested ResourceArea context. As a result, the append operation 
>>> fails in GenericGrowableArray::check_nesting().
>>>
>>> This has so far gone unnoticed because 
>>> CollectedHeap::object_iterate() in existing collectors typically 
>>> don't create new ResourceMarks. This is not true for ZGC (and 
>>> potentially other concurrent collectors), which needs to walk thread 
>>> stacks, which in turn requires a ResourceMark.
>>>
>>> The proposed fix is to make this array C Heap allocated.
>>
>> That seems fine in itself but I'm not clear what the OOM behaviour of 
>> either the old or the new code is here??
> 
> Thanks for looking at this David.
> 
> On OOM, both the old and the new code will eventually end up in 
> vm_exit_out_of_memory().

Okay - that's not good. Probably non trivial to fix as the dump has to 
allow for failure - and can't directly propagate an exception. I'll file 
a bug.

Thanks,
David

> /Per
> 
>>
>> Thanks,
>> David
>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8203885
>>> Webrev: http://cr.openjdk.java.net/~pliden/8203885/webrev.0
>>>
>>> Testing: hs-tier{1,3}
>>>
>>> /Per