1

The following code creates a double pointer B** to a B*. It'll use that pointer to allocate memory for another pointer which will point to a B instance created when start() is called:

class A:

class A
{
public:
    A()
    {
        fb = new B*;
        *fb = NULL;
    }

    ~A()
    {
        if(*fb)
            delete *fb;

        delete fb;
    }

    B** getfb()
    {
        return fb;
    }

private:
    B** fb;
};

class B:

class B
{
public:
    B()
    {
        B** fb = a->getfb();

        *fb = this;
    }

    ~B()
    {
        B** fb = a->getfb();

        delete *fb;            // <--- stack overflow

        *fb = NULL;
    }

private:
    A* a;
};

start() (a member function of class C):

void C::start()
{
    B** fb = a->getfb();        // 'a' is a pointer to an 'A' instance

    if(*fb == NULL)
        B* f = new B;
}

So, whenever I call start() and then call ~B(), I get a stack overflow!

Unihedron
  • 10,902
  • 13
  • 62
  • 72
Jonas
  • 1,019
  • 4
  • 20
  • 33

3 Answers3

7

Sounds right.

The type of fb is B** so the type of *fb is B*.

So when you say delete *fb you are calling the destructor for class B which is a recursive call that does not make progress toward a base case, hence the stack overflow.

Ray Toal
  • 86,166
  • 18
  • 182
  • 232
6

In B() you assign *fb = this; and then:

~B()
{
  ...
  delete *fb; // delete this;
  ...
}

.. is equivalent to calling delete this; inside the destructor ~B() again and again and thus stackoveflow due to too many recursive functions.

Here is a nice thread for Is it safe to delete this.

Community
  • 1
  • 1
iammilind
  • 68,093
  • 33
  • 169
  • 336
  • Well, the conclusions in the highest ranked response there (`delete this` is generally considered bad form, and has a bad smell) are clearly wrong. Depending on how transactions are managed, `delete this` may be the cleanest and best way for most deletes. (It's certainly the most OO---an object is responsible for itself.) – James Kanze Jun 14 '12 at 08:21
4

This is not surprising. You are inside the destructor for class B, access THE SAME instance of B that is currently having its destructor called and then deleting it again. This will lead to recursion in destructor calls and hence your stack overflow. Your deletion of fb by class A is sufficient to clean your memory up here you dont want class B to delete itself.

You also do not need to do what you are doing in the class B constructor. The code you have in class A makes sense, what you have in class B is dangerous and needless. In class B you have no function to set its member a. This means the pointer is uninitialised and you then try assign this instance of class B to the member pointer of class B of this uninitialised instance of A.

    B()
    {
        B** fb = a->getfb(); // a never gets set to a meaningful value!!!!!

        *fb = this;
    }
mathematician1975
  • 21,161
  • 6
  • 59
  • 101
  • for not initialising `a`, I'v taken care of that I just wanted the code to be clear as possible. I guesse you're right, I should of have writen everything correctly. thanks for the answer. – Jonas Jun 14 '12 at 07:44