RFR: 8035938: Memory leak in JvmtiEnv::GetConstantPool

[serguei.spitsyn at oracle.com]

The fix looks good.

Thanks,
Serguei

On 1/16/15 2: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
> latest http://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
>