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
