[OpenJDK 2D-Dev] RFR: [9] JDK-8132985, , Crash in freetypescaler.c due to double free

[Volker Simonis]

On Wed, Sep 30, 2015 at 6:03 PM, Phil Race <philip.race at oracle.com> wrote:
> Hello Volker,
>
> The test lacks an @test tag on purpose so jtreg won't even try to run it.
>

Ah, I missed that. You're right, thanks for the clarification.

> Thus it is a manual test already - you would have to run it yourself outside
> jtreg.
> It is 'better than nothing' but admittedly not by much, and having
> to use internal APIs is not great either.
>
> The problem with checking in a font is that the only way we can do this
> is by creating one ourselves. 3rd party fonts, no matter how permissive
> the license, can't be checked in without lots of legal work that is out
> of all proportion to the benefit and would take months to complete
> even if we could convince someone it was worth it.
>

I already thought it wouldn't be easy :)

Otherwise I'm fine with the change. You can count me as a reviewer if
that helps.

Regards,
Volker


> If someone wants to create such a font - from scratch - not derived
> from any other work, and contribute under OCA that would be a solution.
>
> -phil.
>
>
> On 09/30/2015 07:42 AM, Volker Simonis wrote:
>>
>> Hi Prasanta,
>>
>> the fix looks good, but I'm a little concerned about the test.
>>
>> Shouldn't it be at least be marked as 'manual' and provide some means
>> of querying the font path from the user.
>> Otherwise I'm afraid it will just fail with an array index out of
>> bounds exception if called from jtreg.
>>
>> Alternatively (and for me the preferred solution) you could check in a
>> small Type 1 font along with the test such that it will be possible to
>> always execute it stand-alone.
>>
>> Thank you and best regards,
>> Volker
>>
>>
>> On Wed, Sep 30, 2015 at 7:27 AM, prasanta sadhukhan
>> <prasanta.sadhukhan at oracle.com> wrote:
>>>
>>> Thanks Phil.
>>> Hi Sergey, Can I get a +1 for this?
>>>
>>> Regards
>>> Prasanta
>>>
>>> On 9/30/2015 4:16 AM, Phil Race wrote:
>>>>
>>>> Approved.
>>>>
>>>> -phil.
>>>>
>>>> On 09/28/2015 10:25 PM, prasanta sadhukhan wrote:
>>>>>
>>>>> Gentle reminder to approve and commit this change.
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>> On 9/28/2015 10:49 AM, prasanta sadhukhan wrote:
>>>>>>
>>>>>> Hi Phil,Sergey,
>>>>>>
>>>>>> Could you please approve this fix?
>>>>>> I have followed your comment regarding the test.
>>>>>>
>>>>>> Regards
>>>>>> Prasanta
>>>>>> On 9/25/2015 10:55 AM, prasanta sadhukhan wrote:
>>>>>>>
>>>>>>> Hi All,
>>>>>>>
>>>>>>> Can this please be reviewed and approved please?
>>>>>>>
>>>>>>> Regards
>>>>>>> Prasanta
>>>>>>> On 9/21/2015 2:57 PM, prasanta sadhukhan wrote:
>>>>>>>>
>>>>>>>> Hi All,
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8132985
>>>>>>>> Webrev: http://cr.openjdk.java.net/~psadhukhan/8132985/webrev.00/
>>>>>>>>
>>>>>>>> Please review a bug fix whereby freeing a FT_StreamRec pointer leads
>>>>>>>> to a crash.
>>>>>>>> It was found that FT_Done_Face() frees the FT_StreamRec pointer if
>>>>>>>> FT
>>>>>>>> allocated memory for it internally.
>>>>>>>> Since Java uses freetype, FT ends up allocating FT_StreamRec
>>>>>>>> structure
>>>>>>>> in Java's TypeFont1. We call FT_Done_Face(FT_Face)
>>>>>>>> and also free(face->stream) resulting in double free causing crash.
>>>>>>>> The solution is to maintain it's own copy of stream pointer so Java
>>>>>>>> knows when it needs to free the stream pointer and when to
>>>>>>>> leave it to FT.
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Prasanta
>>>>>>>
>>>>>>>
>