memory leak

M

mast2as

Hi guys

I wrote a short program to test a Tree class... I plan to use it a lot
in my application so thought I would do a memory leak check before I
start using it. The code seems good to me in a glance (new & delete
seem to be called for all dynamically crated objects) but when I run
valgrind, valgrind reports a memory leak.
The problem seems to come when i start using push().

Could anyone help me with this please ? I just don't see what i am
doing wrong.

thank you, mark

#include <iostream>
#include <deque>

using namespace std;

template<typename T>
class TreeNode
{
public:
TreeNode( ) : m_parent( NULL ) {
cout << "new node" << endl;
//cout << "new data" << endl;
//m_data = new T;
}

~TreeNode() {
cout << "delete node\n";
//cout << "delete data\n";
//delete m_data;
}

void setData( T data ) { m_data = data; }
void setParent( TreeNode<T> *node ) { m_parent = node; }
TreeNode<T> *getParent() const { return m_parent; }
T getData() const { return m_data; }
private:
TreeNode<T> *m_parent;
T m_data;
};

template<typename T>
class Tree
{
public:

Tree() {
m_currentNode = new TreeNode<T>;
m_currentNode->setParent( NULL );
}

~Tree() {
cout << "Tree destructor " << m_nodes.size() << endl;
while ( ! m_nodes.empty() ) {
TreeNode<T> *node = m_nodes.back();
m_nodes.pop_back();
delete node;
}
delete m_currentNode;
}

void push( T data )
{
TreeNode<T> *node = new TreeNode<T>;
cout << "size of Node " << sizeof( *node ) << endl;
node->setParent( m_currentNode );
node->setData( data );
m_nodes.push_back( node );

m_currentNode = node;
}

void pop() {
if ( m_currentNode->getParent() ) {
m_currentNode = m_currentNode->getParent();
} else {
cout << "Tree::data out of range (abort)" << endl;
}
}

T getCurrentData() const { return m_currentNode->getData(); }
int getTreeSize() const { return m_nodes.size(); }

private:
TreeNode<T> *m_currentNode;
deque<TreeNode<T> *> m_nodes;
};

int main()
{
Tree<int> tree;
tree.push( 1 );
return 0;
}

/// running it
new node
new node
size of Node 8
Tree destructor 1
delete node
delete node

/// VALGRIND REPORT
==12771==
new node
new node
size of Node 8
Tree destructor 1
delete node
delete node
==12771== Invalid free() / delete / delete[]
==12771== at 0x1B904616: operator delete(void*)
(vg_replace_malloc.c:156)
==12771== by 0x8048B2B: Tree<int>::~Tree() (in p_proto1/test)
==12771== by 0x80489BD: main (in p_proto1/test)
==12771== Address 0x1BB457C0 is 0 bytes inside a block of size 8
free'd
==12771== at 0x1B904616: operator delete(void*)
(vg_replace_malloc.c:156)
==12771== by 0x8048B05: Tree<int>::~Tree() (in p_proto1/test)
==12771== by 0x80489BD: main (in p_proto1/test)
==12771==
==12771== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 17 from
1)
==12771== malloc/free: in use at exit: 1288 bytes in 2 blocks.
==12771== malloc/free: 4 allocs, 3 frees, 1808 bytes allocated.
==12771== For counts of detected errors, rerun with: -v
==12771== searching for pointers to 2 not-freed blocks.
==12771== checked 2370936 bytes.
==12771==
==12771==
==12771== 8 bytes in 1 blocks are definitely lost in loss record 1 of 2
==12771== at 0x1B90406F: operator new(unsigned)
(vg_replace_malloc.c:133)
==12771== by 0x8048DD0: Tree<int>::Tree() (in p_proto1/test)
==12771== by 0x804899F: main (in p_proto1/test)
==12771==
==12771== LEAK SUMMARY:
==12771== definitely lost: 8 bytes in 1 blocks.
==12771== possibly lost: 0 bytes in 0 blocks.
==12771== still reachable: 1280 bytes in 1 blocks.
==12771== suppressed: 0 bytes in 0 blocks.
==12771== Reachable blocks (those to which a pointer was found) are not
shown.
==12771== To see them, rerun with: --show-reachable=yes
 
