Debug Assertion Failed (_BLOCK_TYPE_IS_VALID)... Is this solution valid?

Question

Disclaimer: This is for a homework assignment. We have to create a calculator that uses class objects to hold numbers. We have a class called Element which goes in a database and holds a class called Container that stores numerical data.

I'm getting the aforementioned error with this piece of code. Container is declared here:

bool assign(const char* lhs, const char* rhs) {
    Container value;
    bool isValid = parse(rhs, value);
    if (isValid)
    {
            // Code omitted, but you add it to a database
        return true;
    }
    return false;
}

Calls this:

bool parse(const char* input, Container& output)
{
    if (IsANumber(input)) // Outside script that checks if input is a scalar
    {
        output = Container(input);
        return true;
    }
}

Basically, this part of the script (of which I have only posted a small part) reads expression, and if it is valid, returns a Container object holding input. I get the error above, which is being caused by my destructor:

Container::~Container()
{
    delete[] data;
}

Where data is the pointer that stores input. I have to comment the body of the destructor out, or my code doesn't work. I assume the destructor is trying to delete Container, but is failing because data doesn't actually exist (for reasons I do not understand). I can avoid this problem by doing something like this:

bool parse(const char* input, Container& output)
{
    if (IsValid(input))
    {
        Container* temp = new Container(input);
        output = *temp;
        return true;
    }
}

This adds temp to the heap and avoids deletion issues. But I can't help but feel like this is a memory leak, since temp will never be deleted. Can anyone point me in the right direction?

Here is all the constructor, copy constructor, and destructor code for the Container class:

// null constructor
Container::Container()
{
    m_nRow = 0;
    m_nCol = 0;
    data = new double[1];
    *(data) = 0;
}

// scalar constructor
Container::Container(double d)
{
    m_nRow = 1;
    m_nCol = 1;
    data = new double[1];
    *(data) = d;
}

// Copy constructor
Container::Container(const Container& copy)
{
    m_nRow = copy.m_nRow;
    m_nCol = copy.m_nCol;

    data = new double[m_nRow * m_nCol];
    for (int i = 0; i < m_nRow; i++)
    {
        for (int j = 0; j < m_nCol; j++)
        {
            *(m_aData + i * m_nCol + j) = *(copy.m_aData + i * m_nCol + j);
        }
    }
}

Container::~Container()
{
    //delete[] data;
}

Answer 1

This line is problematic:

output = Container(input);

You didn't declare a copy assignment operator so you get the default one which is member to member assignment. data pointer will then be duplicated. Since you are using a local object that goes out of scope at the end of the function, data is deleted. Later, when output is destructed, you hit the double delete problem. That is why by introducing a dynamic allocation and not releasing it, it "fixes" the problem.

You should declare a copy assignment operator to also copy data content like in your copy constructor.

You are missing one part of the rule of three. When you need to declare a copy constructor and destructor, you also need a copy assignment operator.

You also have a leak in your copy constructor, you should delete data before re-allocating it.

EDIT after your comment

You have a leak in copy constructor because in your other constructor, you allocate an array of 1 element. In the copy constructor, you allocate for the other size without first deleting the previously allocated data array.

For declaring a copy assignment operator you normally return a reference like so:

CMatrix& CMatrix::operator=(const CMatrix& mat)

but you don't theoretically need to allocate a new object. Although the allocate and swap is a common implementation pattern.