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

[David Holmes]

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