<AWT Dev> RFR JDK-8171836: Memory leak in java.desktop/unix/native/common/awt/fontpath.c

David CARLIER devnexen at gmail.com
Wed Jan 25 10:13:43 UTC 2017


Thanks for the time spent ;-)

On 5 January 2017 at 08:08, Ambarish Rapte <ambarish.rapte at oracle.com>
wrote:

> Hi David,
> +1
> Looks good to me.
>
> Regards,
> Ambarish
>
> -----Original Message-----
> From: Phil Race
> Sent: Thursday, January 05, 2017 12:39 AM
> To: David CARLIER; awt-dev at openjdk.java.net
> Subject: Re: <AWT Dev> RFR JDK-8171836: Memory leak in
> java.desktop/unix/native/common/awt/fontpath.c
>
> This looks correct.
>
> +1
>
> I can commit this if we get a 2nd reviewer to approve.
>
> -phil.
>
> On 01/04/2017 11:05 AM, David CARLIER wrote:
> > 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.rapte at 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 at 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
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20170125/7ddfe917/attachment.html>


More information about the awt-dev mailing list