Search code examples
c++c++11callbackstd-functionstdbind

Using std::bind and std::function with a class member causes the callback to use an old object reference?

This is confusing, but basically what I've got is a class that has a callback function using the c++11 std::function and std::bind. Everything is working fine. But when I need to reset everything by reassigning the object, it seems the new object isn't getting all of the references correct. Here is an example program that demonstrates my problem:

#include <functional>
#include <list>
#include <iostream>
#include <string>

class OtherClass
{
public:
    OtherClass() = default;

    void RegisterCallback(std::function<void(void)> f) {
        callback = f;
    }

    void PrintThings() {
        callback();
    }

    std::function<void(void)> callback;
};

class MyClass
{
public:
    MyClass() {
        list_of_things.push_back("thing1");
        list_of_things.push_back("thing2");
        list_of_things.push_back("thing3");
        list_of_things.push_back("thing4");

        other_class.RegisterCallback(std::bind(&MyClass::MyFunction, this));
    }

    void PrintThings() {
        MyFunction();
        other_class.PrintThings();
    }

    void MyFunction() {
        auto a = this;
        for (auto& thing: list_of_things)
        {
            std::cout << thing << std::endl;
        }
    }

    OtherClass other_class;
    std::list<std::string> list_of_things;
};

int main()
{
    MyClass my_class;
    my_class.PrintThings();
    my_class = MyClass();
    my_class.PrintThings();
    std::cout << "done" << std::endl;
}

It's a little confusing, but basically if you compile with debug flags and step through it, you'll find that the first time we call my_class.PrintThings() it prints them twice; once for the MyFunction() call and once for the other_class.PrintThings() call which calls MyFunction as a callback. Then, we replace the object with my_class = MyClass(), invoking a new constructor and all that. When we step through, we find that it prints the list on the call to MyFunction but not on the call to other_class.PrintThings(). MyFunction has a variable a that I'm using to see the address of the object; the second time through a has a different address depending on whether MyFunction was called as a callback from OtherClass.

In this example, the offending ghost object just has an empty list (presumably as a result of being destroyed) but in the actual program where I encountered this, it was filled with garbage memory and caused a segmentation fault. I also noticed some weird behavior with my debugger; when it reached the ghost object it wouldn't just step in or over, it would skip the function unless I put a breakpoint inside there.

What is going on? Why isn't the callback bound properly the second time through? Is there something special I need to do in the destructor or something? Am I missing some fundamental understanding of function pointers or std::bind?

Solution

  • What is going on?

    Undefined behaviour

    Why isn't the callback bound properly the second time through?

    Because you created a new OtherClass as part of assigning a new MyClass to my_class. You have not initialised its callback.

    Is there something special I need to do in the destructor or something?

    In the destructor, assignment and copy constructors. This is because you're storing the address of "myself" in yourself. This is all well and good until the object changes address, which it will when copied. Note that in the code below, all three are taken care of by the use of a smart_ptr.

    One solution is to refactor MyClass to use the pimpl idiom. i.e. the class is a wrapper around an implementation whose address never changes.

    #include <functional>
#include <iostream>
#include <list>
#include <string>
#include <memory>

class OtherClass
{
public:
    OtherClass() = default;

    void RegisterCallback(std::function<void(void)> f) {
        callback = f;
    }

    void PrintThings() {
        callback();
    }

    std::function<void(void)> callback;
};


class MyClass
{
    struct Impl
    {
        Impl()
        {
            list_of_things.push_back("thing1");
            list_of_things.push_back("thing2");
            list_of_things.push_back("thing3");
            list_of_things.push_back("thing4");
        }

        void MyFunction() 
        {
            for (auto& thing: list_of_things)
            {
                std::cout << thing << std::endl;
            }
        }

        void PrintThings() {
            MyFunction();
            other_class.PrintThings();
        }

        OtherClass other_class;
        std::list<std::string> list_of_things;
    };

    std::unique_ptr<Impl> impl_;

public:
    MyClass() 
    : impl_(std::make_unique<Impl>())
    {
        impl_->other_class.RegisterCallback(std::bind(&Impl::MyFunction, impl_.get()));
    }

    void PrintThings() {
        impl_->PrintThings();
    }


};

int main()
{
    MyClass my_class;
    my_class.PrintThings();
    my_class = MyClass();
    my_class.PrintThings();
    std::cout << "done" << std::endl;
}

    expected output:

    thing1
thing2
thing3
thing4
thing1
thing2
thing3
thing4
thing1
thing2
thing3
thing4
thing1
thing2
thing3
thing4
done