RFR(xs): 8150379: [windows] Fix Leaks in perfMemory_windows.cpp

Daniel D. Daugherty daniel.daugherty at oracle.com
Tue Feb 23 15:46:46 UTC 2016


Adding Serviceability team... and Jerry T since he's worked on
this code recently...

Dan


On 2/23/16 7:37 AM, Thomas Stüfe wrote:
> Hi David,
>
> thanks for the review!
>
> new webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8150379-fix-leaks-perfMemory_windows_cpp/webrev.01/webrev/
>
> comments inline.
>
> On Tue, Feb 23, 2016 at 2:24 AM, David Holmes <david.holmes at oracle.com>
> wrote:
>
>> Hi Thomas,
>>
>> On 23/02/2016 12:43 AM, Thomas Stüfe wrote:
>>
>>> Hi all,
>>>
>>> please take a look at this small fix for two leaks
>>> in perfMemory_windows.cpp.
>>>
>>> bug report: https://bugs.openjdk.java.net/browse/JDK-8150379
>>>
>>> webrev:
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8150379-fix-leaks-perfMemory_windows_cpp/webrev.00/webrev/
>>>
>> Fixes look good, but there seems to be additional leakage here:
>>
>> 1437   // get the name of the user associated with this process
>> 1438   char* user = get_user_name();
>> 1439
>> 1440   if (user == NULL) {
>> 1441     return NULL;
>> 1442   }
>> 1443
>> 1444   // construct the name of the user specific temporary directory
>> 1445   char* dirname = get_user_tmp_dir(user);
>> 1446
>> 1447   // check that the file system is secure - i.e. it supports ACLs.
>> 1448   if (!is_filesystem_secure(dirname)) {
>> 1449     return NULL;
>> 1450   }
>>
>> We need to free user and dirname before returning. These allocation
>> methods are crying out for some RIIA assistance. :(
>>
>>
> I fixed this place too and took a look at other functions returning C heap
> array but did not find anything more. I also played around with an RAII
> object, but the change got big quickly, so I decided to keep the change
> small. I agree this is a mess. If we wanted to clean up this coding, I
> would do this as a separate issue. Also, instead of doing a RAII wrapper,
> could we not just move some allocations to resource area?
>
> Thomas
>
> Thanks,
>> David
>>
>> Thank you,
>>> Thomas
>>>
>>>



More information about the hotspot-runtime-dev mailing list