Freeing a destructor causes a memory leak
I need to write a stack class template using arrays in C ++ for my assignment.
Here 's my code:
#include <iostream>
template <typename Type>
class Stack {
private:
int top;
Type items[];
public:
Stack() { top = -1; };
~Stack() { delete[] items; };
void push(Type);
bool isEmpty() const;
Type pop();
Type peek() const;
};
int main (int argc, char *argv[]) {
Stack<double> st;
return 0;
}
template<typename Type>
void Stack<Type>::push(Type item) {
top++;
if(top == sizeof(items) / sizeof(Type)) {
Type buff[] = new Type[top];
std::copy(items,items+top,buff);
delete[] items;
items = new Type[2*top];
std::copy(buff,buff+top,items);
delete[] buff;
}
items[top] = item;
}
template<typename Type>
bool Stack<Type>::isEmpty() const {
return top == -1 ? true : false;
}
template<typename Type>
Type Stack<Type>::pop() {
//TODO
return items[top--];
}
template<typename Type>
Type Stack<Type>::peek() const{
//TODO
return items[top-1];
}
It is compiled using " g++ -Wall
", however I got this error when running the program:
* discovered by glibc *
./lab3: munmap_chunk(): invalid pointer: 0x00007fff41a3cdf8
After testing a bit with GDB, I found that the error came from the line:
'free[] items' in the destructor.
I don't understand why freeing the array leads to a memory leak? Any hints?
source to share
You don't have new elements, so you cannot delete them in the destructor. Assuming you require "Stack" to be a dynamically sized class, then the array of elements must be dynamically allocated, in which case the declaration for the elements must be
Type *items; (as hacker mentions above)
and the constructor must call "items = new Type [ARRAY_SIZE]" to allocate memory, or at least assign the "items" pointer to NULL first.
Edit based on comments -
To fulfill and protect the memory allocation responsibilities for this class, you must also include a copy constructor and an assignment operator that allocates new memory and copies the values stored in the elements to the new object. This avoids that the pointer is simply copied by a compiler-generated copy constructor or assignment operator, which would result in multiple objects pointing to the same dynamically allocated memory. When the first of these objects is destroyed, this memory will be removed. Further use of the now deleted memory by other objects that share the pointer is likely to crash.
Instead of adding code here, I refer you to the code in Martin's answer for this question .
source to share
You should delete[]
only use what you have explicitly allocated with new[]
. Your member is items
not a dynamically allocated array, so you shouldn't free it as if it were.
On the other hand, you have
Type items[];
which doesn't actually allocate space in the object instances for the stack.
source to share
items - a pointer to Type. This pointer must be initialized before it can be used (it's a wild pointer as it is). Which is why your program crashes. What this happens to the destructor is a coincidence. It might just as well have happened before. For example. in memory, push () is overwritten to the location pointed to by the items.
You need to allocate memory for multiple items. See below.
You already have the logic to dynamically grow the array in place. But an array in C ++ doesn't know its size (it's just a pointer of some type). You need to track from your current capacity / size. Thus, instead of
sizeof(items)
define a new item to store the current capacity.
eg:.
int capacity;
and in the constructor:
capacity = 100;
items = new Type[capacity];
The last line highlights several elements and is the main solution to your problem.
And in push ():
if (top == capacity / sizeof(Type)) {
The redistribution logic must change. For example:.
capacity *= 2; items = new Type[capacity];
instead of items = new Type [2 * top];
I just sketched a solution here. Yon can fill in the details.
Note : you only have one instance of the Stack in your program; main () body:
Stack<double> st;
If instead you have to assign one Stack instance to another instance like
{
Stack<double> st;
Stack<double> st2 = st; //Copy constructor will be invoked for st2. If not defined the compiler generated copy constructor will do a copy of the pointer value.
Stack<double> st3;
st3 = st; //Assignment operator will be invoked for st3. If not defined the compiler generated assignment operator will do a copy of the pointer value.
}
then the copy constructor (to work for st2) and the assignment operator (to work for st3) for Stack should be (make a copy of the reference member Items). Otherwise, double or triple deletion in the destructor and undefined behavior (like fail) will result in st, st2, and st3 out of scope in the closing parenthesis.
source to share