Destructor of dynamically allocated array causing Heap buffer error

Question

So I have an assignement to write a version of the vector library without using it. I need to use dynamically allocated arrays, However their destructor is causing errors. Here is my code:

    class orderedVector {

    friend ostream& operator<<(ostream&, const orderedVector&);
    friend istream& operator>>(istream&, orderedVector&);

public:
    orderedVector(int = 9);
    ~orderedVector();
    int getSize() const;
    int getCapacity() const;
    void doubleCapacity();
    bool find(int) const;
    void insert(int);
    void remove(int);
    void findSum(int);

private:
    int size;
    int capacity;
    int* ptr;
};

orderedVector::orderedVector(int c) {
    capacity = c;
    size = 0;
    ptr = new int[c];
}

orderedVector::~orderedVector() {
    delete[] ptr;
}

int orderedVector::getSize() const {
    return size;
}

int orderedVector::getCapacity() const {
    return capacity;
}

void orderedVector::doubleCapacity() {

    int newCapacity = capacity * 2;

    orderedVector temp(capacity);
    for (int i = 0; i < size; i++) {
        temp.ptr[i] = ptr[i];
    }
    ptr = new int[newCapacity];

    for (int i = 0; i < size; i++) {
        ptr[i] = temp.ptr[i];
    }

    capacity = newCapacity;
}

bool orderedVector::find(int number) const {
    for (int i = 0; i <= size; i++)
        if (number == ptr[i])
            return true;
    return false;
}

void orderedVector::insert(int number){

    if (find(number)) {
        return;
    }

    if (size == capacity)
        doubleCapacity();

    if (size == 0)
        ptr[0] = number;

    else {
        int checkpoint = size;
        for (int i = 0; i < size; i++) {
            if (number < ptr[i]) {
                checkpoint = i;
                break;
            }
        }

        for (int i = size-1; i >= checkpoint; i--) {
            ptr[i + 1] = ptr[i];
        }

        ptr[checkpoint] = number;
    }
    size++;
}           

void orderedVector::remove(int number) {

    if (find(number) == false)
        cout << "Number does not exist in the vector.\n";
    else {
        int checkpoint = 0;

        for (int i = 0; i <= size; i++)
            if (ptr[i] == number)
                checkpoint = i;

        for (int i = checkpoint; i < size; i++)
            ptr[i] = ptr[i + 1];
    }
    ptr[size] = 0;
    size--;
}

void orderedVector::findSum(int number) {

    for (int i = 0; i <= size; i++) {
        for (int j = i+1; j <= size; j++) {
            if (ptr[i] + ptr[j] == number) {
                cout << "The two numbers that have a sum of " << number
                    << " are " << ptr[i] << " and " << ptr[j] << endl;
                return;
            }
        }
    }

    cout << "No two numbers found that give a sum of " << number << endl;
}

ostream& operator<<(ostream& output, const orderedVector& vector) {
    for (int i = 0; i < vector.size; i++)
        output << vector.ptr[i] << "    ";

    output << endl;
    return output;
}

istream& operator>>(istream& input, orderedVector& vector) {
    int x = 0;
    for (int i = 0; i < vector.capacity; i++) {
        input >> x;
        vector.insert(x);
    }
    return input;
}

It appears that when I initialize an element, and the array is completely filled, I get this error: HEAP CORRUPTION DETECTED, CRT detected that the application wrote to memory after end of heap buffer.

I know it's the destructor because when I remove it, no errors occur. Also, when my vector element is not completely filled (the capacity is bigger than the size), no error occurs. I would like to know how to fix this, or how the delete operator works in general, and why it causes an error if I fill my dynamically created array.

Answer 1

A dynamic allocation may be deallocated at most once. As such, when you deallocate in a destructor, it is important to establish a class invariant that at most one instance uniquely owns the same pointer. If such invariant is violated, then the destructors of those instances may attempt to delete same pointer twice, which leads to undefined behaviour. Which is bad.

Your class orderedVector indeed deallocates in its destructor. However, the (implicitly generated) copy constructor of the class copies the member variable, which violates the class invariant of unique ownership.

You failed to provide a MCVE, but it is reasonable to guess that your program makes a copy of an instance of orderedVector, leading to undefined behaviour due to multiple delete of the same pointer.

The solution is to either make the class non-copyable and non-movable, or follow the rule of 3 (or 5) i.e. implement the copy (and move) constructors and assignment operators in a way that enforces the invariant of unique ownership.