memory leak

Discussion in 'C++' started by mast2as@yahoo.com, Jul 5, 2006.

  1. Guest

    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
     
    , Jul 5, 2006
    #1
    1. Advertising

  2. TB Guest

    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;
    > }


    You delete every node in m_nodes,

    > delete m_currentNode;


    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;


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

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

    --
    TB @ SWEDEN
     
    TB, Jul 5, 2006
    #2
    1. Advertising

  3. Guest

    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;
    }
     
    , Jul 5, 2006
    #3
  4. schrieb:
    > 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


    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 ) {}


    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;
    > };


    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)

    --
    Thomas
     
    Thomas J. Gritzan, Jul 5, 2006
    #4
  5. Guest

    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
     
    , Jul 5, 2006
    #5
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. =?Utf-8?B?Y3liZXJzdHJpa2U=?=

    datagrid memory leak?

    =?Utf-8?B?Y3liZXJzdHJpa2U=?=, Jan 3, 2005, in forum: ASP .Net
    Replies:
    0
    Views:
    472
    =?Utf-8?B?Y3liZXJzdHJpa2U=?=
    Jan 3, 2005
  2. s.subbarayan

    Dynamic memory allocation and memory leak...

    s.subbarayan, Mar 18, 2005, in forum: C Programming
    Replies:
    10
    Views:
    720
    Eric Sosman
    Mar 22, 2005
  3. Richard Heathfield

    Leak or no leak ??

    Richard Heathfield, Jul 10, 2006, in forum: C Programming
    Replies:
    4
    Views:
    364
    Richard Heathfield
    Jul 10, 2006
  4. cham
    Replies:
    5
    Views:
    776
  5. Mark Probert
    Replies:
    4
    Views:
    339
    Mark Probert
    Feb 9, 2005
Loading...

Share This Page