Why is my struct destructor not called?

Started by
17 comments, last by King Mir 8 years, 1 month ago

Hmm, the replies seemed to jump straight to advanced concepts, skipping the basic new + delete which is how C++ works at the basic level:


    A *a = new A;
    a->val = 500;

    delete a;
    a = new A();
    printf( "Finally %d\n", a->val );
    a->val = 300;

    delete a;
    return 0;

The recommended practice these days is to use the standard library, but the point to remember is that pointers MUST be used if object destruction is needed before exiting scope.


#include <memory>

struct A { int val; };

int main()
{
    std::unique_ptr a{ new A};
    a->val = 500;

    a.reset(new A);
    printf("Finally %d\n" a->val);
    a->val = 300;
}

I believe manual scoping might work as well in some case but here, it seems you need "a" for the entire scope...


int main()
{
    A a;
    a.val = 500;
 
    {
        A b;
        b.val = 300;
        a.val = b.val;
        printf("Destroying b...\n");
    }
}
Advertisement

Hmm, the replies seemed to jump straight to placement new, skipping the basic new + delete which is how C++ works at the basic level:


No, the first mention of placement new is in Frob's post as #9 when all the relevant bits have been well covered and remained so for days (until Shannon Barber added something which just screamed for correction). Additionally, this is General Programming, not For Beginners, so going more in-depth beyond the scope of the original question is not only allowed but actually desired.

[...] but the point to remember is that pointers MUST be used if object destruction is needed before exiting scope.

Looking at your example, by pointers you mean "allocated on the free store" in which case the answer is an emphatic 'no'.

First, for the example you give you can just remove all need for dynamic allocation (and pointers). If 'a' needs to be destroyed before the scope ends, you can just allocate it on the stack and call the destructor manually and then default-construct it again via placement new. This has the exact same effect as your example except you avoid an unnecessary dynamic allocation. The only reason you need to default-construct it again is because the compiler will destroy it automatically when it leaves scope.

Something more interesting is a delayed construction pattern where you can construct and destroy an object whenever you feel like it but no dynamic memory has to be done. You can do by hand with for example std::aligned_storage or use something pre-made like for example Boost's optional. Something like that can also be used to avoid dynamic allocation while hiding the implementation in a PIMPL-like way.

@Bitmaster...

I said pointers must be used because they are the language facility for managing object... "lifetime," for lack of a better word. Sorry you caught my post before I edited it to reflect my concerns about turning to "advanced concepts" not placement new specifically. The OP asked "Why am I not getting Destruct A [500]?!." The answer, in my opinion should have been something like "Because you need to call "delete a" on a A* or exit the scope which does not automatically happen when you assign." The assignment was addressed early but I reproach the jump to topics about optimization, stack, free-store, dynamic allocation, placement new all to avoid the most basic of C++ concepts: pointers and new+delete.

A user-defined type need not allocate any memory to have a non-trivial destructor but the "basic" way of taking control of an object's lifetime away from the automatic scoping is to use a pointer. Surely it is folly to be suggesting that pointers+new+delete = dynamic allocation = free store = slow = much typing = avoid, avoid, avoid?? The OP wanted to dynamically destroy (ie before end of scope) why are we trying to concoct a static allocation??

The OP clearly hit a gotcha in expecting the destructor to be called within the scope of the function. Pointers are necessary, irrespective of the language facility, allocator, container or custom class used to implement RAII / reduce typing / avoid arrow dereferencing / avoid free-store...etc. C/C++ programmers MUST know that they are responsible for their object's lifetime from the moment they declare a variable. This is the C/C++ way.

Automatic initialization+destruction and support structures exist to allow the programmer to delegate that responsibility not be relieved of it. "Thou shalt assume responsibility for this object's lifetime, on my behalf, from point A (e.g initialization) to point B(e.g end of scope or reset()). Placement new, manual destructor call, and all the other expert "implementations" of handling object lifetime must not change "Good Practice" in using variables/pointers/objects/references especially in main() or any other user-level function.

My example was meant to demonstrate the awareness a programmer should have of object lifetime, with particular respect to end of scope which is why I showed the unique_ptr example as one way of delegating responsibility and avoiding manual new+delete. "...an emphatic no." to the use of free store is a bit extreme. If one prefers to dabble in stack-only allocation then they are free to place "new" there, even for unique_ptr.