T

TB

(e-mail address removed) skrev:
Hi guys

I wrote a short program to test a Tree class... I plan to use it a lot
in my application so thought I would do a memory leak check before I
start using it. The code seems good to me in a glance (new & delete
seem to be called for all dynamically crated objects) but when I run
valgrind, valgrind reports a memory leak.
The problem seems to come when i start using push().

Could anyone help me with this please ? I just don't see what i am
doing wrong.

thank you, mark

#include <iostream>
#include <deque>

using namespace std;

template<typename T>
class TreeNode
{
public:
TreeNode( ) : m_parent( NULL ) {
cout << "new node" << endl;
//cout << "new data" << endl;
//m_data = new T;
}

~TreeNode() {
cout << "delete node\n";
//cout << "delete data\n";
//delete m_data;
}

void setData( T data ) { m_data = data; }
void setParent( TreeNode<T> *node ) { m_parent = node; }
TreeNode<T> *getParent() const { return m_parent; }
T getData() const { return m_data; }
private:
TreeNode<T> *m_parent;
T m_data;
};

template<typename T>
class Tree
{
public:

Tree() {
m_currentNode = new TreeNode<T>;
m_currentNode->setParent( NULL );
}

~Tree() {
cout << "Tree destructor " << m_nodes.size() << endl;
while ( ! m_nodes.empty() ) {
TreeNode<T> *node = m_nodes.back();
m_nodes.pop_back();
delete node;
}
Click to expand...

You delete every node in m_nodes,
delete m_currentNode;
Click to expand...

then you delete m_currentNode, which if you look below
actually already is deleted.
}

void push( T data )
{
TreeNode<T> *node = new TreeNode<T>;
cout << "size of Node " << sizeof( *node ) << endl;
node->setParent( m_currentNode );
node->setData( data );
m_nodes.push_back( node );

m_currentNode = node;
Click to expand...

^^^^^^^^^^^^^^^^^^^^^ This statement is causing you
all the trouble!

With your first push, you loose the first allocated TreeNode
created in the constructor.
 
M

mast2as

of course ! thank you very much... well the whole class wasn't really
well thought anyway...

I just did the following changes and it seems to work now. Thanks again

#include <iostream>
#include <deque>

using namespace std;

template<typename T>
class TreeNode
{
public:
TreeNode( ) : m_parent( NULL ) {
cout << "new node" << endl;
//cout << "new data" << endl;
//m_data = new T;
}

~TreeNode() {
cout << "delete node\n";
//cout << "delete data\n";
//delete m_data;
}

void setData( T data ) { m_data = data; }
void setParent( TreeNode<T> *node ) { m_parent = node; }
TreeNode<T> *getParent() const { return m_parent; }
T getData() const { return m_data; }
private:
TreeNode<T> *m_parent;
T m_data;

};

template<typename T>
class Tree
{
public:
Tree() : m_depth( 0 ) {}

~Tree() {
cout << "Tree destructor " << m_nodes.size() << endl;
while ( ! m_nodes.empty() ) {
TreeNode<T> *node = m_nodes.back();
m_nodes.pop_back();
delete node;
}
//delete m_currentNode;
}

void push( T data )
{
TreeNode<T> *node = new TreeNode<T>;
cout << "size of Node " << sizeof( *node ) << endl;
if ( m_depth ) {
node->setParent( m_currentNode );
}
node->setData( data );
m_nodes.push_back( node );
m_depth;
m_currentNode = node;
}

void pop() {
if ( m_depth ) {
m_currentNode = m_currentNode->getParent();
m_depth--;
} else {
cout << "Tree::data out of range (abort)" << endl;
}
}

T getCurrentData() const { return m_currentNode->getData(); }
int getTreeSize() const { return m_nodes.size(); }

private:
int m_depth;
TreeNode<T> *m_currentNode;
deque<TreeNode<T> *> m_nodes;
};

int main()
{
Tree<int> tree;
tree.push( 1 );
tree.push( 2 );
return 0;
}
 
T

Thomas J. Gritzan

of course ! thank you very much... well the whole class wasn't really
well thought anyway...

I just did the following changes and it seems to work now. Thanks again
Click to expand...

See below.
#include <iostream>
#include <deque>

using namespace std;

template<typename T>
class TreeNode
{
public:
TreeNode( ) : m_parent( NULL ) {
cout << "new node" << endl;
//cout << "new data" << endl;
//m_data = new T;
}

~TreeNode() {
cout << "delete node\n";
//cout << "delete data\n";
//delete m_data;
}

void setData( T data ) { m_data = data; }
void setParent( TreeNode<T> *node ) { m_parent = node; }
TreeNode<T> *getParent() const { return m_parent; }
T getData() const { return m_data; }
private:
TreeNode<T> *m_parent;
T m_data;

};

template<typename T>
class Tree
{
public:
Tree() : m_depth( 0 ) {}
Click to expand...

Add: m_currentNode(0)
~Tree() {
cout << "Tree destructor " << m_nodes.size() << endl;
while ( ! m_nodes.empty() ) {
TreeNode<T> *node = m_nodes.back();
m_nodes.pop_back();
delete node;
}
//delete m_currentNode;
}

void push( T data )
{
TreeNode<T> *node = new TreeNode<T>;
cout << "size of Node " << sizeof( *node ) << endl;
if ( m_depth ) {
node->setParent( m_currentNode );
}
node->setData( data );
m_nodes.push_back( node );
m_depth;
m_depth++;

m_currentNode = node;
}

void pop() {
if ( m_depth ) {
m_currentNode = m_currentNode->getParent();
m_depth--;
} else {
cout << "Tree::data out of range (abort)" << endl;
}
}

T getCurrentData() const { return m_currentNode->getData(); }
int getTreeSize() const { return m_nodes.size(); }

private:
int m_depth;
TreeNode<T> *m_currentNode;
deque<TreeNode<T> *> m_nodes;
};
Click to expand...

Thats not a tree, its a stack (well, actually a stack is a special kind
of tree).

Why don't you use deque<> or list<> directly, or implement a thin
wrapper over it? You don't need the new/delete stuff:

template <typename T>
class stack
{
public:
stack() {}
~stack() {}

void push(const T& value) { m_data.push_back(value); }
void pop() { m_data.pop_back(); }
const T& top() const { return m_data.back(); }
size_t size() const { return m_data.size(); }

private:
std::deque<T> m_data;
};

(not tryied to compile)
 
M

mast2as

Hi Thomas

Yes sorry for some reason the m_depth++ disappeared and ended up being
m_depth is the post (?)

I was using deque originally but them problem is that i need a stack of
objects which are actually pretty big (in terms of memory) and a lot of
them will be saved on that stack I need the same mechanism as
pop_back() and push_back() excepted that I need to keep the entire
"tree" structure until termination of the program. Meaning when I call
pop() i actually want to go one level down in the queue but want to
keep the object in the stack *I don't want to get rid of it with a
pop_back()*

Basically they are a bunch of objects (class Object lets say) that
points to the TreeNodes stored in the Tree object.

Tree<int> atree;
atree.push( 1 );
atree.puhs( 2 );
Object A, B, C;
A.setWith( atree.getCurrentInteger() );
B.setWith( atree.getCurrentInteger() );
atree.pop(); // go down one level but don't get rid of 2 as A and B
points to it
C.setWith( atree.getCurrentInteger() );
atree.puhs( 3 );

A.doSomethingUsingIndex(); // 2
B.doSomethingUsingIndex(); // 2
C.doSomethingUsingIndex(); // 1

It's a tree in the sense that it ends up looking that

A--|
|-B
|-C
|-D
E-|
|-F --|
|-G
|-H
|-I
J

Hope it make sense, Mark
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Infinite loop problem 1
C program: memory leak/ segmentation fault/ memory limit exceeded 0
I need help 1
Possible memory leak? 11
Is there in memory tree structure faster than std::map? 11
memory leak with deque ? 4
pthread memory leaks 15
strstream Memory Leak 5

Members online

No members online now.
Total: 23 (members: 0, guests: 23)
Robots: 473

Forum statistics

Threads
473,769
Messages
2,569,581
Members
45,056
Latest member
GlycogenSupporthealth

Latest Threads

Top