How do I solve this memory leak

Question

I am trying to recreate the vector class and I believe there is a memory leak in my code, but I don't know how to solve it. Using the CRT Library in my Visual Studio, it tells me that there is a supposed memory leak that doubles for each time that reserve is called.

I am not quite sure why that is or if there even is a memory leak. The memory leak detection says that it is this line in the reserve function int* temp = new int[n];

This is what I understand to be happening in the reserve function:

Once the contents of arr are copied into temp, it's fine to delete arr. Assigning arr = temp should work because all I'm doing is making arr point to the same place as temp. Because arr was previously deleted, I only have 1 array in the heap and arr and temp both point to the same array so there should be no memory leak. Temp shouldn't matter because it disappears after it exits the scope. On subsequent calls to the reserve function, every thing repeats and there should only be one array in the heap which arr points to.

I do believe that my thinking is probably erroneous in some way.

    #include "Vector.h"

    namespace Vector {

    vector::vector() {
        sz = 0;
        space = 0;
        arr = nullptr;
    }
    vector::vector(int n) {
        sz = n;
        space = n;
        arr = new int[n];
        for(int i = 0; i < n; i++) {
            arr[i] = 0;
        }
    }
    void vector::push_back(int x) {
        if(sz == 0) {
            reserve(1);
        } else if (sz == space) {
            reserve(2*space);
        }
        arr[sz] = x;
        sz++;
    }
    void vector::reserve(int n) {
        if (n == 1) {
            arr = new int[1]; //arr was a nullptr beforehand
        }
        int* temp = new int[n]; 
        for(int i = 0; i < n; i++) {
            temp[i] = arr[i];
        }
        delete[] arr;
        arr = temp;
        space = n;      
    }

Answer 1

Your code assumes in vector::reserve(int n) that arr is null.

Instead maybe spilt up how reserve functions based on whether or not arr is null.

void vector::reserve(int n) {
    if (arr) { //handle case when arr is null
        space += n;
        arr = new int[space];
        //no need to copy anything!
    } else { //handle case when arr is not null
        int* tmp(new int[space + n]);
        for(int i = 0; i < space; i++) {
            tmp[i] = arr[i];
        }
        delete[] arr;
        arr = tmp;
        space += n;
    }
}

Also the above code assumes you mean to reserve space+n instead of allowing reserve to shrink the array as you'll lose data if you reserve less then a previous reserve. It's usually better practice to not use assumptions about a pointer's state when working with them because when your code gets more complex the assumptions can end up getting forgotten or more obscure.

Answer 2

I have same issues too. I have created two pointers that points in the same address in heap. When I'm trying too deallocate the memory, and the result is only one pointer that can do that, it's the first pointers that point that address. The second or third pointers that points that address doesn't have an authority to deallocate the memory, but only the first pointers who have that authority.

Example

int *a = new int[5];
int *b = a;
int *c = a;

Pointers b and c doesn't have an authority too dealloacte the memory address that pointers a pointed. Therefore, the memory wasn't deallocated if i'm saying delete[] b nor delete[] c, they didn't have an authority for doing that. Then I tried to write delete [] a and that worked. I don't have an real answers, and I just trying to approaching through my try and errors that I have done. And that's what I got.

Actually this case is violating the rules, but C++ still allowed us to do it, it's called undefined behaviors. We are violating the rules of delete[] operators by, but C++ still allowed you to do, and as the result, you get unexpected output.

Answer 3

Not too much wrong in there.

Bugs

    if (n == 1) {
        arr = new int[1]; //arr was a nullptr beforehand
    }

The comment cannot be guaranteed. Nothing prevents multiple calls of resize including a call of reserve(1), and that will leak whatever memory was pointed at by arr. Instead consider

    if (arr == nullptr) {
        arr = new int[n]; //arr was a nullptr beforehand
    }

now the comment is guaranteed to be true.

The copy loop overshoots the end of arr every time the size of the array is increased.

    for(int i = 0; i < n; i++) {
        temp[i] = arr[i];
    }

arr is only good up to arr[sz-1]. If n is greater than space, and it almost always will be, arr[i] wanders into the great wilds of Undefined Behaviour. Not a good place to go.

    for(int i = 0; i < n && i < sz; i++) {
        temp[i] = arr[i];
    }

Checks both n and sz to prevent overrun on either end and copying of data that has not been set yet. If there is nothing to be copied, all done.

Targets of opportunity

The class needs a destructor to release any memory that it owns (What is ownership of resources or pointers?) when it is destroyed. Without it, you have a leak.

vector::~vector() {
    delete[] arr;
}

And if it has a destructor, the Rule of Three requires it to have special support functions to handle (at least) copying of the class or expressly forbid copying.

// exchanges one vector for the other. Generally useful, but also makes moves 
// and assignments easy
void vector::swap(vector& a, vector& b)
{
    std::swap(a.sz, b.sz);
    std::swap(a.space, b.space);
    std::swap(a.arr, b.arr);
}

// Copy constructor
vector::vector(const vector& src):
    sz(src.sz),
    space (src.space),
    arr(new int[space])
{
    for(int i = 0; i < sz; i++) {
        arr[i] = src.arr[i];
    }
}

// move constructor
vector::vector(vector&& src): vector()
{
    swap(*this, src);
}

// assignment operator
vector::vector& vector::operator=(vector src)
{
    swap(*this, src);

    return *this;
}

The Copy Constructor uses a Member Initializer List. That's the funny : bit.

The assignment operator makes use of the Copy and Swap Idiom. This isn't the fastest way to implement the assignment operator, but it is probably the easiest. Start with easy and only go to hard if easiest doesn't meet the requirements.