1

I have been debugging an issue involving an std::function (created from std::bind) where the issue boils down to a more fundamental question.

An example follows below, but in short it's about the case when a class in one way or another stores a pointer to itself (or an instance variable) in its constructor and the class itself is then reassigned. From my experiments it seems the stored pointer will then no longer point to the class (or its instance variable). I wonder what are the best practices to avoid these issues?

An example:

#include <iostream>

class Interface
{
public:
    Interface() {};
    virtual ~Interface() {};

    virtual void callback() = 0;
};

class B
{
public:
    B() : interface(NULL) {};
    virtual ~B() {};

    void set_interface(Interface* interface)
    {
        this->interface = interface;
    }

    void print()
    {
        std::cout << "B is at: " << this << std::endl;
        std::cout << "     Interface (A) from B is at: " << interface << std::endl;
    }

private:
    Interface* interface;
};

class A : public Interface
{
public:
    A()
    {
        b.set_interface(this);
    };
    virtual ~A() {};

    virtual void callback()
    {
        std::cout << "Callback" << std::endl;
    }

    void print()
    {
        std::cout << "A is at: " << this << std::endl;
        b.print();
    }

private:
    B b;
};


int main(int argc, char** argv)
{
    A a;
    a.print();

    std::cout << "------" << std::endl;
    a = A();
    a.print();

    return 0;
}

When I run this on my machine I get:

A is at: 0x7ffdb8910af0
B is at: 0x7ffdb8910af8
     Interface (A) from B is at: 0x7ffdb8910af0
------
A is at: 0x7ffdb8910af0
B is at: 0x7ffdb8910af8
     Interface (A) from B is at: 0x7ffdb8910b10

So, at first everything is fine. The pointer to Interface from B points to the address of A. However, after reassigning (a=A()) the pointer to Interface no longer points to A! I realize this is because as the constructor of A is running to create the second instance, it is in a new memory location and stores the interface pointer to this new memory location. When a is then reassigned the object is moved in to the old location which invalidates the pointer to Interface.

This seems like a common use case to me. Should one always avoid referencing memory in constructors of objects?

In my case I would like to bind a callback function and pass it to a class member, it seems I cannot do it in the constructor then and must do it at a later time?

Isak T.
  • 335
  • 1
  • 3
  • 16
  • `a = A()` copied data, while pointers stay the same. See `&a` is `0x7ffdb8910af0` and `&a.b` is `0x7ffdb8910af8`. After `a = A()` then `&a` is still `0x7ffdb8910af0` and and `&a.b` is `0x7ffdb8910af8`, but only the data are copied. That means that the value of `a.b.interface` changed, cause it was copied from a newer instance, but the location of data (ie. `&a`) didn't change. The object `A()` is created temporary, then all data from it are copied using 'implicitly-defined copy assignment' and the memory from `A()` is released, but the data are stored inside `a`. – KamilCuk Aug 22 '18 at 09:31

1 Answers1

4

A is at fault:

A()
{
    b.set_interface(this);
};

You define a constructor for A which handles a resource: itself. According to the rule of 0/3/5, A should have a user-defined copy constructor and an assignment operator, and if it makes sense, their move conterparts.

A()
{
    b.set_interface(this);
};

A(A const&)
{
    b.set_interface(this);
}

A& operator=(A const&)
{
    b.set_interface(this);
    return *this;
}
YSC
  • 38,212
  • 9
  • 96
  • 149
  • 1
    Great. But A& operator=(A const&) should return *this right? – Isak T. Aug 22 '18 at 09:33
  • To elaborate a bit, `this->interface = interface` calls the copy constructor of A. I have to admit, I would have expected it to just assign the pointer, seeing that interface is a pointer. There must be something I don't understand here... – GeckoGeorge Aug 22 '18 at 09:50
  • @IsakT. you're right ^^ fixed – YSC Aug 22 '18 at 09:54