C++ pointer is allocated -> error: pointer being freed was not allocated

Question

I have a destructor which checks if a pointer has been allocated, if it has, it deletes it:

ShadeRec::~ShadeRec(){

    cout << "before deleting ShadeRec" << endl;
    if(material){
        cout << material << endl;
        delete material;
        material = NULL;
    }
    cout << "after deleting ShadeRec" << endl;
}

The first pointer goes through fine and then on the second one the program gives me the error.

I have checked with the couts and there is something inside the pointer, which makes sense as it got into the if statement... so why is it giving me the error?

Constructors:

ShadeRec::ShadeRec(World& world)
    :   hit(false),
        material(NULL),
        hitPoint(),
        localHitPoint(),
        normal(),
        ray(),
        depth(0),
        colour(0),
        t(0.0),
        w(world)
{}

ShadeRec::ShadeRec(const ShadeRec& sr)
    :   hit(sr.hit), 
        material(sr.material), 
        hitPoint(sr.hitPoint), 
        localHitPoint(sr.localHitPoint), 
        normal(sr.normal), 
        ray(sr.ray), 
        depth(sr.depth), 
        colour(sr.colour), 
        t(sr.t),
        w(sr.w)
{}

Material operator=

Material& 
Material::operator= (const Material& rhs) {
    if (this == &rhs)
        return (*this);

    return (*this);
}

Matte is a child of Material:

Matte* matte1 = new Matte;

which as both of these:

Matte& operator= (const Matte& rhs);

virtual Material* clone(void) const;

Answer 1

Most likely, it is this:

• material is a member variable if ShadeRec
• Your constructor of ShadeRec takes/sets the pointer
• The copy-ctor is not defined or, if it is, just copies the pointer, not creating a new instance of the object it points to.
• There are at least two instances of ShadeRec, maybe it's copied as a parameter or return value.
• The first dtor deletes the object and set its own pointer to NULL
• The second dtor works on a different object, its pointer is still set
• Bam!

To make it visible, also print this in your dtor and you'll see it's a different instance of ShadeRec.

---

After your edit: It's this line:

material(sr.material),

which should create a copy of the object, not just copy the plain pointer. Another option, often preferable, is using a std::shared_ptr (if you can use C++11, otherwise check out boost::shared_ptr), but be aware that copies of ShadeRec will then share the same material instance.

Given what you have shown so far, replace the above line from the copy-ctor with

material(sr.material->clone()),

and see if it works.

Answer 2

Problem

You are not defining a deep copy constructor. When you make a copy of a ShadeRec object the pointers are copied as they are therefore leading to a conflict between the two classes.

When using it like:

ShadeRec a;
ShadeRec b(a);

both instances contains the same address, which means that when the first one frees his pointer correctly, the second will still have to free its pointers (which is already freed by the other one).

Solution examples

Here's an example of a correct deep copy constructor. Considering material to be a pointer to a dynamic allocated Material object and that the Material class has a properly defined copy constructor:

ShadeRec::ShadeRec(const ShadeRec& sr)
    :   hit(sr.hit), 
        material(0), // <--- NULL 
        hitPoint(sr.hitPoint), 
        localHitPoint(sr.localHitPoint), 
        normal(sr.normal), 
        ray(sr.ray), 
        depth(sr.depth), 
        colour(sr.colour), 
        t(sr.t),
        w(sr.w)
{
    if (sr.material)
        material = new Material(*(sr.material));
}

Here's an example of how to write a proper assignment operator:

Material& 
Material::operator= (const Material& rhs) {
    if (this != &rhs) {
        if (material)
            delete material;
        // make member per member assignments here
        material = new Material(*(rhs.material));
    }
    return (*this);
}

More

If you can, you should use smart pointers which, as of C++11, are implemented as std::shared_ptr on the STL or, previous to C++11, available in the boost library as boost::shared_ptr.