Hi here an updated version. Thanks. regards.
>>>>>>> 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
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>
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 = origIndex; index < nPaths; index++ ) {
+ free( newFontPath[index] );
+ }
+
+ free( ( void *) newFontPath);
XFreeFontPath ( origFontPath );
return;
}
