Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C++ Explanation for this delete[] Error?

After making a lot of changes to a project, I created an error that took me quite a while to track down.

I have a class which contains a dynamically allocated array. I then create a dynamic array of this class. I can then delete[] that array. But, if I replace an item in the array before deleting it, it causes an error. In debug mode, it gives an assertion message from dbgdel.cpp "Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)". Here is a small program to demonstrate.

class SomeClass {
public:
    int *data;

    SomeClass() {
        data = nullptr;
    }
    SomeClass(int num) {
        data = new int[num];
    }
    ~SomeClass() {
        if (data != nullptr) { 
            delete[] data;
        }
    }
};

int main(int argc, char *args[]) {
    SomeClass *someArray = new SomeClass[10];

    //If you comment this out, there is no error.  Error gets thrown when deleting
    someArray[0] = SomeClass(10);

    delete[] someArray;
    return 0;
}

I'm curious, why does this happen? When the item in the array gets replaced, its destructor gets called. Then the new item allocates its data in a location separate from the array. Then delete[] calls the destructors of all the objects in the array. When the destructors get called, they should delete the item's data array. I can't imagine what the problem is, but I'd like if someone could explain.

like image 539
Janz Dott Avatar asked Feb 26 '14 09:02

Janz Dott


1 Answers

Your class is broken: It has a non-trivial destructor, but you do not define copy constructors and copy assignment operators. That means that the class cannot be correctly copied or assigned-to (since the destructible state is not copied or assigned appropriately), as you are noticing in your example code.

You can either make your class uncopiable (in which case your code won't compile any more), or move-only, in which case you need to define move construction and move-assignment, or properly copyable by implementing a deep copy of the data.

Here's how, add the following definitions:

Non-copyable:

SomeClass(SomeClass const &) = delete;
SomeClass & operator(SomeClass const &) = delete;

Moveable-only:

SomeClass(SomeClass const &) = delete;
SomeClass(SomeClass && rhs) : data(rhs.data) { rhs.data = nullptr; }
SomeClass & operator(SomeClass const &) = delete;
SomeClass & operator(SomeClass && rhs) {
    if (this != &rhs) { delete data; data = rhs.data; rhs.data = nullptr; }
    return *this;
}

Copyable:

SomeClass(SomeClass const & rhs) : ptr(new int[rhs->GetSizeMagically()]) {
    /* copy from rhs.data to data */
}
SomeClass & operator(SomeClass const & rhs) {
    if (this == &rhs) return *this;

    int * tmp = new int[rhs->GetSizeMagically()];
    /* copy data */
    delete data;
    data = tmp;
}
// move operations as above

The upshot is that the nature of the destructor determines the invariants of the class, because every object must be consistently destructible. From that you can infer the required semantics of the copy and move operations. (This is often called the Rule of Three or Rule of Five.)

like image 76
Kerrek SB Avatar answered Oct 27 '22 08:10

Kerrek SB