Destructor seems to be called 'early'

Question

Turned out it was a simple constructor miss-use problem. Please see the "edit" section for the updated information.

Sorry for yet another C++ dtor question... However I can't seem to find one exactly like mine as all the others are assigning to STL containers (that will delete objects itself) whereas mine is to an array of pointers.

So I have the following code fragment

#include<iostream>

class Block{
public:
    int x, y, z;
    int type;
    Block(){
        x=1;
        y=2;
        z=3;
        type=-1;
    }
};

template <class T> class Octree{
    T* children[8];
public:
    ~Octree(){
        for( int i=0; i<8; i++){
            std::cout << "del:" << i << std::endl;
            delete children[i];
        }
    }    
    Octree(){
        for( int i=0; i<8; i++ )
            children[i] = new T;
    }
    // place newchild in array at [i]
    void set_child(int i, T* newchild){
        children[i] = newchild;
    }
    // return child at [i]
    T* get_child(int i){
        return children[i];
    }
    // place newchild at [i] and return the old [i]
    T* swap_child(int i, T* newchild){
        T* p = children[i];
        children[i] = newchild;
        return p;
    }
};

int main(){
    Octree< Octree<Block> > here;
    std::cout << "nothing seems to have broken" << std::endl;
}

Looking through the output I notice that the destructor is being called many times before I think it should (as Octree is still in scope), the end of the output also shows:

del:0
del:0
del:1
del:2
del:3

Process returned -1073741819 (0xC0000005)   execution time : 1.685 s
Press any key to continue.

For some reason the destructor is going through the same point in the loop twice (0) and then dying.

All of this occures before the "nothing seems to have gone wrong" line which I expected before any dtor was called.

Thanks in advance :)

EDIT The code I posted has some things removed that I thought were unnecessary but after copying and compiling the code I pasted I no longer get the error. What I removed was other integer attributes of the code. Here is the origional:

#include<iostream>

class Block{
public:
    int x, y, z;
    int type;
    Block(){
        x=1;
        y=2;
        z=3;
        type=-1;
    }
    Block(int xx, int yy, int zz, int ty){
        x=xx;
        y=yy;
        z=zz;
        type=ty;
    }
    Block(int xx, int yy, int zz){
        x=xx;
        y=yy;
        z=zz;
        type=0;
    }
};

template <class T> class Octree{
    int x, y, z;
    int size;
    T* children[8];
public:
    ~Octree(){
        for( int i=0; i<8; i++){
            std::cout << "del:" << i << std::endl;
            delete children[i];
        }
    }

    Octree(int xx, int yy, int zz, int size){
        x=xx;
        y=yy;
        z=zz;
        size=size;
        for( int i=0; i<8; i++ )
            children[i] = new T;
    }
    Octree(){
        Octree(0, 0, 0, 10);
    }
    // place newchild in array at [i]
    void set_child(int i, T* newchild){
        children[i] = newchild;
    }
    // return child at [i]
    T* get_child(int i){
        return children[i];
    }
    // place newchild at [i] and return the old [i]
    T* swap_child(int i, T* newchild){
        T* p = children[i];
        children[i] = newchild;
        return p;
    }
};

int main(){
    Octree< Octree<Block> > here;
    std::cout << "nothing seems to have broken" << std::endl;
}

Also, as for the problems with set_child, get_child and swap_child leading to possible memory leaks this will be solved as a wrapper class will either use get before set or use swap to get the old child and write this out to disk before freeing the memory itself.

I am glad that it is not my memory management failing but rather another error. I have not made a copy and/or assignment operator yet as I was just testing the block tree out, I will almost certainly make them all private very soon.

This version spits out -1073741819.

Thank you all for your suggestions and I apologise for highjacking my own thread :$

SOLVED Issue with one constructor calling another.

Thanks all for your help and apologies for any time wasted :)

Answer 1

Someone defined a constructor and destructor but no copy constructor. It's the copies being destroyed that mess up the count. Follow the rule of three.

Answer 2

It's not going through the same loop twice. Your top level Octtree has 8 child Octrees, so you are seeing nested destruction. I'm not sure why it is dying though.

Answer 3

The problem is with the default constructor (which you didn't add until the edit!); it constructs a temporary Octree instance, where I expect you thought it would simply call the other constructor:

Octree(){
    Octree(0, 0, 0, 10);
}

It is this instance that you see being destructed before the crash. You then try to delete some children that were never newed (or initialised).

Extracting the initialisation code from Octree(int, int, int, int ) into a method will solve your problem. For example:

Octree(int xx, int yy, int zz, int size){
    init(xx, yy, zz, size);
}
Octree(){
    init(0, 0, 0, 10);
}

void init(int xx, int yy, int zz, int)
{
    x=xx;
    y=yy;
    z=zz;
    for( int i=0; i<8; i++ )
        children[i] = new T;

}

Alternatively, remove your default constructor and add default values to each of the arguments to your remaining constructor:

Octree(int xx = 0, int yy = 0, int zz = 0, int size = 10)
    :x(xx)
    ,y(yy)
    ,z(zz)
    ,size(size)
{
    for( int i=0; i<8; i++ )
        children[i] = new T;
}

However, do you really, really need to be handling raw pointers? If you do, then you'll almost certainly need to do something about copying before your class is useful.

In answer to your next question, "Nope!" "not until C++11!"

In c++11 (and its successors):

You can now delegate the construction, but the syntax is a little different to how you tried to express this:

Octree()
    :Octree(0, 0, 0, 10)
{
}

Answer 4

I suspect that you simply finished destructing and the program terminated before all the output was written to console. It's expected that the thing goes through 0 twice as it's the Octree<> 0 and then the Octree<Octree<>> 0.

You need to change your code to guarantee that the destructors run, AND that all the console I/O is done before process exit.

int main(){
 {
  Octree< Octree<Block> > here;
 }
 std::cout << "nothing seems to have broken" << std::endl;
 std::cin.get();
}

Of course, this code has plenty of OTHER flaws. But I'll start with the exact problem presented.

Answer 5

valgrind said:

==11907== HEAP SUMMARY:
==11907==     in use at exit: 0 bytes in 0 blocks
==11907==   total heap usage: 72 allocs, 72 frees, 1,536 bytes allocated
==11907== 
==11907== All heap blocks were freed -- no leaks are possible