Memory leak because of new() or malloc

Question

I'm trying to implement an alternative to usual contiguous dynamic arrays, where I am using a vector of pointers, each is pointing to a constant size array, referring to it as XVector.

It works fine for a certain limit of inputs - say 150 elements -, but beyond that it starts throwing exceptions.

I tried to use "new and delete" instead of "malloc and free", it did increased the limit - say about 1200 elements -, but still exists the same problem.

I'm using C++.

Here's my main program:

XVector<int> xv;

for(int i=0;i<1210;i++){
    int temp = rand()%100;
    xv.pushBack(temp);
}


for(int i=0;i<xv.size();i++){
    cout<<xv.getValue(i)<<" ";
}

cout<<"\n\n"<<xv.capacity();
return 0;

And here's is the XVector (The class of theD header file:

private:
    const int b = 10;
    vector<T*> arr;
    int currentIndex;
    int maxSize;


protected:
    void expand(){
        T* temp = new T[b];
        arr.push_back(temp);
        maxSize+=(b);

    }

    void shrink(){
        delete[] arr[arr.size()-1];
        arr[arr.size()-1] = NULL;
        arr.pop_back();
        maxSize-=(b);
    }

    int ceil(float num) {
        int inum = (int)num;
        if (num == (float)inum) {
            return inum;
        }
        return inum + 1;
    }

    pair<int,int> position(int index){
        pair<int,int> pos;
        float result = ((float)index/b);
        pos.first = result; //Row #
        pos.second = ceil((result - pos.first)*b); //Exact cell in the row
        return pos;
    }



    public:

    XVector(){
        currentIndex=0;
        maxSize=b;
        arr.reserve(120);
        arr.push_back(new T[b]);
    }

    void pushBack(T value){
        if(currentIndex>=maxSize-1){expand();}
        pair<int,int> indeces = position(currentIndex);
        arr[indeces.first][indeces.second]=value;
        currentIndex++;
    }

    void popBack(){
        currentIndex--;
        if(maxSize>=currentIndex+(2*b)){shrink();}
    }

    T getValue(int index){
        pair<int,int> indeces = position(index);
        return arr[indeces.first][indeces.second];
    }

    void setValue(int index,T value){
        pair<int,int> indeces = position(index);
        arr[indeces.first][indeces.second] = value;
    }

    int capacity(){
        return maxSize;
    }

    int size(){
        return currentIndex;
    }

    bool empty(){
        return currentIndex==0?true:false;
    }

PS: I tried to use Valgrind, but failed to identify the exact problem.

Looks like you are building a [deque](http://en.cppreference.com/w/cpp/container/deque) — NathanOliver, Feb 15 '18 at 15:50
I don't see that you implement a destructor, copy constructor, or copy assignment operator. — Fred Larson, Feb 15 '18 at 15:52
@FredLarson I understand many things are missing, but this problem stopped me from continuing. Do you mean that any of those missing could lead to memory leak?! — Ahmed Hammad, Feb 15 '18 at 15:57
Handy reading: [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — user4581301, Feb 15 '18 at 16:00
BTW, this has nothing to do with your problem, but it drives me crazy: `return currentIndex==0?true:false;`. Just `return currentIndex==0;` is quite sufficient and much more readable. — Fred Larson, Feb 15 '18 at 16:00
I think you should look into std::unique_ptr and std::shared_ptr. You will still have to worry about the moving / copying etc; but it will remove your memory leak with no effort at all. — UKMonkey, Feb 15 '18 at 16:27
_"it starts throwing exceptions"_ Is the content of these exceptions a secret? — Lightness Races in Orbit, Feb 15 '18 at 17:20
@AhmedHammad: _"Your advises are worthless, guys. Thank you"_ Wow, nice guy. — Lightness Races in Orbit, Feb 15 '18 at 17:20
I find the main problem of your code but you gave tik to other person and that person give negative point. check you app again. your problem isn't memory leak. — hamed, Feb 15 '18 at 17:27
I typed out some "advices" about (not) using `new` or `malloc`, but I deleted them. Best of luck to you. — Jive Dadson, Feb 15 '18 at 18:48

score 4 · Answer 1 · answered Feb 15 '18 at 16:06

4

Your program leaks memory because you never free the pointers in a destructor. You must implement a destructor to solve your memory leak (in addition to a move constructor, copy constructor, assignment copy, and assignment move).

In addition to valgrind, you can use ASAN which has better output and also runs faster.

answered Feb 15 '18 at 16:06

user1413793

9,057
7
30
42

1

this program has index out of range exception. the main problem isn't memory leak. memory leak doesn't lead to application crash in short term. – hamed Feb 15 '18 at 17:00
@hamed: That entirely depends on how much you're leaking, and how often – Lightness Races in Orbit Feb 15 '18 at 17:18
@user1413793 , this application has another problem that leads app to crash befor memory leakage but you dont mention the main problem. this app has access violation . if you solve memory leakage you have exception yet. – hamed Feb 15 '18 at 17:23
@hamed that may be, but the question was about the memory leak. Best to solve one problem at a time or the question sprawls and becomes less applicable to future programmers with a similar problem. – user4581301 Feb 15 '18 at 17:26
your idea is right when the big problem doesn't appear in question. but the big access violation is obvious and you don't speak about that. – hamed Feb 15 '18 at 17:30

score 0 · Accepted Answer · edited Feb 15 '18 at 17:36

The main problem of your code that leads your code to crash isn't memory leak. Totally memory leak doesn't leads to crash in short term. in memory leak case your application works until there is enough space on your RAM and then if your RAM being full , crash occurs. you make a mistake in position() method in finding second dimension index.

For example when you call position(index:29) because of implementation of float and float precision result of (result - pos.first)*b is 9.00000095. It means its result has a little difference with real result(9). and then you call ceil(9.00000095) and it return 10 for result. It means you access index 10 for your second dimension whereas you can use index from 0 to 9 and you have index out of range access that leads you to crash after some period of time and your program may have undefined behavior.

the correct sample for position method is :

pair<int, int> position(int index){
    pair<int, int> pos;
    float result = ((float)index / b);
    pos.first = result; //Row #
    pos.second = index%b; // changed line
    return pos;
}

Finally you should define destructor and delete all memories you allocated by new operator. All of vector element(arrays) need to be deleted.

_"Totally memory leak doesn't leads to crash in short term"_ That entirely depends on how much you're leaking, and how often. — Lightness Races in Orbit, Feb 15 '18 at 17:19
Totally ignores the asked question. Useful information, but useless in the context of the question. — user4581301, Feb 15 '18 at 17:19
_"in memory leak case your application works until there is enough space on your ram"_ You're omitting several layers of abstraction. — Lightness Races in Orbit, Feb 15 '18 at 17:19
This was Awesome! This very little tip worked magically! Thank you so much, buddy. — Ahmed Hammad, Feb 15 '18 at 17:31

Memory leak because of new() or malloc

2 Answers2