Code review request: three native memory tracking related bugs (original) (raw)

David Holmes david.holmes at oracle.com
Fri Jul 13 23:57:55 PDT 2012

On 14/07/2012 5:43 AM, Zhengyu Gu wrote:

7181989: NMT ON: Assertion failure when NMT checks thread's native CR: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7181989 Webrev: http://cr.openjdk.java.net/~zgu/7181989/webrev.00/

We try to assert Thread's stack base to ensure that Thread::recordstackbaseandsize() 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); }

7181986: NMT ON: Assertion failure when running jdi ExpiredRequestDeletionTest CR: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7181986 Webrev: http://cr.openjdk.java.net/~zgu/7181986/webrev.00/

This is a racing condition when C/C++ runtime exit handler is ran before NMT worker thread exits. The exit handler calls querylock's destructor while NMT worker thread is still holding it. The fix is to make querylock a heap object, instead of static object, but the drawback is that, it does not seem that querylock can be safely deleted. Also, I reassigned MemSnaphot lock and query lock, so they participate in the deadlock detection logic.

src/share/vm/services/memSnapshot.cpp

We don't use std::nothrow when allocating other VM mutexes, and you are not checking for allocation failures.

Can't comment on whether the 'level' assigned to the mutexes is appropriate - only testing and time will tell.

7182543: NMT ON: Aggregate a few NMT related bugs CRhttp://bugs.sun.com/bugdatabase/viewbug.do?bugid=7182543 Webrev: http://cr.openjdk.java.net/~zgu/7182543/webrev.00/

- Fixed generationsinused calculation - Wait MemRecorder instance count to drop to zero before completely shutdown NMT

So previously the instance_count was only present in debug. Is this likely to be a contention bottleneck?

- Added assertion for JavaThread in threadblocked state.

Is the check for a safepoint needed? I'm assuming this assertion is trying to verify that threads in _thread_blocked don't do allocation/deallocation (though I'm unclear why the constraint exists??). Whether a safepoint is presently active or not seems irrelevant ??

Cheers, David

Thanks, -Zhengyu

More information about the hotspot-dev mailing list