Review Request: 7196045 Possible JVM deadlock in ThreadTimesClosure

[David Holmes]

On 5/09/2012 7:16 PM, Kevin Walls wrote:
>
> Hi David,
>
> We could allocate some tens of kb... more if there are several thousand
> threads with very long names... It's not a commonly called method, but
> that's why the deadlock hasn't been exposed before. Yes you'd need to be
> very near that existing cliff-edge...
>
>
> Just thinking that yes if we are low on memory and don't die from the
> NEW_C_HEAP_ARRAY, then the strdup could fail. I'll add a check that we
> have something to free() in case that fails on some platforms. In the
> loop below if _names_chars[i] was null, the app will get a null string,
> again probably the least of it's worries at that point.

It's okay to call free(NULL) so no need to check for it explicitly. And 
the NULL is handled by create_from_str.

But the CHECK macro will cause us to return if an exception is posted 
and we will leak the dup'd string in that case.

David

> Thanks
> Kevin
>
>
> for (int i=0; i<_count; i++) {
> Handle s = java_lang_String::create_from_str(_names_chars[i], CHECK);
> _names_strings->obj_at_put(i, s());
> if (_names_chars[i] != NULL) {
> free(_names_chars[i]);
> }
> }
>
>
>
> On 05/09/12 02:51, David Holmes wrote:
>> Hi Kevin,
>>
>> On 5/09/2012 8:35 AM, Kevin Walls wrote:
>>> I'd like to get this reviewed, it's a deadlock that can happen due to
>>> Java objects being allocated while holding Threads_lock.
>>>
>>> We need to collect just the characters of the thread names while holding
>>> the lock, and create the String objects when it's released. You do need
>>> to hit the (non-public) HotspotInternal MBean very rapidly to trigger
>>> this reliably, but there's a small chance the deadlock could happen when
>>> it's called by the tool that is meant to call it.
>>>
>>> http://cr.openjdk.java.net/~kevinw/7196045/webrev.01/
>>
>> The only thing I don't like here is that the use of NEW_C_HEAP_ARRAY
>> will cause an abort on out-of-memory and I'm not sure how big this
>> array might be. If you are running out of C-Heap then you're on the
>> edge of the cliff anyway and likely to fall at any minute, but I still
>> dislike seeing more cases added to the code. I don't see an
>> alternative though. :(
>>
>> David
>>
>>
>>> Thanks!
>>> Kevin
>>>
>