Code review request: three native memory tracking related bugs

Thomas Stüfe thomas.stuefe at
Sun Jul 15 07:35:54 PDT 2012

On Sat, Jul 14, 2012 at 8:57 AM, David Holmes <david.holmes at> wrote:
> On 14/07/2012 5:43 AM, Zhengyu Gu wrote:
>> 7181989: NMT ON: Assertion failure when NMT checks thread's native
>> CR:
>> Webrev:
>> We try to assert Thread's stack base to ensure that
>> Thread::record_stack_base_and_size() to record native stack to NMT, but
>> there is scenario that the thread fails to start, which no native stack
>> is created and should not be asserted.
> I'm not really clear on why we need to refer to the assert inside
> stack_base(). If any started thread fails to setup the stackbase and
> stacksize then it will likely crash well prior to the Thread destructor. It
> seems to me that all that is needed here is to only call
> MemTracker::record_virtual_memory_release for a thread that actually
> started, and for that you can simplify the check to:
> if (os_thr != NULL && os_thr->get_state() > INITIALIZED)
> and so the whole thing can be expressed simply as:
> // Update NMT on Thread destruction, but only if the Thread
> // actually started
> OSThread* os_thr = osthread();
> if (os_thr != NULL && os_thr->get_state() > INITIALIZED) {
>      MemTracker::record_virtual_memory_release(
>           (stack_base() - stack_size()), stack_size(), this);
> }

I agree with that.

There are a number of fatal()'s e.g. in Linux::current_stack_region()
which chould kick in if stack dimension querying fails. However, a
guarantee() or fatal() inside Thread::record_stack_base_and_size()
would be nice to document that this should never fail.

On a related note, it feels a bit wrong to me to let
Thread::record_stack_base_and_size() record stack allocation to the
NMT as an unexpected side effect. Also feels asymetric, because
recording stack deallocation is done in ~Thread(), as one would

I think I see why you did it - there are 8 or so places where
Thread::record_stack_base_and_size() is called. However, the general
pattern seems to be that all children of Thread call factored-out
functions in their constructors, and I also would do this for NMT
stack recording, e.g.:

void WatcherThread::run() {

Kind Regards,
Thomas Stüfe
SAP Germany

More information about the hotspot-dev mailing list