Memory Leak on Font::loadGlyph

[cesarizu]

Hello,

I've been running a sample app through valgrind and I see that sf::Font::loadGlyph is leaking memory. According to https://lists.nongnu.org/archive/html/freetype-devel/2008-08/msg00014.html the call to FT_Glyph_To_Bitmap creates a new glyph that needs to be free'd with FT_Done_Glyph. Doing that, that particular leak is gone.

I have a patch ready to be submitted that fixes this leak, should I open a PR on github for that?

Thanks!
César

[Laurent]

https://github.com/SFML/SFML/blob/master/src/SFML/Graphics/Font.cpp#L646

?

[cesarizu]

The problem is that FT_Glyph_To_Bitmap at https://github.com/SFML/SFML/blob/master/src/SFML/Graphics/Font.cpp#L559 changes the glyphDesc pointer to a new value, the original one is lost. The solution is to save a copy and free both at the end of the function.

[binary1248]

Read carefully through the mailing list excerpt again and the corresponding code in SFML. In SFML's case, we are calling FT_Glyph_To_Bitmap with the destroy parameter set to 1, instructing FreeType to destroy the glyph that got replaced before returning if no error occurred. If FreeType does not do this in your case then it is bugged. There have been many valgrind passes over SFML in the past, and this kind of leak would have been obvious if it had been a problem.

[cesarizu]

It might be a bug with that parameter in FT. In my tests I changed that to 0, leaving it at one didn't clear the memory, and trying to clear manually with 1 gave some other errors. The 1 there seems to free some resources but not all, that's the original concern of the mailing list post.

Running though valgrind is really clear there's a memory leak there. If you want, I can post a PR with some more detailed valgrind logs so you can see how it changes before and after.

[Laurent]

The mailing list clearly says that they call FT_Done_Glyph twice because they leave the "destroy" parameter of FT_Glyph_To_Bitmap to 0. The official documentation of the function is also very clear about the behaviour of this argument. And if you insist we can find other sources that confirm this. So I still see nothing wrong in what SFML does.

If you agree that SFML is using FreeType as it should, then maybe you should try to reproduce the leak in a complete and minimal code with only FreeType calls, and submit it to the FreeType mailing list. Make sure that your version of FreeType is up-to-date first

[cesarizu]

You are right FT is not working as expected, I'll create a sample app that replicates this leak and post it to the FT devel list. In any case it's not a critical leak as it only happens when loading fonts, which is normally just at the start of the app.