<AWT Dev> RFR: 8181139: Memory leak in awt_Font.cpp / AwtFont::Create

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

<AWT Dev> RFR: 8181139: Memory leak in awt_Font.cpp / AwtFont::Create

Phil Race
Bug: https://bugs.openjdk.java.net/browse/JDK-8181139
Webrev : http://cr.openjdk.java.net/~prr/8181139/

if we aren't using / returning the awtFont C object we should free
(delete) it before returning
to avoid a leak in this theoretical situation.

The object has only just been allocated and doesn't have any clean up
that needs to be
done besides calling its C++ destructor.

[NB sending to awt-dev, not 2d-dev since this C++ Windows-specific class
is used only to
support Composite Logical Fonts on AWT GDI heavyweights .. ]

-phil.
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8181139: Memory leak in awt_Font.cpp / AwtFont::Create

Semyon Sadetsky
Should not it be deleted in the catch block as well?

--Semyon


On 11/8/17 4:13 PM, Philip Race wrote:

> Bug: https://bugs.openjdk.java.net/browse/JDK-8181139
> Webrev : http://cr.openjdk.java.net/~prr/8181139/
>
> if we aren't using / returning the awtFont C object we should free
> (delete) it before returning
> to avoid a leak in this theoretical situation.
>
> The object has only just been allocated and doesn't have any clean up
> that needs to be
> done besides calling its C++ destructor.
>
> [NB sending to awt-dev, not 2d-dev since this C++ Windows-specific
> class is used only to
> support Composite Logical Fonts on AWT GDI heavyweights .. ]
>
> -phil.

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8181139: Memory leak in awt_Font.cpp / AwtFont::Create

Phil Race
I don't see how it can be safe to do that.
That is more likely to lead to a crash than to free memory.
In the loop the awtFont is made to reference several HFONTs and
added to a disposer to release these. If the awtFont is freed and the
disposer
then run it will be a bad thing.

I'd far sooner have that leak than the crash.

And it seems like this can only happen if memory allocation fails.
ie if the AwtFont could not be constructed to begin with .. so
there isn't even then a valid thing to free.

If anything this points to the issue that all the calls to throw
std::bad_alloc()
make clean-up hard but it is a risky project to modify all of that.

-phil.


On 11/08/2017 04:55 PM, [hidden email] wrote:

> Should not it be deleted in the catch block as well?
>
> --Semyon
>
>
> On 11/8/17 4:13 PM, Philip Race wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8181139
>> Webrev : http://cr.openjdk.java.net/~prr/8181139/
>>
>> if we aren't using / returning the awtFont C object we should free
>> (delete) it before returning
>> to avoid a leak in this theoretical situation.
>>
>> The object has only just been allocated and doesn't have any clean up
>> that needs to be
>> done besides calling its C++ destructor.
>>
>> [NB sending to awt-dev, not 2d-dev since this C++ Windows-specific
>> class is used only to
>> support Composite Logical Fonts on AWT GDI heavyweights .. ]
>>
>> -phil.
>

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8181139: Memory leak in awt_Font.cpp / AwtFont::Create

Semyon Sadetsky
OK, looks good.

--Semyon


On 11/09/2017 11:10 AM, Phil Race wrote:

> I don't see how it can be safe to do that.
> That is more likely to lead to a crash than to free memory.
> In the loop the awtFont is made to reference several HFONTs and
> added to a disposer to release these. If the awtFont is freed and the
> disposer
> then run it will be a bad thing.
>
> I'd far sooner have that leak than the crash.
>
> And it seems like this can only happen if memory allocation fails.
> ie if the AwtFont could not be constructed to begin with .. so
> there isn't even then a valid thing to free.
>
> If anything this points to the issue that all the calls to throw
> std::bad_alloc()
> make clean-up hard but it is a risky project to modify all of that.
>
> -phil.
>
>
> On 11/08/2017 04:55 PM, [hidden email] wrote:
>> Should not it be deleted in the catch block as well?
>>
>> --Semyon
>>
>>
>> On 11/8/17 4:13 PM, Philip Race wrote:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8181139
>>> Webrev : http://cr.openjdk.java.net/~prr/8181139/
>>>
>>> if we aren't using / returning the awtFont C object we should free
>>> (delete) it before returning
>>> to avoid a leak in this theoretical situation.
>>>
>>> The object has only just been allocated and doesn't have any clean
>>> up that needs to be
>>> done besides calling its C++ destructor.
>>>
>>> [NB sending to awt-dev, not 2d-dev since this C++ Windows-specific
>>> class is used only to
>>> support Composite Logical Fonts on AWT GDI heavyweights .. ]
>>>
>>> -phil.
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8181139: Memory leak in awt_Font.cpp / AwtFont::Create

Sergey Bylokhov
+1

On 09/11/2017 11:15, Semyon Sadetsky wrote:

> OK, looks good.
>
> --Semyon
>
>
> On 11/09/2017 11:10 AM, Phil Race wrote:
>> I don't see how it can be safe to do that.
>> That is more likely to lead to a crash than to free memory.
>> In the loop the awtFont is made to reference several HFONTs and
>> added to a disposer to release these. If the awtFont is freed and the
>> disposer
>> then run it will be a bad thing.
>>
>> I'd far sooner have that leak than the crash.
>>
>> And it seems like this can only happen if memory allocation fails.
>> ie if the AwtFont could not be constructed to begin with .. so
>> there isn't even then a valid thing to free.
>>
>> If anything this points to the issue that all the calls to throw
>> std::bad_alloc()
>> make clean-up hard but it is a risky project to modify all of that.
>>
>> -phil.
>>
>>
>> On 11/08/2017 04:55 PM, [hidden email] wrote:
>>> Should not it be deleted in the catch block as well?
>>>
>>> --Semyon
>>>
>>>
>>> On 11/8/17 4:13 PM, Philip Race wrote:
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8181139
>>>> Webrev : http://cr.openjdk.java.net/~prr/8181139/
>>>>
>>>> if we aren't using / returning the awtFont C object we should free
>>>> (delete) it before returning
>>>> to avoid a leak in this theoretical situation.
>>>>
>>>> The object has only just been allocated and doesn't have any clean
>>>> up that needs to be
>>>> done besides calling its C++ destructor.
>>>>
>>>> [NB sending to awt-dev, not 2d-dev since this C++ Windows-specific
>>>> class is used only to
>>>> support Composite Logical Fonts on AWT GDI heavyweights .. ]
>>>>
>>>> -phil.
>>>
>>
>


--
Best regards, Sergey.