crtdbg dumps a memory leak when sf::Text::setOutlineThickness is used

[Chringo]

Hi, I need confirmation on this bug before I post an issue on GitHub:
http://stackoverflow.com/a/44074908/5044558
As the answer states, it looks like a line in Font.cpp is incorrect?
https://github.com/SFML/SFML/blob/master/src/SFML/Graphics/Font.cpp#L561

Working with SFML 2.4.2 on Windows 7 64-bit version, I've noticed an issue with sf::Text::setOutlineThickness(float). Once it is used in the program, except for default value 0, crtdbg dumps a memory leak of various sizes of bytes but always the same amount. I believe this is related to the size of the string, if the text gets drawn, and if the parameter of setOutlineThickness is accepted, demonstrated here:

    /// Initial set-up
    sf::Text test;
    test.setString("A");
    // ... Set charactersize, font, fillcolor, etc ...
    test.setOutlineThickness(1);
    test.setOutlineColor(sf::Color::Black);

    /// Make a drawcall for test later in the program
    void Game::draw(sf::RenderTarget & target, sf::RenderStates states) const
    {
        target.draw(test, states);
    }

This produces a leak the size of the string, i.e. "A": 

{8601} normal block at 0x0000000005CA5C90, 60 bytes long.
 Data: <                > 03 00 07 00 0B 00 0F 00 13 00 17 00 1B 00 1F 00
{8600} normal block at 0x0000000005E03A20, 120 bytes long.
 Data: <                > 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01
{8599} normal block at 0x0000000005E2A680, 960 bytes long.
 Data: <                > 00 00 00 00 80 07 00 00 0D 01 00 00 80 07 00 00
{8598} normal block at 0x0000000005CA36B0, 72 bytes long.
 Data: <        h       > F0 1A 9D 05 00 00 00 00 68 AE 83 DB FE 07 00 00

I could expand more from my question on StackOverflow if this isn't enough.

This person has the same problem but I wasn't sure if I should revive the post.
https://en.sfml-dev.org/forums/index.php?topic=21391.0

[eXpl0it3r]

Make sure you test SFML with the latest code changes on the master branch on GitHub.

We just recently fixed a memory leak in sf::Font, so maybe this is connected?

[Chringo]

I'll do that since I've only pulled 2.4.2 from your website - https://www.sfml-dev.org/download/sfml/2.4.2/

Glyphs shouldn't be destroyed then, as the documentation states on Freetype? - https://www.freetype.org/freetype2/docs/reference/ft2-glyph_stroker.html#FT_Glyph_Stroke

[eXpl0it3r]

It might still be an issue, I'm not very familiar with FT, but before digging deeper into an issue, it should always be verified against the latest code base, especially when there​ has been a modification on a similar topic.

[Chringo]

Tested with the master branch on GitHub and the memory leak still exist. Then I changed the parameter in Font.cpp on line 561 to true and there are no longer any memory leaks. I built SFML in Win32 debug/release but I guess we have to be more thorough?

[Hapax]

It looks like setting that parameter to true destroys the glyph object; this glyph object is still used in the code that follows so destroying it doesn't seem to make sense.

[eXpl0it3r]

Tested with the master branch on GitHub and the memory leak still exist. Then I changed the parameter in Font.cpp on line 561 to true and there are no longer any memory leaks. I built SFML in Win32 debug/release but I guess we have to be more thorough?

Thanks for testing it! Feel free to open an issue on GitHub, so we can track it there as well and it doesn't get forgotten.

It looks like setting that parameter to true destroys the glyph object; this glyph object is still used in the code that follows so destroying it doesn't seem to make sense.

I'm not certain how FT works in that case, but it seems to me, that the glyph may get replaced with the "stroked" glyph. But by not destroying the origin/source glyph, SFML creates a memory leak, as the clean up code only destroys the "stroked" glyph and never destroys the source glyph.

I'll make sure to get someone with more insights on FT to take a closer look as well.

[Chringo]

Posted the issue. Not sure if the body text from the initial post is necessary anymore since we've kind of drifted towards Freetype and Font.cpp. Anyway, it was the initial findings. Tell me if the issue needs any clarifications.

The body of FT_Glyph_Stroke can be found here. Might look into it later in the week.
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/base/ftstroke.c#n2302

[agordeevw]

I'm not certain how FT works in that case, but it seems to me, that the glyph may get replaced with the "stroked" glyph. But by not destroying the origin/source glyph, SFML creates a memory leak, as the clean up code only destroys the "stroked" glyph and never destroys the source glyph.

Exactly this happens.

FT_Glyph_Stroke(...) definition is in the link in previous message.
All functions except FT_ALLOC(...) are defined in
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/base/ftglyph.c

FT_Glyph is defined as a pointer to a struct FT_GlyphRec, which is defined in
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/ftglyph.h.

Inside FT_Glyph_Stroke(...) a new pointer FT_Glyph is created and origin glyph is copied in the new FT_Glyph using FT_Glyph_Copy(...) function, which allocates memory to the new glyph, so the new FT_Glyph now points to memory containing the copy of origin.

FT_EXPORT_DEF( FT_Error )
  FT_Glyph_Stroke( FT_Glyph    *pglyph,
                   FT_Stroker   stroker,
                   FT_Bool      destroy )
  {
    [...]
    FT_Glyph  glyph = NULL;

    [...]

    glyph = *pglyph;
    [...]
    {
      FT_Glyph  copy;

      error = FT_Glyph_Copy( glyph, &copy );   // Memory to origin glyph copy is allocated here
      if ( error )
        goto Exit;

      glyph = copy;
    }
    [...]
 

FT_Glyph_Copy(...) calls ft_new_glyph(...), which in turn calls FT_ALLOC(...), which allocates memory for the copy.

After manipulations with copy (outline and other things related to modification of the glyph) pointer *pglyph is redirected to the new FT_Glyph, and if origin glyph isn't destroyed by specifying destroy argument as "true" (which would call FT_Done_Glyph(...) on origin), memory for origin glyph is left hanging, which supposedly produces the leak, because origin glyph isn't used anywhere in subsequent code in Font.cpp.

   
[...]
if ( destroy )
      FT_Done_Glyph( *pglyph );   // Memory for origin glyph is cleared

    *pglyph = glyph;    // If destroy is "false", origin glyph wouldn't be destroyed and will be left hanging
                        // for someone else to use/clear
    goto Exit;

[...]
} // the function end
 

So it seems to me that setting destroy argument to "true" is a fix.