Review Request: 7196045 Possible JVM deadlock in ThreadTimesClosure

[Kevin Walls]

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