RFR: 8294370: Fix allocation bug in java_lang_Thread::async_get_stack_trace()

[Patricio Chilano Mateo]

On Tue, 27 Sep 2022 06:48:50 GMT, David Holmes <dholmes at openjdk.org> wrote:

> Good find! Looks good! A couple of queries at this stage.
> 
> Thanks.
>
Thanks for looking at this David.

> src/hotspot/share/classfile/javaClasses.cpp line 2004:
> 
>> 2002:       const bool skip_hidden = !ShowHiddenFrames;
>> 2003: 
>> 2004:       // Pick some initial length
> 
> The comment should at least hint at there being some reasonable reason for choosing the value that follows. :)

How about: "Pick minimum length that will cover most cases"?

> src/hotspot/share/classfile/javaClasses.cpp line 2008:
> 
>> 2006:       _methods = new (ResourceObj::C_HEAP, mtInternal) GrowableArray<Method*>(init_length, mtInternal);
>> 2007:       _bcis = new (ResourceObj::C_HEAP, mtInternal) GrowableArray<int>(init_length, mtInternal);
>> 2008: 
> 
> Couldn't you just do this in the constructor? I'm not clear if there is a subtle reason for needing lazy-init as well as moving to the C_Heap.

Yes, this could have been done in the constructor too. But since there are additional checks in the closure that could fail I move the allocation here to avoid unnecessary allocation/deallocation. The allocation still needs to be done in the C_Heap in case the target executes the handshake. Otherwise if the target allocates the arrays in its resource area they could be deallocated by the time the requester tries to access them after the handshake.

-------------

PR: https://git.openjdk.org/jdk/pull/10424