Review Request: 8011161 NMT: Memory leak when encountering out of memory error while initializing memory snapshot

[Ron Durbin]

This I do have some experience with. And the answer might be a special test hook.

There is one I put in a few weeks ago. That you could copy and modifiy the copy.

Mine is armed by an option to the jvm and triggered by a limit to the maximum memory.

 

 

Yours needs a slightly different twist.

 

I am gate keeping at the moment, but maybe in a couple of hours.

 

 

 

From: Zhengyu Gu 
Sent: Thursday, April 04, 2013 11:41 AM
To: Ron Durbin
Cc: hotspot-runtime-dev at openjdk.java.net
Subject: Re: Review Request: 8011161 NMT: Memory leak when encountering out of memory error while initializing memory snapshot

 

Hi Ron,

Thanks for reviewing.

"Copyright" is optional, the script will fix them during promotion (?) Any suggestion on how to create one?

There is not unit test for it. How can you get malloc to fail at right time?

I ran vm.quick.testlist on Linux x86.

Thanks,

-Zhengyu

On 4/4/2013 9:31 AM, Ron Durbin wrote:

Zhengyu,

 

Code looks good.

You need to fix the copyright, it is now 2013 not 2012.

Do you have a unit test for this fix?

What platforms have you unit tested on?

Have or do you plan a JPRT run on the changed code?

 

Ron

 

From: Zhengyu Gu 
Sent: Wednesday, April 03, 2013 2:47 PM
To: Calvin Cheung
Cc: HYPERLINK "mailto:hotspot-runtime-dev at openjdk.java.net"hotspot-runtime-dev at openjdk.java.net
Subject: Re: Review Request: 8011161 NMT: Memory leak when encountering out of memory error while initializing memory snapshot

 

Thanks for reviewing.

MemTracker.cpp #552 has similar problem, which does not free _worker_thread due to initialization failure.  I will fix it and post updated webrev later.

Thanks,

-Zhengyu




On 4/3/2013 4:35 PM, Calvin Cheung wrote:

Not directly related to this fix, I'm seeing shutdown(NMT_initialization) can be called twice if start_worker() has an error before returning false.
552 shutdown(NMT_initialization); 

again in MemTracker::start():
144 shutdown(NMT_initialization);

Perhaps the one at line 552 isn't necessary?

Calvin

On 4/3/2013 11:01 AM, Zhengyu Gu wrote:

Resend. Could anyone review it?

Thanks,

-Zhengyu

On 4/2/2013 10:09 AM, Zhengyu Gu wrote:

Fix a memory leak when initializing memory snapshot due to out of memory. 
In normal case, memory snapshot is released by worker thread. But this case, the worker thread is never started.


HYPERLINK "http://bugs.sun.com/view_bug.do?bug_id=8011161"bug: http://bugs.sun.com/view_bug.do?bug_id=8011161
HYPERLINK "http://cr.openjdk.java.net/%7Ezgu/8011161/webrev.00/"webrev: http://cr.openjdk.java.net/~zgu/8011161/webrev.00/


Thanks,

-Zhengyu

 

 

 

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130404/86956b53/attachment.html