On Mon, 26 Sep 2022 14:23:38 GMT, Patricio Chilano Mateo <pchilanom...@openjdk.org> wrote:
> Please review this small fix in async_get_stack_trace(). The GrowableArrays > created to store the bci and Method* of each frame found while traversing the > stack are allocated in the resource area of the thread that calls > async_get_stack_trace(). But if the handshake is executed by the target and > if the number of frames in the stack exceeds the initial size of the > GrowableArrays then we will hit an assertion when trying to grow the size of > the arrays (see bug description). > Currently we don't see any issues because the initial size of the > GrowableArrays is 512 and our tests don't test beyond that (the maximum value > of DEPTH in the vmTestbase/nsk/stress/strace/ tests is 500). The issue can be > easily reproduced by either decreasing the initial size of the GrowableArrays > or by increasing the value of DEPTH in those strace tests. > To fix it I allocated the arrays in the C heap instead. Also I lowered the > initial size of the arrays since 512 seemed too much to start with. > Tested it by running all tests in the vmTestbase/nsk/stress/strace/ directory. > > Thanks, > Patricio Good find! Looks good! A couple of queries at this stage. Thanks. 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. :) 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. ------------- PR: https://git.openjdk.org/jdk/pull/10424
