Review Request: 7196045 Possible JVM deadlock in ThreadTimesClosure

[Kevin Walls]

Just to add: this is only relevant to internal VM threads, so the 
numbers really should be quite small... (there won't be thousands of 
theads examined, unless we're getting excessive with e.g. GC workers...).



On 05/09/12 10:16, 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.
>
> 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
>>>
>