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

[David CARLIER]

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

diff --git a/src/java.desktop/unix/native/common/awt/fontpath.c 
b/src/java.desktop/unix/native/common/awt/fontpath.c
--- a/src/java.desktop/unix/native/common/awt/fontpath.c
+++ b/src/java.desktop/unix/native/common/awt/fontpath.c
@@ -289,6 +289,12 @@
         onePath = SAFE_SIZE_ARRAY_ALLOC(malloc, strlen (fDirP->name[index]) + 
2, sizeof( char ) );
         if (onePath == NULL) {
             free ( ( void *) appendDirList );
+
+            for ( index = origNumPaths; index < totalDirCount; index++ ) {
+                free( newFontPath[index] );
+            }
+
+            free( ( void *) newFontPath);
             XFreeFontPath ( origFontPath );
             return;
         }