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.hol...@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
