Reusing a pointer of double array without memory leak in C++ via "delete" crashes the program

Question

I've tried the (somewhat questionable) convention of deleteing after usage, but that doesn't seem to work. The program is supposed to receive an input of a single integer, sort a randomly created array, and print elapsed time for sorting, yet when I leave the delete in there, the program abnormally ends without even a warning after I do the input. In other words, it crashes. However, when I comment out just the delete line, the program executes perfectly.

The MWE is measuring time for a simple Quick Sort Algorithm, and since this is a school project I cannot change the main() function and using the QuickSort class and its pointers, etc..

The only things I can change are the stuff that goes in the various functions, and although it does seem that set(double*, int) can just be integrated into the constructor, that's not a possible option here for some reason.

The objective is to define a default double* in the constructor, and then delete it and copy input_array into this->arr in set:

EDIT: I use Windows 10 and the GCC C++ Compiler from MinGw-w64. All compilations have been executed in the Windows Command Prompt.

main.cpp

#include <iostream>
#include <cstdlib> // Just for good measure, though this shouldn't be needed
#include "Sort.hpp"

bool check_quick(QuickSort *quick_sort) {
    int i = 0;
    while(i < (quick_sort->size) - 1) {
        if (quick_sort->arr[i] > quick_sort->arr[i + 1]) break;
        ++i;
    } if (i == (quick_sort->size) - 1) return true;
    else return false;
}

int main() {
    int n; cin >> n;
    double *input_array = new double[n];
    srand((unsigned int)time(NULL));
    for (int k = 0; k < n; k++) input_array[k] = (double)((rand() % n));

    QuickSort* quick_sort = new QuickSort();
    quick_sort->set(input_array, n);
    quick_sort->run();
    if (check_quick(quick_sort)) {
        cout << "QuickSort is validated" << endl << endl;
    } delete quick_sort;
}

Sort.hpp

#define CLOCKS_PER_SECOND 1000
#include <iostream>
#include <ctime>
#include <iomanip> // Use to call setprecision(4)
using namespace std;

class QuickSort {
    friend bool check_quick(QuickSort*); // Give access for private variables
public:
    void print_time() const {
        cout << "QuickSort : " << fixed << setprecision(4) << seconds
             << " sec" << endl;
        // << fixed << setprecision(4) always prints to four numbers after point
    }
    QuickSort() {
        this->arr = new double[10];
        for (int i = 0; i < 10; ++i) this->arr[i - 1] = i; // Set default array
        seconds = clock(); // Set current Millisecond to starting time
    }
    ~QuickSort() {
        delete this->arr; // Delete array in object of this class
    }
    void sorter(double *arr, int begin, int end) { // Sorting Recursive Function
        // Size of array without pivot is: end - begin
        int pivot = arr[end];
            // PIVOT is element at end of subarray "arr[begin...end]"
        int i = begin, j = end;
        while (i <= j) {
            while (arr[i] < pivot) i++; // Increment until arr[i] is larger than
            while (arr[j] > pivot) j--; // Decrement until arr[j] is lesser than
            if (i <= j) { // If the larger element precedes lesser element
                swap(arr[i], arr[j]); // Call Swap function
                i++; j--;
            } // If i is larger than j now, i was 1 lesser than j before,
              // effectively leaving no more elements to scan.
        }
        if (begin < j) sorter(this->arr, begin, j); // Recursive, larger part
        if (end > i) sorter (this->arr, i, end); // Recursive, lesser part
    }
    void run() {
        sorter(this->arr, 0, this->size - 1); // Call Sorter function
        seconds = (double)(clock() - seconds) / (double)(CLOCKS_PER_SECOND);
            // Calculate Difference of Ticks and divide by Ticks per second.
            // Now, `seconds` is passed seconds with millisecond precision.
    }
    void set(double *arr, int size) {
        this->arr = new double[size]; // Make new array of `size` size
        for (int i = 0; i < size; i++) this->arr[i] = arr[i]; // Copy input_arr
        for (int i = 0; i < size; i++) cout << this->arr[i] << endl; // Copy input_arr
        this->size = size; // Save global `size` to object of class
    }
    void swap(double &p, double &q) { // Swap Function
                                      // Ampersand precedence to change input
        double x = p; // Temporary `double` saver
        p = q; // p is q
        q = x; // q is x, which is p
    }
private:
    double *arr;
    int size;
    double seconds;
};

Answer 1

In your QuickSort constructor you are writing outside the bounds of the array that you are allocating:

this->arr = new double[10];
for (int i = 0; i < 10; ++i) this->arr[i - 1] = i; // Set default array

In the first iteration i is 0 so this->arr[i - 1] writes to the element -1. This is only crashing when you call delete as if you don't the runtime doesn't notice this corruption and exits cleanly.

Presumably just changing i-1 to i will produce the desired behaviour.

Answer 2

The first line of QuickSort::set:

this->arr = new double[size]; // Make new array of `size` size

leaks the array allocated in QuickSort's constructor. You need to delete[] that array first, before assigning arr to point to another one:

delete[] arr;
this->arr = new double[size]; // Make new array of `size` size

---

If this were the real world, and not a shcool assignment, it would be much better not to use a raw pointer in this case. Rather, a std::vector<double> would be much more appropriate.

Answer 3

A quick look reveals that you are using delete instead of delete[]..

this line:

delete this->arr; // Delete array in object of this class

should be:

delete[] this->arr; // Delete array in object of this class

Additionally, as per the convention of delete you should also do this:

delete[] this->arr;
this->arr = nullptr; // This is important, else your are double deleting which is calling for trouble.