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

Gerald Thornbrugh gerald.thornbrugh at oracle.com
Tue Feb 23 22:50:22 UTC 2016


Hi Thomas,

Your changes look good to me.

Jerry
> 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