Question C++ weird destructor behavior.

[Carlis]

Hi
I have encountered a strange behavior of my c++ code when calling a destructor. Basically, I do the following:

I have a pointer to an object v of type diagVertex.
Then I delete it:
delete v; v=NULL;
This should call the destructor of diagVertex:

diagVertex::~diagVertex()
{
____diagTable->popVertex(this);
}

The method popVertex removes the object from a data base called diagTable. Now, what is weird is that it is only removed sometimes. On some occasions, it remains in the database non the less. And, using breakpoints, I notice that the function popVertex is not always called upon deleting v, but only sometimes. However, if I change my code and call the pop function before deleting, then it works like a charm, i.e.:

v->diagTable->popVertex(v);
delete v; v=NULL;

and

diagVertex::~diagVertex()
{

}

--------
This is puzzling to me to say the least. Is there something I have gotten wrong about the destructor? Does anyone have an idea about what I am doing wrong?

Best
//
Johan

 

[Hail The Brain Slug]

You're gonna need to post some actual code snippets to help boil it down.

 

[BFG10K]

How is v initialized? In a single place, or at multiple points in the program? I'm guessing you're leaking memory by sometimes re-assigning v before deleting it. The destructor will never be called in those cases.

But you shouldn't be using manual new/delete in C++ anyway unless you have a very good reason. Use std::unique_ptr or better yet, use a stack object instance.

 

[Carlis]

Thanks for the input. My thought was also that it would be a memory leak.

After some reading, and thinking, I realized what the problem is:
diagVertex is an inherited class of type diagElement. In some instances, the pointer to v is of type diagElement. When calling the destructor, the call is made to the destructor of diagElement it seems, https://www.quantstart.com/articles/C-Virtual-Destructors-How-to-Avoid-Memory-Leaks/ .
So this seems to explain the source of the error... You learn something everyday I guess. I'm a physicist and completely self-taught when it comes to programming, but with c++ this is dangerous sometimes

On a side note, you mention new/delete is bad form in general. I do this all the time . Why do you think so?

 

[BFG10K]

On a side note, you mention new/delete is bad form in general. I do this all the time . Why do you think so?

It's old style C++ which is extremely error-prone. A single mismatched new/delete anywhere in the program is a serious problem that can be almost impossible to find, and is extremely easy to happen without realizing.

Stack objects and STL containers are far safer because the memory management is encapsulated away from you. If you have to use pointers, either make them non-owning so they're never responsible for the memory, or use std::unique_ptr if you need ownership.

As an aside, I also avoid inheritance as well. It quickly becomes a nightmare to track what's actually going on with all that internal coupling. Object composition is far simpler because each class remains fully shielded from the others. Or better yet, use free functions if they're obviously the simpler choice. So many people rush to create multi-inheritance templated monstrosities for no good reason.

 

[Hail The Brain Slug]

Thanks for the input. My thought was also that it would be a memory leak.

After some reading, and thinking, I realized what the problem is:
diagVertex is an inherited class of type diagElement. In some instances, the pointer to v is of type diagElement. When calling the destructor, the call is made to the destructor of diagElement it seems, https://www.quantstart.com/articles/C-Virtual-Destructors-How-to-Avoid-Memory-Leaks/ .
So this seems to explain the source of the error... You learn something everyday I guess. I'm a physicist and completely self-taught when it comes to programming, but with c++ this is dangerous sometimes

I had a feeling it had something to do with inheritance but you didn't give enough information and I didn't want to speculate.

Code snippets are worth a million words.

 

[mv2devnull]

On a side note, you mention new/delete is bad form in general. I do this all the time . Why do you think so?

Memory is a resource and releasing it properly on all possible execution paths (some of which may be exceptionally unforeseeable) is tedious. RAII helper handles, such as std::unique_ptr, save your time.

Resource Acquisition Is Initialization or RAII:

RAII - cppreference.com

en.cppreference.com

Stroustrup: C++ Style and Technique FAQ

www.stroustrup.com

Stroustrup: C++ Style and Technique FAQ

www.stroustrup.com