Re: RFR JDK-8171836: Memory leak in java.desktop/unix/native/common/awt/fontpath.c

[Phil Race]

+            for ( index = origNumPaths; index < totalDirCount; index++ ) {
+                free( newFontPath[index] );
+            }


Whilst the loop termination condition of "< totalDirCount" is fine
in the code from which you have copied it, it is not correct here,
since you are still in the loop which populates the array.
The likely resulting crash will be a lot worse than the leak ..

Also I am confused about who is the OCA contributor of this fix, and
who is the sponsor (JDK 9 committer) for this fix.

And, BTW, this should really have been reviewed on 2d-dev.


-phil.

On 01/03/2017 12:13 PM, David CARLIER wrote:

Hi all,

this is a new version.

Cheers,

On 22 December 2016 at 05:35, Ambarish Rapte <ambarish.ra...@oracle.com> wrote:

Hi Abhijit,

                 There can be references added to newFontPath[] in previous
iterations of loop at Line 298: newFontPath[nPaths++] = onePath;



                 So these references should also be freed, something similar
to code at line 308:



                 for ( index = origNumPaths; index < totalDirCount; index++ )
{

                                 free( newFontPath[index] );

}



As many references added to newFontPath should be freed accordingly.



Regards,

Ambarish



From: Abhijit Roy
Sent: Thursday, December 22, 2016 12:08 AM
To: Vadim Pakhnushev; awt-dev@openjdk.java.net
Subject: Re: <AWT Dev> RFR JDK-8171836: Memory leak in
java.desktop/unix/native/common/awt/fontpath.c



Hi Vadim,

Yes. I did a mistake here. Please find the correct webrev below.

Webrev: http://cr.openjdk.java.net/~rpatil/8171836/webrev.01/


Thanks
Abhijit

On 12/21/2016 7:47 PM, Vadim Pakhnushev wrote:

Abhijit,
I think there's some misunderstanding here.
The pointer you are trying to free is NULL already:

      if ( newFontPath == NULL ) {
        free ( ( void *) appendDirList );
+      free((void*) newFontPath);

Thanks,
Vadim

On 21.12.2016 16:02, Abhijit Roy wrote:

Hi all,







Please review the fix for the bug below:



Bug: https://bugs.openjdk.java.net/browse/JDK-8171836



Description: Memory leak in java.desktop/unix/native/common/awt/fontpath.c



Webrev: http://cr.openjdk.java.net/~rpatil/8171836/webrev.00/





To prevent memory leak issue, I have released the newFontPath in
java.desktop/unix/native/common/awt/fontpath.

Moving forward it for review.







Regards,



Abhijit