It is certainly interesting to explore the advanced concepts related to object allocation but all that "expert" knowledge belongs in "encapsulation." "General Programming" should be a place to exemplify good coding practices, moreso than "For Beginners" IMO. Bjarne Stroustrup and most of the "top brass" regularly bemoan the tendency of experts to turn basic questions into in-depth analyses of advanced language concepts instead of demonstrating the virtues and wonders of simplicity.

"The Pointer" is basic C/C++. It is what clearly defines the languages from Java, C# and others. The years have afforded us facilities to delegate object lifetime management but it remains our greatest advantage that we can say and aspire to guarantee that a particular scope behaves as it appears (as long as we don't go fighting the language with "expert" tricks).

You could explicitly invoke the dtor before you invoke the ctor again.

a.~A();

a = A();

That is not the correct syntax for calling a constructor on a deleted object. For correctness, you need to use placement new:
a.~A();
new(&a) A();

However, the use cases for this are very rare. If you want to change a value to the default constructed value, you would just do assignment, with destruction:
a = A();

@Bitmaster...

I said pointers must be used because they are the language facility for managing object... "lifetime," for lack of a better word. Sorry you caught my post before I edited it to reflect my concerns about turning to "advanced concepts" not placement new specifically. The OP asked "Why am I not getting Destruct A [500]?!." The answer, in my opinion should have been something like "Because you need to call "delete a" on a A* or exit the scope which does not automatically happen when you assign." The assignment was addressed early but I reproach the jump to topics about optimization, stack, free-store, dynamic allocation, placement new all to avoid the most basic of C++ concepts: pointers and new+delete.

A user-defined type need not allocate any memory to have a non-trivial destructor but the "basic" way of taking control of an object's lifetime away from the automatic scoping is to use a pointer. Surely it is folly to be suggesting that pointers+new+delete = dynamic allocation = free store = slow = much typing = avoid, avoid, avoid?? The OP wanted to dynamically destroy (ie before end of scope) why are we trying to concoct a static allocation??

The OP clearly hit a gotcha in expecting the destructor to be called within the scope of the function. Pointers are necessary, irrespective of the language facility, allocator, container or custom class used to implement RAII / reduce typing / avoid arrow dereferencing / avoid free-store...etc. C/C++ programmers MUST know that they are responsible for their object's lifetime from the moment they declare a variable. This is the C/C++ way.

Automatic initialization+destruction and support structures exist to allow the programmer to delegate that responsibility not be relieved of it. "Thou shalt assume responsibility for this object's lifetime, on my behalf, from point A (e.g initialization) to point B(e.g end of scope or reset()). Placement new, manual destructor call, and all the other expert "implementations" of handling object lifetime must not change "Good Practice" in using variables/pointers/objects/references especially in main() or any other user-level function.

My example was meant to demonstrate the awareness a programmer should have of object lifetime, with particular respect to end of scope which is why I showed the unique_ptr example as one way of delegating responsibility and avoiding manual new+delete. "...an emphatic no." to the use of free store is a bit extreme. If one prefers to dabble in stack-only allocation then they are free to place "new" there, even for unique_ptr.

It is certainly interesting to explore the advanced concepts related to object allocation but all that "expert" knowledge belongs in "encapsulation." "General Programming" should be a place to exemplify good coding practices, moreso than "For Beginners" IMO. Bjarne Stroustrup and most of the "top brass" regularly bemoan the tendency of experts to turn basic questions into in-depth analyses of advanced language concepts instead of demonstrating the virtues and wonders of simplicity.

"The Pointer" is basic C/C++. It is what clearly defines the languages from Java, C# and others. The years have afforded us facilities to delegate object lifetime management but it remains our greatest advantage that we can say and aspire to guarantee that a particular scope behaves as it appears (as long as we don't go fighting the language with "expert" tricks).

Using pointers may be basic C++, but so is not using them when you don't need them. In the OP, "use dynamic allocation" is the wrong advice to give. If you want to talk about tools avalible to control when an object is destroyed, the foremost to consider is not dynamic memory, it's using an extra set of braces to contain the variable.

You could explicitly invoke the dtor before you invoke the ctor again.
a.~A();
a = A();

This would be a bad idea in the general case. The assignment operator would assume that the object is in a valid state, but the destructor for most non trivial objects would put it in an invalid state.

