🔒 Locked

[C++] delete on pointer doesn't call destructor?!

Started by c4c0d3m0n Oct 24, 2008 at 4:07 PM 15 replies 14.8k views

Original Post

Oct 24, 2008 at 4:07 PM

Hi all I have a std::vector vec in my code. I'm adding stuff to this vector using

vec.push_back( new T )

Once I want to remove the content from the program, I use

delete vec.back();
vec.pop_back();

For some reason, this method is not calling the destructor on whatever I'm deleting... Below is the exact code from the project:

// m_state_stack is the std::vector <R2e::State*>

template <class T>
void
R2e::Engine::
changeState()
{
    // Make sure the new state is derived from R2e::State
    BOOST_STATIC_ASSERT(( boost::is_base_of <R2e::State, T>::value ));

    if ( !m_state_stack.empty() ) {
        delete m_state_stack.back();
        m_state_stack.pop_back();
    }

    m_state_stack.push_back( new T );

    m_state_stack.back()->init( this );
}


template <class T>
void
R2e::Engine::
pushState()
{
    // Make sure the new state is derived from R2e::State
    BOOST_STATIC_ASSERT(( boost::is_base_of <R2e::State, T>::value ));

    if ( !m_state_stack.empty() )
      m_state_stack.back()->pause();

    m_state_stack.push_back( new T );

    m_state_stack.back()->init( this );
}


void
R2e::Engine::
popState()
{
    if ( !m_state_stack.empty() ) {
        delete m_state_stack.back();
        m_state_stack.pop_back();
    }

    if ( !m_state_stack.empty() )
      m_state_stack.back()->resume();
}

How come that the destructor isn't called on objects once I call delete on the back of the vector?

Driv3MeFar

Oct 24, 2008 at 4:13 PM

Quote:
Original post by c4c0d3m0n
For some reason, this method is not calling the destructor on whatever I'm deleting...

How do you know this? Have you set a breakpoint in your object's destructor that isn't getting hit?

emeyex

Oct 24, 2008 at 4:15 PM

I'm assuming your vector is actually declared as:

std::vector<R2e::State*>

and that your R2e::State class doesn't have a virtual destructor, i.e.

class State { virtual ~State( ) { } };

If the base class destructor is not virtual, then deleting a pointer to the base class will not result in the destructor of the derived class being called.

Otherwise, I don't see any explanation for it not being called.

c4c0d3m0n

Oct 24, 2008 at 4:16 PM

Not using an IDE, so no breakpoints for me.

I figured it out by having my destructors log something, and this something never appears in the logs.
I've also tried by just throwing in a assert(false); in the destructor, but the program continues joyfully.

edit

thanks emeyex! That was indeed the problem, I hadn't set a virtual destructor in the base class. I'm new to this inheritance thing. Thanks again!

Sc4Freak

Oct 24, 2008 at 4:44 PM

Quote:
Original post by c4c0d3m0n
Not using an IDE, so no breakpoints for me.

You should consider using one, if at all possible. Visual Studio 2008 Express Edition is a free download from Microsoft, and it's one of the best IDE's available.

c4c0d3m0n

Oct 24, 2008 at 5:47 PM

It may be one of the best IDE's available, but it's a sad thing they didn't make it for one of the best operating systems available ;) I'm a Linux user, so no VS for me I'm afraid. I'm perfectly comfortable using an editor and a terminal (plus some hardcoded makefiles) for my little projects.

rip-off

Oct 24, 2008 at 5:51 PM

I'm sure GDB is capable of breakpoints.

SiCrane

Oct 24, 2008 at 5:52 PM

Even if you don't want to use gdb directly (which is totally understandable), you can always use a wrapper like Code::Blocks or DDD.

implicit

Oct 24, 2008 at 5:59 PM

Quote:
Original post by c4c0d3m0n
It may be one of the best IDE's available, but it's a sad thing they didn't make it for one of the best operating systems available ;) I'm a Linux user, so no VS for me I'm afraid. I'm perfectly comfortable using an editor and a terminal (plus some hardcoded makefiles) for my little projects.

