confusing delete problem

Discussion in 'C++' started by Joerg Toellner, Feb 24, 2004.

  1. Hi Group,

    i stumbled over a delete problem which confuses me. I'm not very experienced
    with C++ programming and so i am not able to understand what's going on
    here.

    I have the following situation:

    -------------SNIP---------------------
    // File: map.cpp

    class tile;

    class map
    {
    map();
    ~map();
    // ... some other unrelated stuff here
    tile *m_MyTiles[144];
    }

    void map::map()
    {
    int x;
    for(x = 0; x < 144; x++)
    m_MyTiles[x] = new tile;
    }

    void map::~map()
    {
    for(x = 0; x < 144; x++)
    delete m_MyTiles[x];
    }
    -------------SNIP---------------------

    When i construct a new instance of class map and later delete it again my
    program crash at the delete command in the destructor. It seems that the (in
    the constructor of class map) created 144 instances of class Tile will be
    deleted somehow automagically. When i comment out the code in the destructor
    of class map, all works fine. But i assume that then i will leave some
    memory leaks behind, right?

    Can s.o. please direct me in the right direction what i'm doing wrong? Must
    i delete my with new created instances of the tiles in the destructor of map
    (where the pointers are member variables)? Or who will do this for me behind
    the scene? Or when this will leave memory leaks, how do i delete my
    tiles-objects correctly without a program crash?

    Sorry for the dumb question, but i can't figure it out myself.

    TIA very much
    Joerg Toellner
    Joerg Toellner, Feb 24, 2004
    #1
    1. Advertising

  2. Joerg Toellner

    Clark Cox Guest

    In article <c1fpmi$pce$02$-online.com>,
    "Joerg Toellner" <> wrote:

    > Hi Group,
    >
    > i stumbled over a delete problem which confuses me. I'm not very experienced
    > with C++ programming and so i am not able to understand what's going on
    > here.
    >
    > I have the following situation:
    >
    > -------------SNIP---------------------
    > // File: map.cpp
    >
    > class tile;
    >
    > class map
    > {
    > map();
    > ~map();
    > // ... some other unrelated stuff here
    > tile *m_MyTiles[144];
    > }
    >
    > void map::map()
    > {
    > int x;
    > for(x = 0; x < 144; x++)
    > m_MyTiles[x] = new tile;
    > }
    >
    > void map::~map()
    > {
    > for(x = 0; x < 144; x++)
    > delete m_MyTiles[x];
    > }
    > -------------SNIP---------------------


    The code as you have posted it is fine (assuming that tile is defined
    somewhere before map::map(), and you actually define a variable 'x' in
    map::~map(). Your problem must be elsewhere. Maybe some other code is
    overwriting the m_MyTiles array, or maybe there is a problem in tile's
    destructor, but I can't really tell from what you've posted
    Clark Cox, Feb 24, 2004
    #2
    1. Advertising

  3. "Joerg Toellner" <> wrote in message
    news:c1fpmi$pce$02$-online.com...
    > Hi Group,
    >
    > i stumbled over a delete problem which confuses me. I'm not very

    experienced
    > with C++ programming and so i am not able to understand what's going on
    > here.
    >
    > I have the following situation:
    >
    > -------------SNIP---------------------
    > // File: map.cpp
    >
    > class tile;
    >
    > class map
    > {
    > map();
    > ~map();
    > // ... some other unrelated stuff here
    > tile *m_MyTiles[144];
    > }
    >
    > void map::map()
    > {
    > int x;
    > for(x = 0; x < 144; x++)
    > m_MyTiles[x] = new tile;
    > }
    >
    > void map::~map()
    > {
    > for(x = 0; x < 144; x++)
    > delete m_MyTiles[x];
    > }
    > -------------SNIP---------------------
    >
    > When i construct a new instance of class map and later delete it again my
    > program crash at the delete command in the destructor. It seems that the

    (in
    > the constructor of class map) created 144 instances of class Tile will be
    > deleted somehow automagically.


    That's not true, you new them so you delete them.

    > When i comment out the code in the destructor
    > of class map, all works fine. But i assume that then i will leave some
    > memory leaks behind, right?
    >


    Right.

    > Can s.o. please direct me in the right direction what i'm doing wrong?

    Must
    > i delete my with new created instances of the tiles in the destructor of

    map
    > (where the pointers are member variables)? Or who will do this for me

    behind
    > the scene? Or when this will leave memory leaks, how do i delete my
    > tiles-objects correctly without a program crash?
    >


    What you've forgotten to do is write a copy constructor and assignment
    operator for your class.

    If you have two copies of the same object, then at present you will have two
    copies of the same pointers. When the first object gets destructed
    everything is OK, but when the second gets destructed you are deleteing
    pointers that have been deleted already, so your program crashes.

    > Sorry for the dumb question, but i can't figure it out myself.
    >


    This is *the* classic newbie C++ gotcha. You need a good C++ book, any C++
    book that doesn't explain copy constructors is not worth reading.


    > TIA very much
    > Joerg Toellner
    >


    john
    John Harrison, Feb 24, 2004
    #3
  4. Hi John and Clark,

    thx for your replies.

    I haven't my code right by hand now. So i only wrote the situation out of
    the head. Of course x is defined in the real code destructor.

    I'm glad that my suggestion that i have to delete it, is right.

    To John:
    Can't understand your comments about copy constructor. There will be
    definitely ONE instance/object of type class map. map is a map (or say a
    playfield) for a game that will hold the parts of the playfield (the single
    tiles of the map). So there is only one map consisting of maaaaany tiles.

    Do i need the copy constructor in this case too? And why do i need it when i
    never want to copy the map-object? Even if it is a good fashion or
    recommended by default.

    Of course i'm thankful for your hint. I will remember this for the objects
    where i need more than one instance.

    I'll check my code further to find who is deleting my tiles before i get
    hand on it. Your comments are important for me that it is worth searching
    for the bug further and not hunting C++ ghosts which every experienced
    C++-Guru already knows.

    Thx. again to you both and to John for maybe another little comment about
    the copy constructor.

    CU
    Joerg Toellner

    PS:
    I already own Stroustroup C++ Language and Effective C++ and More effective
    C++ as books. But you will understand, nobody can remember all the details
    and s.t. it is like chasing an elephant with an microscope. You overseas the
    obvious.

    "John Harrison" <> schrieb im Newsbeitrag
    news:c1fqa1$1he3vq$-berlin.de...
    >
    > "Joerg Toellner" <> wrote in message
    > news:c1fpmi$pce$02$-online.com...
    > > Hi Group,
    > >
    > > i stumbled over a delete problem which confuses me. I'm not very

    > experienced
    > > with C++ programming and so i am not able to understand what's going on
    > > here.
    > >
    > > I have the following situation:
    > >
    > > -------------SNIP---------------------
    > > // File: map.cpp
    > >
    > > class tile;
    > >
    > > class map
    > > {
    > > map();
    > > ~map();
    > > // ... some other unrelated stuff here
    > > tile *m_MyTiles[144];
    > > }
    > >
    > > void map::map()
    > > {
    > > int x;
    > > for(x = 0; x < 144; x++)
    > > m_MyTiles[x] = new tile;
    > > }
    > >
    > > void map::~map()
    > > {
    > > for(x = 0; x < 144; x++)
    > > delete m_MyTiles[x];
    > > }
    > > -------------SNIP---------------------
    > >
    > > When i construct a new instance of class map and later delete it again

    my
    > > program crash at the delete command in the destructor. It seems that the

    > (in
    > > the constructor of class map) created 144 instances of class Tile will

    be
    > > deleted somehow automagically.

    >
    > That's not true, you new them so you delete them.
    >
    > > When i comment out the code in the destructor
    > > of class map, all works fine. But i assume that then i will leave some
    > > memory leaks behind, right?
    > >

    >
    > Right.
    >
    > > Can s.o. please direct me in the right direction what i'm doing wrong?

    > Must
    > > i delete my with new created instances of the tiles in the destructor of

    > map
    > > (where the pointers are member variables)? Or who will do this for me

    > behind
    > > the scene? Or when this will leave memory leaks, how do i delete my
    > > tiles-objects correctly without a program crash?
    > >

    >
    > What you've forgotten to do is write a copy constructor and assignment
    > operator for your class.
    >
    > If you have two copies of the same object, then at present you will have

    two
    > copies of the same pointers. When the first object gets destructed
    > everything is OK, but when the second gets destructed you are deleteing
    > pointers that have been deleted already, so your program crashes.
    >
    > > Sorry for the dumb question, but i can't figure it out myself.
    > >

    >
    > This is *the* classic newbie C++ gotcha. You need a good C++ book, any C++
    > book that doesn't explain copy constructors is not worth reading.
    >
    >
    > > TIA very much
    > > Joerg Toellner
    > >

    >
    > john
    >
    >
    >
    Joerg Toellner, Feb 24, 2004
    #4
  5. "Joerg Toellner" <> wrote in message
    news:c1frar$7t1$07$-online.com...
    > Hi John and Clark,
    >
    > thx for your replies.
    >
    > I haven't my code right by hand now. So i only wrote the situation out of
    > the head. Of course x is defined in the real code destructor.
    >
    > I'm glad that my suggestion that i have to delete it, is right.
    >
    > To John:
    > Can't understand your comments about copy constructor. There will be
    > definitely ONE instance/object of type class map. map is a map (or say a
    > playfield) for a game that will hold the parts of the playfield (the

    single
    > tiles of the map). So there is only one map consisting of maaaaany tiles.
    >
    > Do i need the copy constructor in this case too? And why do i need it when

    i
    > never want to copy the map-object? Even if it is a good fashion or
    > recommended by default.


    If you really don't think a map object will ever be copied then do this

    class map
    {
    public:
    ...
    private:
    map(const map&);
    map& operator=(const map&);
    };

    I.e. declare the copy constructor and assignment operator private, this will
    stop the compiler copying them when you don't expect it to.

    You problem does sound exactly like a problem caused by a lack of copy
    constructor, are you sure the map is never copied, for instance to you use
    the map as a parameter to a function, or a return value from a function?
    Either of those could cause the map to be copied.

    john
    John Harrison, Feb 24, 2004
    #5
  6. Joerg Toellner

    Chris Theis Guest

    "Clark Cox" <> wrote in message
    news:clarkcox3-146AB4.10244424022004@localhost...
    > In article <c1fpmi$pce$02$-online.com>,
    > "Joerg Toellner" <> wrote:
    >
    > > Hi Group,
    > >
    > > i stumbled over a delete problem which confuses me. I'm not very

    experienced
    > > with C++ programming and so i am not able to understand what's going on
    > > here.
    > >
    > > I have the following situation:
    > >
    > > -------------SNIP---------------------
    > > // File: map.cpp
    > >
    > > class tile;
    > >
    > > class map
    > > {
    > > map();
    > > ~map();
    > > // ... some other unrelated stuff here
    > > tile *m_MyTiles[144];
    > > }
    > >
    > > void map::map()
    > > {
    > > int x;
    > > for(x = 0; x < 144; x++)
    > > m_MyTiles[x] = new tile;
    > > }
    > >
    > > void map::~map()
    > > {
    > > for(x = 0; x < 144; x++)
    > > delete m_MyTiles[x];
    > > }
    > > -------------SNIP---------------------

    >
    > The code as you have posted it is fine (assuming that tile is defined
    > somewhere before map::map(), and you actually define a variable 'x' in
    > map::~map(). Your problem must be elsewhere. Maybe some other code is
    > overwriting the m_MyTiles array, or maybe there is a problem in tile's
    > destructor, but I can't really tell from what you've posted


    It's rather what the OP did not write than what he's written. The trouble is
    that the delete statement might be called twice on the pointers which will
    result in undefined behavior. This might be caused for example by the
    omittance of the copy constructor. Hence, the compiler synthesizes a binary
    copy and two map objects will contain pointers to the same elements.
    Consequently, the elements will be deleted as soon as one of those two
    objects gets destroyed. As soon as the other one attempts to say good bye
    the program will end up in nirvana as the delete statement is called again
    for the already deleted elements.

    As a guideline one should remember always to take care of the copy ctor (and
    assignment op) if one needs to implement a dtor.

    Cheers
    Chris
    Chris Theis, Feb 24, 2004
    #6
  7. Joerg Toellner wrote:
    >
    > Hi John and Clark,
    >
    > thx for your replies.
    >
    > I haven't my code right by hand now. So i only wrote the situation out of
    > the head. Of course x is defined in the real code destructor.
    >
    > I'm glad that my suggestion that i have to delete it, is right.
    >
    > To John:
    > Can't understand your comments about copy constructor. There will be
    > definitely ONE instance/object of type class map. map is a map (or say a
    > playfield) for a game that will hold the parts of the playfield (the single
    > tiles of the map). So there is only one map consisting of maaaaany tiles.
    >
    > Do i need the copy constructor in this case too? And why do i need it when i
    > never want to copy the map-object?


    You may create a copy by accident and never notice.

    eg.

    int main()
    {
    map tiles;

    ...

    foo( tiles );
    }

    and now your function foo is declared like this

    void foo( map AllTiles )
    {
    ...
    }

    You didn't want this, you wanted to pass per reference, but as you are
    human you made an error and forgot the &.
    So what happens now? When function foo is called, a copy of tiles is made.
    Both, the original tiles and the copy contain identical pointers. When
    the function returns the copy is detroyed, meaning it deletes all the memory
    where the pointer points to. But wait: tiles also has this pointer to the very
    same memory. Boom.

    > Of course i'm thankful for your hint. I will remember this for the objects
    > where i need more than one instance.


    The only thing you should remember is the simple rule:
    Don't allocate dynamically if you don't have to.

    class map
    {
    map();
    ~map();

    ...

    tile m_MyTiles[144];
    }

    Bingo. No allocation in the constructor, no deallocationn in the destrustor
    and creating copies automatically does the right thing: it copies the tiles
    and not only the pointer to the tiles.

    --
    Karl Heinz Buchegger
    Karl Heinz Buchegger, Feb 24, 2004
    #7
  8. Joerg Toellner wrote:

    > Hi John and Clark,
    >
    > thx for your replies.
    >
    > I haven't my code right by hand now. So i only wrote the situation out of
    > the head. Of course x is defined in the real code destructor.
    >
    > I'm glad that my suggestion that i have to delete it, is right.
    >
    > To John:
    > Can't understand your comments about copy constructor. There will be
    > definitely ONE instance/object of type class map. map is a map (or say a
    > playfield) for a game that will hold the parts of the playfield (the single
    > tiles of the map). So there is only one map consisting of maaaaany tiles.
    >
    > Do i need the copy constructor in this case too? And why do i need it when i
    > never want to copy the map-object? Even if it is a good fashion or
    > recommended by default.
    >
    > Of course i'm thankful for your hint. I will remember this for the objects
    > where i need more than one instance.
    >
    > I'll check my code further to find who is deleting my tiles before i get
    > hand on it. Your comments are important for me that it is worth searching
    > for the bug further and not hunting C++ ghosts which every experienced
    > C++-Guru already knows.
    >
    > Thx. again to you both and to John for maybe another little comment about
    > the copy constructor.
    >
    > CU
    > Joerg Toellner
    >
    > PS:
    > I already own Stroustroup C++ Language and Effective C++ and More effective
    > C++ as books. But you will understand, nobody can remember all the details
    > and s.t. it is like chasing an elephant with an microscope. You overseas the
    > obvious.
    >
    > "John Harrison" <> schrieb im Newsbeitrag
    > news:c1fqa1$1he3vq$-berlin.de...
    >
    >>"Joerg Toellner" <> wrote in message
    >>news:c1fpmi$pce$02$-online.com...
    >>
    >>>Hi Group,
    >>>
    >>>i stumbled over a delete problem which confuses me. I'm not very

    >>
    >>experienced
    >>
    >>>with C++ programming and so i am not able to understand what's going on
    >>>here.
    >>>
    >>>I have the following situation:
    >>>
    >>>-------------SNIP---------------------
    >>>// File: map.cpp
    >>>
    >>>class tile;
    >>>
    >>>class map
    >>>{
    >>> map();
    >>> ~map();
    >>> // ... some other unrelated stuff here
    >>> tile *m_MyTiles[144];
    >>>}
    >>>
    >>>void map::map()
    >>>{
    >>> int x;
    >>> for(x = 0; x < 144; x++)
    >>> m_MyTiles[x] = new tile;
    >>>}
    >>>
    >>>void map::~map()
    >>>{
    >>> for(x = 0; x < 144; x++)
    >>> delete m_MyTiles[x];
    >>>}
    >>>-------------SNIP---------------------
    >>>
    >>>When i construct a new instance of class map and later delete it again

    >
    > my
    >
    >>>program crash at the delete command in the destructor. It seems that the

    >>
    >>(in
    >>
    >>>the constructor of class map) created 144 instances of class Tile will

    >
    > be
    >
    >>>deleted somehow automagically.

    >>
    >>That's not true, you new them so you delete them.
    >>
    >>
    >>>When i comment out the code in the destructor
    >>>of class map, all works fine. But i assume that then i will leave some
    >>>memory leaks behind, right?
    >>>

    >>
    >>Right.
    >>
    >>
    >>>Can s.o. please direct me in the right direction what i'm doing wrong?

    >>
    >>Must
    >>
    >>>i delete my with new created instances of the tiles in the destructor of

    >>
    >>map
    >>
    >>>(where the pointers are member variables)? Or who will do this for me

    >>
    >>behind
    >>
    >>>the scene? Or when this will leave memory leaks, how do i delete my
    >>>tiles-objects correctly without a program crash?
    >>>

    >>
    >>What you've forgotten to do is write a copy constructor and assignment
    >>operator for your class.
    >>
    >>If you have two copies of the same object, then at present you will have

    >
    > two
    >
    >>copies of the same pointers. When the first object gets destructed
    >>everything is OK, but when the second gets destructed you are deleteing
    >>pointers that have been deleted already, so your program crashes.
    >>
    >>
    >>>Sorry for the dumb question, but i can't figure it out myself.
    >>>

    >>
    >>This is *the* classic newbie C++ gotcha. You need a good C++ book, any C++
    >>book that doesn't explain copy constructors is not worth reading.
    >>
    >>
    >>
    >>>TIA very much
    >>>Joerg Toellner
    >>>

    >>
    >>john


    If you only want one instance of the board / map,
    I suggest you search the web and newsgroups for
    the "Singleton Design Pattern". The Singleton pattern
    is when you only want one instance of an object.

    --
    Thomas Matthews

    C++ newsgroup welcome message:
    http://www.slack.net/~shiva/welcome.txt
    C++ Faq: http://www.parashift.com/c -faq-lite
    C Faq: http://www.eskimo.com/~scs/c-faq/top.html
    alt.comp.lang.learn.c-c++ faq:
    http://www.raos.demon.uk/acllc-c /faq.html
    Other sites:
    http://www.josuttis.com -- C++ STL Library book
    Thomas Matthews, Feb 24, 2004
    #8
  9. Joerg Toellner

    zoopy Guest

    > void map::map()
    This should be:
    map::map()


    > void map::~map()

    Same here:
    map::~map()


    Regards,
    planetzoom
    zoopy, Feb 25, 2004
    #9
  10. Dear John and Chris,

    OOOps! Of course i use the map object (better to say a pointer to the
    object) as function parameter and i store the pointer to the map-object in
    my application-object which owns a map* GetMap() method to be able to get
    access to the map from within other classes.

    Ok i saw it is neccesary to go for a little reading *WhereAreMyBooks?* about
    copy constructors.

    Thx. again for your patience and kindness. I really appreciate to learn from
    you and i hope i will be welcome later if there are other problems coming
    up.

    For now....happy coding *TurnThePageInMyBook-where to hell is the chapter
    about operators?* :))))))

    CU
    Joerg Toellner
    "John Harrison" <> schrieb im Newsbeitrag
    news:c1fro6$1iga9s$-berlin.de...
    >
    > "Joerg Toellner" <> wrote in message
    > news:c1frar$7t1$07$-online.com...
    > > Hi John and Clark,
    > >
    > > thx for your replies.
    > >
    > > I haven't my code right by hand now. So i only wrote the situation out

    of
    > > the head. Of course x is defined in the real code destructor.
    > >
    > > I'm glad that my suggestion that i have to delete it, is right.
    > >
    > > To John:
    > > Can't understand your comments about copy constructor. There will be
    > > definitely ONE instance/object of type class map. map is a map (or say a
    > > playfield) for a game that will hold the parts of the playfield (the

    > single
    > > tiles of the map). So there is only one map consisting of maaaaany

    tiles.
    > >
    > > Do i need the copy constructor in this case too? And why do i need it

    when
    > i
    > > never want to copy the map-object? Even if it is a good fashion or
    > > recommended by default.

    >
    > If you really don't think a map object will ever be copied then do this
    >
    > class map
    > {
    > public:
    > ...
    > private:
    > map(const map&);
    > map& operator=(const map&);
    > };
    >
    > I.e. declare the copy constructor and assignment operator private, this

    will
    > stop the compiler copying them when you don't expect it to.
    >
    > You problem does sound exactly like a problem caused by a lack of copy
    > constructor, are you sure the map is never copied, for instance to you use
    > the map as a parameter to a function, or a return value from a function?
    > Either of those could cause the map to be copied.
    >
    > john
    >
    >
    Joerg Toellner, Feb 25, 2004
    #10
  11. "Joerg Toellner" <> wrote in message
    news:c1hkvj$rko$06$-online.com...
    > Dear John and Chris,
    >
    > OOOps! Of course i use the map object (better to say a pointer to the
    > object) as function parameter and i store the pointer to the map-object in
    > my application-object which owns a map* GetMap() method to be able to get
    > access to the map from within other classes.
    >
    > Ok i saw it is neccesary to go for a little reading *WhereAreMyBooks?*

    about
    > copy constructors.



    Well if all you are passing is a pointer to the map, then that is not a
    problem (at least as far as copy constructors go).

    There is a simple way to tell however. Do what I said and declare the copy
    constructor and assignment operator private (you don't have to define them,
    just declare them). If you do that and then you get compile or link errors
    that is because somewhere you are copying the map object, if not, you
    aren't.

    john
    John Harrison, Feb 25, 2004
    #11
    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. VB Programmer
    Replies:
    1
    Views:
    660
    VB Programmer
    Sep 22, 2004
  2. Marc
    Replies:
    2
    Views:
    464
  3. Replies:
    1
    Views:
    917
    bjeremy
    Jan 9, 2007
  4. david jensen
    Replies:
    10
    Views:
    409
    Peter Otten
    Apr 16, 2010
  5. Angus Guan
    Replies:
    0
    Views:
    142
    Angus Guan
    Dec 8, 2008
Loading...

Share This Page