Right, but the assignment is of new object (ie. a ctor is being assigned). It is more so a bad idea in the general case to cause memory leaks by stomping over an object without deallocating any memory its constructor allocates. (imho) So if you gotta call its dtor to clean up the object you want to overwrite, then just do it the hell with how awful the design is.. just get the job done if that is what it has come down to. (note: I am against cleanup or reset methods to do the job of a dtor just for the sake of avoiding explicitly calling a dtor)

If there is something to worry about with the assignment itself, then the class should have an operator overload for the assignment operator.

I just don't see the issue.

Edit: Though I should note your example of deleting internal buffers where the assignment operator expects valid buffers to be in place.. this is more of a case as to why you should not perform in place constructions. Or so it would seem to me.
There are many tools available and the assignment operator is just one. It might be a case where you should use a copy constructor, or a move constructor.

Personally, I use assignments for POD structures and rarely if every on a class. It is just bad design if you ask me.

You could explicitly invoke the dtor before you invoke the ctor again.
a.~A();
a = A();

This would be a bad idea in the general case. The assignment operator would assume that the object is in a valid state, but the destructor for most non trivial objects would put it in an invalid state.

Right, but the assignment is of new object (ie. a ctor is being assigned). It is more so a bad idea in the general case to cause memory leaks by stomping over an object without deallocating any memory its constructor allocates. (imho) So if you gotta call its dtor to clean up the object you want to overwrite, then just do it the hell with how awful the design is.. just get the job done if that is what it has come down to. (note: I am against cleanup or reset methods to do the job of a dtor just for the sake of avoiding explicitly calling a dtor)

If there is something to worry about with the assignment itself, then the class should have an operator overload for the assignment operator.

I just don't see the issue.

Edit: Though I should note your example of deleting internal buffers where the assignment operator expects valid buffers to be in place.. this is more of a case as to why you should not perform in place constructions. Or so it would seem to me.
There are many tools available and the assignment operator is just one. It might be a case where you should use a copy constructor, or a move constructor.

Personally, I use assignments for POD structures and rarely if every on a class. It is just bad design if you ask me.

Assignment to a valid object should not cause a memory leak. However, assignment to invalid object can cause problems. For example, if A here stored a pointer to dynamically allocated memory. In that case, calling the destructor would free the pointed to value, but the destructor probably would not set the pointer to null (because it does not need to leave the object in a valid state, and it would be an extra operation to null the pointer). Calling the assignment operator after the destructor would again try to free the same pointer, resulting in a double free. This problem occurs even if A does not have an explicit destructor, but happens to contain another object that manages heap memory like std::string.

Yes, classes that need an explicit assignment operator to have correct assignment semantics should use one. That goes without saying.

I disagree that there's anything bad about assigning to a class. This suggests to me that you probably don't use classes enough. Classes offer a layer of encapsulation of a concept that POD structs don't, so they are often preferable. But being an OOP class does not mean that it cannot make sense to be able to use assignment with it.


Calling the assignment operator after the destructor would again try to free the same pointer, resulting in a double free.

To be clear, you are talking about the temporary's destructor? Or using destruction semantics before completing the assignment? Presumably the latter, yes?

In which case the moral of this story is to merely ensure the operation in question accounts for all moving parts(if they are in use), as to avoid explicitly invoking the destructor.

It isn't that I don't use a lot of classes, it is merely that I don't have a lot of objects that need assignments. I deal mostly in systems, and less with the objects in those systems. From a top down view I don't find many niches to use assignment operator overloads, let alone places I need to use them.

You've made the point a lot clearer to me, and thank you for that.


Calling the assignment operator after the destructor would again try to free the same pointer, resulting in a double free.

To be clear, you are talking about the temporary's destructor? Or using destruction semantics before completing the assignment? Presumably the latter, yes?

In which case the moral of this story is to merely ensure the operation in question accounts for all moving parts(if they are in use), as to avoid explicitly invoking the destructor.

The assignment operator would be the one freeing the pointer.

Part of the moral is that the assignment operator should do it's own cleanup. But calling a destructor actually invokes different semantic guarantees about an objects state than simply calling a cleanup method. Effectively, you're changing the semantic type of the memory at that location so it's not a valid object. In theory a compiler could use this fact for optimization, although I'm not aware of any such optimization actually in use.

This topic is closed to new replies.

Advertisement