[SFFont] Valgrind is not happy when loading fonts from file

[larta]

Hi all

I'm using SFML (2.1) for a personal project and i want everything to be as good as possible.
I'm using a class to load all of my ressources (images, fonts, sounds) at the program starts.
This class is a singleton. The problem is not with the class itself, but with the "loadFromFile" method, in the sf::Font class.
Whenever i have a sf::Font attribute in my class, the code randomly crashes.

I already had this issue and I fixed it by doing something a little bit tricky (Here is the old code) :

class Blob
{
private:
sf::Font *_font;

public:
Blob();
virtual ~Blob();
};
// Now the CPP source file

Blob::Blob()
{
_font = new sf::Font();
font->loadFromFile("arial.ttf");
}
 

Doing this makes the code crash.
Here is the fix :

// Header file
class Blob
{
private:
static sf::Font _font;

public:
// Ctor / Dtor
void init();
};

// CPP Source code

Blob::Blob()
{
}

void Blob::init()
{
_font.loadFromFile("arial.ttf");
}

sf::Font Blob::_font;
 

Now, about the real problem.
I have a FontLoader class, which is supposed to load all the fonts needed in the whole program.
It contains a std::map<std::string, sf::Font>.
Then when i want a font, i just have to do "FontLoader->getFont("NAME"), what returns the case in the map corresponding to "NAME"

But seeing the problem i show before, this cannot work !
Actually, it doesnt. I'm having a malloc() memory corruption when trying to allcate an object of type sf::Font
See the code here :
(Singleton is just a way to make my class global, available for all the other class)

class                           SFFontLoader : public Singleton<SFFontLoader>
{
  friend class Singleton<SFFontLoader>;

private:
  std::map<std::string, sf::Font *>       _fontMap;

public:
  void                  loadFont(std::string const &path, std::string const &name);
  sf::Font              &getFont(std::string const &name) {return *_fontMap[name];};

private:
  SFFontLoader();
  virtual ~SFFontLoader();

};
 

And the CPP file

#include                        "SFFontLoader.hh"

SFFontLoader::SFFontLoader()
{
}

SFFontLoader::~SFFontLoader()
{
}

void                            SFFontLoader::loadFont(std::string const &path,
                                                       std::string const &name)
{
  _fontMap[name] = new sf::Font;
  if (!(_fontMap[name]->loadFromFile(path)))
    std::cerr << "Error : failed to load font \"" << path << "\"" << std::endl;
}
 

And valgrind's console :

==5936== Invalid write of size 8
==5936==    at 0x5266038: std::vector<unsigned char, std::allocator<unsigned char> >::_M_fill_insert(__gnu_cxx::__normal_iterator<unsigned char*, std::vector<unsigned char, std::allocator<unsigned char> > >, unsigned long, unsigned char const&) (in /usr/lib/libsfml-graphics.so.2.1)
==5936==    by 0x5264D75: sf::Font::loadGlyph(unsigned int, unsigned int, bool) const (in /usr/lib/libsfml-graphics.so.2.1)
==5936==    by 0x526540D: sf::Font::getGlyph(unsigned int, unsigned int, bool) const (in /usr/lib/libsfml-graphics.so.2.1)
==5936==    by 0x5285BE9: sf::Text::updateGeometry() (in /usr/lib/libsfml-graphics.so.2.1)
==5936==    by 0x40C079: SFLabel::setText(std::string const&) (SFLabel.cpp:25)
==5936==    by 0x403100: main (main.cpp:52)
==5936==  Address 0xce07828 is 0 bytes after a block of size 104 alloc'd
==5936==    at 0x4C2C7A7: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5936==    by 0x40BAA8: SFFontLoader::loadFont(std::string const&, std::string const&) (SFFontLoader.cpp:24)
==5936==    by 0x40B6DE: SFRessourcesManager::loadDefaultRessources() (SFRessourcesManager.cpp:30)
==5936==    by 0x402DAD: main (main.cpp:27)
 

(Have fun reading this, the only usefull thing is at the top)

I have a solution to make this work :
Instead of map<string, sf::Font *>, juste do map<strring, sf::Font> then remove the new sf::Font line.
This works, the font appears, not memory corruption, but valgrinds yeld me some strange things (i think it tells me there's a problem when accessing to the _fontMap[name], well i actually have no idea why)

So, can anyone tell me why i cannot allocate an object of type sf::Font ?
Sorry, the post is a bit long but i wanted to be as accurate as possible.

Thanks for your help.

[eXpl0it3r]

I feel like you're mixing multiple things together here. Upfront I can tell you however that your problems will most likely be gone, once you don't use singletons, nor global variables, nor manual memory management.

There are enough discussions on the forum already why singletons and global variables are not a good idea, so I don't feel like repeating myself. Global SFML resources are a no-go, because SFML itself uses some global things and destruction order at global scope is undefined, thus can lead to crashes.
You'll have to think of a better code design, so you don't have to inject everything into the global scope.

By not using manual memory management and instead using RAII, you can basically say good bye to Valgrind, because changes of running into a leak go towards zero. Most of the time variables on the stack are enough and in about all the other cases smart pointers will be all you need.

[Nexus]

I can totally confirm everything eXpl0it3r said. In modern C++, there is almost never a good reason to use global variables, singletons, or manual memory management. It may take a while to get rid of those habitudes, but you will see that code becomes cleaner, simpler and safer.

Sounds too good to be true? Take a look at this example

[larta]

Thanks for your answers.

I read about RAII, and I still don't get how it's supposed to be "usefull". I mean, how am i supposed to load my ressources only once and access it everywhere i need it with it ? (That's a stupid question in fact, RAII seems to be more a way to code than something to apply line by line)

I know, many things have to change in my conception if i want to do it, but i don't get how to do.
For me, the only way to store things, load it only once and access it everywhere in the code is singleton / global variables and i don't understand how RAII would help me here. Maybe i didn't understand what RAII is supposed to be use for.
I think i need some help here.

[EDIT :
And about manual memory management, i'm used to manage everything by hand, and delete what i need when it needs to be deleted. Well maybe with this concept it can save me some times, seeing your example code Nexus)

[Nexus]

First, RAII and globals/singletons are unrelated. The former simply describes the principles that classes can take care of resource allocation and deallocation in their constructor and destructor. Combined with automatic variables, this allows for simple and safe usage of arbitrary resources in C++, most notably memory.

For me, the only way to store things, load it only once and access it everywhere in the code is singleton / global variables

Yes, and that's wrong. Just because you need something once, it doesn't mean it should be a singleton. In fact, that's mostly a bad idea.

If you need a single instance of something, then instantiate it only once (as a member variable, or local variable in a function). You can then pass that instance to other classes through function parameters -- just as you do with everything else.

And about manual memory management, i'm used to manage everything by hand, and delete what i need when it needs to be deleted.

Yes, and there is absolutely no good reason to do that manually, when you have RAII.

[wintertime]

The solution would be to not access everything from everywhere. If you structure your program in a tree-like way that shouldn't be difficult.
Then if you really need something at a second place put it inside a function parameter.

The good thing about unique_ptr is you just create something and it automatically destroys it when the scope ends, without you remembering, and is automatically exception safe. If really needed, you can still return it and it safely allows to give away responsibility of a pointer, without you worrying the user of the function may ignore the return value, because it then destroys it.