std::bind causes segmentation fault on destructor

Question

So something strange is happening. I'm binding a callback function (which I've done several times in other parts of the code) but for some reason this time it's causing the destructor to be called, and a segfault on it…

Here's the outline of my code with all the superfluous stuff stripped

GUILogicComponent.h

class GUILogicComponent : public LogicComponent {
private:
    std::function<void()> callback;
    EventListenerComponent* eventListener;
    SpriteComponent& sprite;
public:
    GUILogicComponent(EventListenerComponent* el, SpriteComponent& s, std::function<void()> cb);
    ~GUILogicComponent();
    void clicked(int x, int y);
};

GUILogicComponent.cpp

GUILogicComponent::GUILogicComponent(EventListenerComponent* el, SpriteComponent& s, std::function<void()> cb) : eventListener(el), sprite(s), callback(cb) {
    eventListener->addMouseFunction(std::bind(&GUILogicComponent::clicked, *this, std::placeholders::_1, std::placeholders::_2), SDL_BUTTON_LEFT);
    // TODO: Binding causes seg fault
}

GUILogicComponent::~GUILogicComponent() {
    delete eventListener;
}

void GUILogicComponent::clicked(int x, int y) {
    if (sprite.pointInSprite(x, y))
        callback();
}

The GDB error

Thread 3 received signal SIGSEGV, Segmentation fault.
0x0000000100004e73 in Thor_Lucas_Development::GUILogicComponent::~GUILogicComponent (
    this=<error reading variable: Cannot access memory at address 0x7fff5f3ffff8>)
    at Components/GUILogicComponent.cpp:11
11  GUILogicComponent::~GUILogicComponent() {

Not sure what's going on. Strangely enough, removing the other constructor parameters (removing sprite and callback and commenting out all the relevant code) leaves me with this error instead.

Thread 3 received signal SIGSEGV, Segmentation fault.
0x00000001000027b8 in std::__1::__tree<std::__1::__value_type<Thor_Lucas_Development::Mousecode, std::__1::function<void (int, int)> >, std::__1::__map_value_compare<Thor_Lucas_Development::Mousecode, std::__1::__value_type<Thor_Lucas_Development::Mousecode, std::__1::function<void (int, int)> >, std::__1::less<Thor_Lucas_Development::Mousecode>, true>, std::__1::allocator<std::__1::__value_type<Thor_Lucas_Development::Mousecode, std::__1::function<void (int, int)> > > >::destroy(std::__1::__tree_node<std::__1::__value_type<Thor_Lucas_Development::Mousecode, std::__1::function<void (int, int)> >, void*>*) (this=0x10070c768, __nd=0x1007365d0)
    at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/__tree:1377
1377        if (__nd != nullptr)

Here's the Mousecode definitions in my Util.h

/**
 * Used to store a mouse state for mapping to functions.
 */
struct Mousecode {
    Uint8 button; /**< The mouse button pressed (i.e\. SDL_BUTTON_LEFT). */
    Uint8 state; /**< The state of the mouse button (i.e\. SDL_RELEASED). */
};

inline bool const operator==(const Mousecode& l, const Mousecode& r) {
    return (l.button == r.button) && (l.state == r.state);
}

inline bool const operator<(const Mousecode& l, const Mousecode& r) {
    return (l.button < r.button) || ((l.button == r.button) && (l.state < r.state));
}

Here's what EventListenerComponent is doing

void EventListenerComponent::addMouseFunction(std::function<void(int, int)> func, Uint8 button, Uint8 state) {
    Mousecode code = {button, state};
    mouseFunctionMap[code] = func;
}

And mouseFunctionMap

std::map<Mousecode, std::function<void(int, int)>> mouseFunctionMap;

Any help will be greatly appreciated… Thank you!

`bind` was mostly obsolete when it was added to C++11; I see nothing here that could not be done more clearly with a lambda. Consider doing that; less magic, more explicit in what is a copy and what is a reference. — Yakk - Adam Nevraumont, Sep 26 '17 at 02:07
Try `std::bind(&GUILogicComponent::clicked, this, ...)`, without a star before `this`. I suspect you are making a copy of `GUILogicComponent` and passing that to `bind`. — Igor Tandetnik, Sep 26 '17 at 02:39

score 1 · Accepted Answer · answered Sep 26 '17 at 02:43

1

You are creating a temporary copy passing *this here:

eventListener->addMouseFunction(std::bind(&GUILogicComponent::clicked, *this, std::placeholders::_1, std::placeholders::_2), SDL_BUTTON_LEFT);

which is immediatly destroyed after this line.

You are not showing other copy/move constructors and operators, and I suspect you haven't coded them. See What is The Rule of Three?
This leads to double deletion of the same pointer.

You should use a std:::unique_ptr as your component appears to have its ownership.

answered Sep 26 '17 at 02:43

O'Neil

3,790
4
16
30

Wow, I feel incredibly stupid! Thank you, I just removed the * to not dereference this. Side note: Should I consider using shared_ptr's for my entities? The entities have ownership but sometimes individual components need to know about each other. The way I've been doing it is passing a pointer* to the entity to delete in the destructor, but passing a reference& to the other components. – Thor Correia Sep 26 '17 at 14:01
@ThorCorreia If your components share that ownership, then yes, use a shared_ptr instead. – O'Neil Sep 26 '17 at 14:07

std::bind causes segmentation fault on destructor

1 Answers1