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

[Kevin Walls]

Thanks!

On 28/01/2015 17:45, Daniel D. Daugherty wrote:
> src/share/vm/prims/jvmtiClassFileReconstituter.hpp
>     No comments.
>
> Thumbs up.
>
> Dan
>
>
> On 1/28/15 10: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/6df922fc/attachment.html>