double free or corruption when trying to erase in unordered_map

I have a class Block that inherits from a class case :

class Case {
public:
    Case(sf::Vector2f const& pos, sf::IntRect const& textureRect, int type = 1);

protected:
    int type_;

    sf::Vector2f pos_;
    sf::FloatRect hitBox_;
    sf::IntRect textureRect_;
};

class Block : public Case {

public:
    Block(sf::Texture* const& texture, sf::Vector2f const& pos, sf::IntRect const& textureRect, int const& type = 2);

    sf::Sprite& getSprite();

private:
    std::shared_ptr<sf::Texture> texture_;
    sf::Sprite sprite_;

};

(Both constructor are really basic, I'm not using any new anywhere)

and I have an unordered_map of unordered map to stock my blocks :

std::unordered_map<int, std::unordered_map<int, Block>> blocks_;

But when I try to delete one :

if(blocks_[i%lenght].find((i-i%lenght)/lenght) != blocks_[i%lenght].end())
    blocks_[i%lenght].erase((i-i%lenght)/lenght);

I get thos error :

double free or corruption (out)

I tried to print the destructor, only the destructor from Block is called before I get this error.

It's been around 2 hours I'm looking for a solution so I finally ask it here, Thanks !

Solution

Your problem is with the constructor:

Block::Block(sf::Texture *const &texture, sf::Vector2f const &pos,
             sf::IntRect const &textureRect, int const &type)
    : Case(pos, textureRect, 2), texture_(texture),
      sprite_(*texture_, textureRect_) {}

While it is not necessarily wrong to write it that way, it would be wrong if you pass the same Texture to multiple Blocks:

sf::Texture *tex = new sf::Texture(/* ... params ... */);

Block b1(tex, /* ... remaining params ... */);
Block b2(tex, /* ... remaining params ... */);

Now two separate shared_ptr think that they are the only owner of tex, so as soon as one of those Blocks is deleted the Texture is deleted as well, so if both blocks are deleted then you have a double free.

As of that this is considered an anti pattern. In general if you work with smart pointers, then you should see a raw pointer as not owning pointers and you should never construct a smart pointer from a not owning pointer. _{(However, there is an exception to this rule, if you work with libraries that don't use smart pointers, then you need check if the raw pointer they return or receive are considered as owing pointers and if so it might be valid to convert that raw pointer to a smart pointer.)}

Your code should look that way:

Block::Block(const std::shared_ptr<sf::Texture> &texture, sf::Vector2f const &pos,
             sf::IntRect const &textureRect, int const &type)
    : Case(pos, textureRect, 2), texture_(texture),
      sprite_(*texture_, textureRect_) {}

And you should construct your Blocks like that:

std::shared_ptr<Texture> tex = std::make_shared<Texture>(/* ... params ... */);

Block b1(tex, /* ... remaining params ... */);
Block b2(tex, /* ... remaining params ... */);

This does not mean that you should never use raw pointers. If you e.g. would have a function that would draw the texture to screen, then a raw pointer is perfectly fine:

void draw_texture( sf::Texture * const tex) {
   // do some drawing but don't store tex somewhere
   // so tex is only used within that draw_texture call
}

Then you would call it like that:

std::shared_ptr<Texture> tex = std::make_shared<Texture>(/* ... params ... */);

draw_texture(tex.get());