RFR(S) (8u): 8035938: Memory leak in JvmtiEnv::GetConstantPool

[serguei.spitsyn at oracle.com]

Looks good.

Thanks,
Serguei

On 1/28/15 9:34 AM, Kevin Walls wrote:
> Hi,
>
> I'd like a sanity check review for backporting this from latest/9 to 
> jdk8u.  It's changing os:free to delete.
>
> In 8u the same change is relevant, the changeset does not import 
> directly as we have to remove an (unnecessary) NMT parameter (which 
> gets removed by 8060074 in jdk9).
>
> Thanks
> Kevin
>
> diff --git a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> --- a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> +++ b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> @@ -68,11 +68,11 @@
>   
>     ~JvmtiConstantPoolReconstituter() {
>       if (_symmap != NULL) {
> -      os::free(_symmap, mtClass);
> +      delete _symmap;
>         _symmap = NULL;
>       }
>       if (_classmap != NULL) {
> -      os::free(_classmap, mtClass);
> +      delete _classmap;
>         _classmap = NULL;
>       }
>     }
>
>
>
> -------- Forwarded Message --------
> Subject: 	Re: RFR: 8035938: Memory leak in JvmtiEnv::GetConstantPool
> Date: 	Fri, 16 Jan 2015 18:28:50 +0000
> From: 	Kevin Walls <kevin.walls at oracle.com>
> Organization: 	Oracle Corporation
> To: 	daniel.daugherty at oracle.com, Mattis Castegren 
> <mattis.castegren at oracle.com>
> CC: 	serviceability-dev at openjdk.java.net
>
>
>
> Thanks Serguei, thanks Dan!
>
>
> On 16/01/2015 17:51, Daniel D. Daugherty wrote:
> > > src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> >
> >     These lines allocate:
> >
> >     line 59:      _symmap = new SymbolHashMap();
> >     line 60:      _classmap = new SymbolHashMap();
> >
> >     and these lines free incorrectly:
> >
> >     line 71:        os::free(_symmap);
> >     line 75:        os::free(_classmap);
> >
> >     so the proposed fix looks good!
> >
> >
> > Dan
> >
> >
> > On 1/16/15 3:17 AM, Mattis Castegren wrote:
> >> Hi
> >>
> >> This bug is targeted for 7u80, with rdp2 next Tuesday. It would be
> >> great to get a review for this fix as soon as possible, so that we
> >> can get this fix out in the last public JDK 7 release.
> >>
> >> Kind Regards
> >> /Mattis
> >>
> >> -----Original Message-----
> >> From: Kevin Walls
> >> Sent: den 15 januari 2015 15:18
> >> To:serviceability-dev at openjdk.java.net
> >> Subject: RFR: 8035938: Memory leak in JvmtiEnv::GetConstantPool
> >>
> >> Hi,
> >>
> >> This is a review request for a change in JVMTI, where we os::free() some
> >> objects where we should delete them.  The problem was logged against 7,
> >> though it exists up to the present day, below is a diff in current
> >> latesthttp://hg.openjdk.java.net/jdk9/hs-rt/hotspot
> >>
> >>
> >> Testing:
> >> I've used a native JVMTI agent to call GetConstantPool() during an
> >> object allocation callback.  Memory usage does appear less after the
> >> change, it can be 5-6MB in a trivial testcase with a small number of
> >> allocations monitored, each inoking GetConstantlPool().
> >>
> >> (In my test agent the  GetConstantPool() call returns a JVMTI error 101
> >> after a fairly short time and a relatively small number of allocations
> >> are monitored.)
> >>
> >> If this is OK to submit without testcase that's great.  I don't see
> >> other examples of native agents in the standard hotspot tests.
> >>
> >> bug
> >>https://bugs.openjdk.java.net/browse/JDK-8035938
> >>
> >> diff:
> >> $ hg diff src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> >> diff --git a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> >> b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> >> --- a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> >> +++ b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> >> @@ -68,11 +68,11 @@
> >>
> >>      ~JvmtiConstantPoolReconstituter() {
> >>        if (_symmap != NULL) {
> >> -      os::free(_symmap);
> >> +      delete _symmap;
> >>          _symmap = NULL;
> >>        }
> >>        if (_classmap != NULL) {
> >> -      os::free(_classmap);
> >> +      delete _classmap;
> >>          _classmap = NULL;
> >>        }
> >>      }
> >>
> >>
> >>
> >> Thanks
> >> Kevin
> >>
> >>
> >>
> >
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20150128/c8f06e2d/attachment.html>