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

[Thomas Stüfe]

David, Jerry, thank you!

On Wed, Feb 24, 2016 at 12:57 AM, David Holmes <david.holmes at oracle.com>
wrote:

> On 24/02/2016 12: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/
>>
>
> Looks good. I can sponsor this for you.
>
> comments inline.
>>
>> On Tue, Feb 23, 2016 at 2:24 AM, David Holmes <david.holmes at oracle.com
>> <mailto: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
>>
>
> Ok.
>
> 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?
>>
>
> I'm unclear on the allocation lifecycles in this code.
>
> Thanks,
> David
>
>
> Thomas
>>
>>     Thanks,
>>     David
>>
>>         Thank you,
>>
>>         Thomas
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20160224/cda57254/attachment.html>