Reassigning variable, delete called twice (C++)

Question

My programming background is the Java world, but I've just started learning C++. I've stumbled across this rather trivial and probably pretty noobish problem that somehow puzzles me as a Java programmer:

I have a class with an array which is initialized via new in the constructor and deleted in the destructor. Now when I create an object of this class and assign another object of this class to the same variable (at least that is what I think it is), the delete[] method in the destructor seems to be called twice when the variables leave the scope (in this case the main() function) (the debugger gives me an _BLOCK_TYPE_IS_VALID assertion failed warning).

Why is that? Why isn't the deconstructor called before I assign a new object to f? How could I explicitely delete Foo(1)? What exactly happens here?

class Foo{
    private:
        int *field;

    public:

    Foo(int size){
       field = new int[size];
    }

   ~Foo(){
       delete[] field;
    }
};


int main(){
    Foo f = Foo(1);
    f = Foo(2);
}

Relevant: http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three........................................................................................ More relevant: http://dl.dropbox.com/u/6101039/Modern%20C++.pdf — chris, Mar 18 '13 at 23:37
You could save yourself all the trouble by using an `std::vector` instead of the dynamically allocated array. — juanchopanza, Mar 18 '13 at 23:45

Drew Dormann · Accepted Answer · 2013-03-19T02:49:23.120

6

There is something in the C++ world called the Rule Of Three.

A class will automatically generate a destructor, copy constructor, and assignment operator for you.

If you have to manually define one of those functions, you probably have to define all three of them.

In your case, you should define the two copy functions, so that a copy of Foo gets its own copy of field. Add these two functions:

class Foo{

    Foo( const Foo &f ) {
        size = f.size;
        field = new int[size]; 
        std::copy( f.field, f.field + size, field );
    }

   Foo& operator=( const Foo &f ) {
       // Leverage the logic that was already written in the copy constructor
       Foo tmp(f);
       std::swap( *this, temp );
       return *this;
   }
};

Note that I'm assuming that you've stored size in the Foo object. You would probably need to store that information anyway in a real-life application

edited Mar 19 '13 at 02:49

answered Mar 18 '13 at 23:37

Drew Dormann

59,987
13
123
180

@Treecj Lucian Grigore makes good points too. If you're coming from Java, it will be *much breezier* if you avoid pointer management wherever you can, such as with his `vector` suggestion. That would put you on the "zero" side of the RoT. – Drew Dormann Mar 18 '13 at 23:49
Thx again. There might be a little error in your definition of the copy constructor however: The old field does not exist (does it?) in the context of this constructor, hence cannot be deleted...? At least when I tried implementing it that way I got a runtime error. – Treecj Mar 19 '13 at 02:44

score 2 · Answer 2 · answered Mar 18 '13 at 23:38

You're not following the rule of three, which is bad. Because the default copy constructor does a shallow copy, all your automatic variables (f and the temporaries) have the field member pointing to the same int, which is destroyed multiple times when the destructor is called.

Implement a proper copy constructor, or use C++ idioms and avoid manual management alltogether - i.e. use a std::vector instead.

Reassigning variable, delete called twice (C++)

2 Answers2