Code review request: 8002273 NMT to report JNI memory leaks when -Xcheck:jni is on

[Zhengyu Gu]

Hi Dan,

Thanks for reviewing.

On 11/9/2012 2:30 PM, Daniel D. Daugherty wrote:
> Do we have a way of detecting this issue at thread-exit time?
>
> In other words, once a JNI thread has attached to the VM and has
> become a JavaThread, do all exit paths from the JNI thread now
> go through VM code, e.g., JavaThread::exit()? Or because this is
> a JNI thread that has attached to the VM, does the thread exit
> path get done differently?
>
I don't think that VM has any control over JNI thread's exit path.  The 
thread can just call AttachCurrentThread() and exit.

> Why am I wondering this? Because it would be good if we could
> provide a more detailed message about the JNI thread that exited
> (or is about to exit) without detaching from the VM.
I thought about this, the alternative is to walk threads 
(Threads::do_threads()), which we might be able to identify the 
offender, but what kind of information we can get? its stack is no 
longer walkable and thread name is something like Thread-nnn.

>
> Also, thumbs up on the code modulo clearing up the comment lines
> that David H pointed out...
>
I made change.

Thanks,

-Zhengyu

> Dan
>
>
>
> On 11/8/12 9:06 PM, David Holmes wrote:
>> Hi Zhengyu,
>>
>> On 9/11/2012 3:23 AM, Zhengyu Gu wrote:
>>> This fix is related to JDK-8001743: Overlapped thread stacks. The cause
>>> of 8001743 is that, an attached JNI thread exited without detaching the
>>> thread, so JVM will leak a JavaThread object that attached to the JNI
>>> thread.
>>>
>>> The patch allows NMT to report such case when -Xcheck:jni VM option is
>>> specified.
>>>
>>> http://cr.openjdk.java.net/~zgu/8002273/webrev.00/
>>
>> I'm a little unclear who is doing the checking here. Is it the case 
>> that a regular thread while updating its NMT records, detects that 
>> there is an overlap with an existing region, and infers from that 
>> that the other region must belong to a thread that failed to detach?
>>
>> This comment doesn't read clearly to me, and we don't generally 
>> include references to bug reports:
>>
>>  138   // an attached JNI thread can exit without detaching the 
>> thread, that results
>>  139   // JVM to leak JavaThread object (JDK-8001743)
>>
>>
>> If my understanding of the check is correct then to me a clearer 
>> comment would be:
>>
>> // Overlapping stack regions indicate that a JNI thread failed to
>> // detach from the VM before exiting. This leaks the JavaThread object.
>>
>> Cheers,
>> David
>>
>>> Thanks,
>>>
>>> -Zhengyu
>