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

[Kevin Walls]

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/d220f2e3/attachment.html>