Then at least pick up some debugger. All of the GDB interfaces I've ever tried suck badly compared to Visual Studio's integrated debugger but they're still infinitely better than using nothing at all.

c4c0d3m0n

Oct 25, 2008 at 4:48 AM

I must admit that I've been wanting to learn how to use a debugger for a while now. I've never really looked into it, I've always just programmed along happily. Is there any chance that there is any good resources of how to use a debugger? I'm totally in the dark here, since I don't even know exactly what a debugger does, let alone how to use one.

The reason I don't use an IDE is because Code::Blocks or Anjuta haven't really offered me anything that gedit didn't also have (syntax highlighting for example). If Anjuta proves to have a good interface for GDB, I may actually think of converting my project to it.

DevFred

Oct 25, 2008 at 6:11 AM

Quote:
Original post by c4c0d3m0n
Universität Karlsruhe - Informatik

You should check if your University participates in the MSDNAA (Acadamic Alliance). Then you can get the full version of Visual Studio 2008 for free.

InsaneBoarder234

Oct 25, 2008 at 6:24 AM

Quote:
Original post by DevFred
Quote:
Original post by c4c0d3m0n
Universität Karlsruhe - Informatik

You should check if your University participates in the MSDNAA (Acadamic Alliance). Then you can get the full version of Visual Studio 2008 for free.

Why, if he doesn't even use Windows?

streamer

Oct 25, 2008 at 6:49 AM

And what about:
std::vector::iterator it = m_state_stack.end();
delete *it;

rip-off

Oct 25, 2008 at 6:57 AM

Quote:
Original post by streamer
And what about:
std::vector::iterator it = m_state_stack.end();
delete *it;

Undefined behaviour.

std::vector::end() returns a non-dereferenceable (but still comparable) iterator. std::vector::back() is correct.

streamer

Oct 25, 2008 at 7:12 AM

Quote:
Original post by rip-off
Quote:
Original post by streamer
And what about:
std::vector::iterator it = m_state_stack.end();
delete *it;

Undefined behaviour.

std::vector::end() returns a non-dereferenceable (but still comparable) iterator. std::vector::back() is correct.

Then if his code is correct, there must be a problem with desctructor.

Anyway the code can be defined as:

if ( !m_state_stack.empty() ) {       m_state_stack.back()->init( this );    }else{    m_state_stack.push_back( new T );    m_state_stack.back()->init( this );}or simplierif ( m_state_stack.empty() ) {      m_state_stack.push_back( new T );    }    m_state_stack.back()->init( this );

because you are deleting the last element, then pushing on the end
a new element.

You could just skip deleting the last element, and creating the new one, because there is already element on the end, and just initialize it with new values.

Maybe it is not possible, because I don't know what are you doing in Init function, but code looks cleaner.

Just some thoughts.

MaulingMonkey

Oct 25, 2008 at 11:27 AM

Quote:
Original post by streamer
Quote:
Original post by rip-off
Quote:
Original post by streamer
And what about:
std::vector::iterator it = m_state_stack.end();
delete *it;

Undefined behaviour.

std::vector::end() returns a non-dereferenceable (but still comparable) iterator. std::vector::back() is correct.

Then if his code is correct, there must be a problem with desctructor.

Indeed, there was:

Quote:
Original post by c4c0d3m0n
thanks emeyex! That was indeed the problem, I hadn't set a virtual destructor in the base class. I'm new to this inheritance thing. Thanks again!

Quote:
Anyway the code can be defined as:
*** Source Snippet Removed ***

because you are deleting the last element, then pushing on the end
a new element.

You could just skip deleting the last element, and creating the new one, because there is already element on the end, and just initialize it with new values.

Maybe it is not possible, because I don't know what are you doing in Init function, but code looks cleaner.

It's not possible, not because of whatever he's doing in Init, but because he's using polymorphism. Notice how both times he uses new, he isn't doing, "push_back( new R2e::State(...) );", he's doing "push_back( new T(...) );", where T is a template parameter. That type is going to change around.

streamer

Oct 25, 2008 at 4:59 PM

Shame on me. Didn't thought about that [sad]

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.