Destruction of a non-existing object

Question

I am struggling a little bit with move semantics. I read a lot about that topic, however, there are two concrete problems to which I did not found any answers and, thus, like to present them to you.

First, I have the following exemplary code:

#include <iostream>
#include <cstddef>

class A
{
    public:
        A *ptr;
        virtual ~A() 
        { 
            std::cout << "dtor" << std::endl; 
            delete ptr; 
            ptr=nullptr;
        }
};

main()
{
    A x, y;
    x.ptr = &y;
}
// compile and link like this with g++: g++ -std=c++0x -lstdc++ code.cc

Class A is able to have a member of itself. I use this "composite pattern" concept to build a hierarchy. However, in this case, when object x is destructed, its destructor deletes the pointer to object y. When finally object y should be destructed, it is already which yields an error... How can I solve this?

The second case looks like this:

main()
{
    A x, y;
    y = std::move(x);
    std::cout << "x.ptr: " << x.ptr << std::endl;
    std::cout << "y.ptr: " << y.ptr << std::endl;
}

Here, I can see that both references are equal which means both exists. I though that std::move moves the content from x to y. I would have expected that x.ptr is "empty"....?

Thanks in advance!

Cheers

When tested I got both x.ptr and y.ptr as null values. Also I did not get any errors for the second one. — Jake Freeman, Nov 26 '17 at 18:39
addressing the second case: `std::move` doesn't do much itself, the *moving* of the data is done by a move constructor, which the default move constructor does a member-wise move, but for primitives and such (like a pointer) is just a copy. Therefore, it will not set `x.ptr` to "empty" — kmdreko, Nov 26 '17 at 18:41
Doing `ptr=nullptr;` at the end of your destructor is *pointless*. — Jesper Juhl, Nov 26 '17 at 18:41
For what `std::move` does, please see http://en.cppreference.com/w/cpp/utility/move — Jesper Juhl, Nov 26 '17 at 18:43
addressing the first case: this is a reason why raw pointers, `A*`, are a bad way to denote ownership. `unique_ptr` and `shared_ptr` are preferred when "owning" another object because they are both clear in semantics and have well-defined ways of cleaning up after themselves. — kmdreko, Nov 26 '17 at 18:45
If you don't want the `delete` in the destructor to blow up, then `A *ptr;` should probably be `A *ptr = nullptr;` but I think that's still not what you really want - you probably want to actually create an object somewhere. — Jesper Juhl, Nov 26 '17 at 18:47

score 2 · Answer 1 · answered Nov 26 '17 at 18:50

In the first case, both of your x and y objects are created on stack so they will be destructed automatically. However, your class A's destructor uses delete which assumes that the object referred by ptr was created on heap with new. So it's an error to set x.ptr to refer an object created on stack (like y). A correct code would be

x.ptr = new A;

This is the danger of raw pointers - you can't differentiate between pointers to dynamically allocated objects (that you have to eventually delete) and "just pointers" (that you can't).

It's better if you use std::unique_ptr<A> instead of raw pointer A*. It will also call delete my itself, so your delete ptr won't be needed.

As for the second case, for primitive types move is actually just a copy. If you move from int to int, or from ptr to ptr, it's just a copy as there is nothing to optimize with move for primitive types, there is no special "emptying" of the moved-from object. But for your case there are good things called smart pointers that actually do "emptying" (because it guarantees correctness). You guessed it - I'm talking about std::unique_ptr.

So again, if you change your ptr from A* to std::unique_ptr<A> then move will do the job you're looking for.

In two words, std::unique_ptr is your real friend here.

score 0 · Answer 2 · answered Nov 26 '17 at 18:54

In the first, you create two objects of type A on the stack

A x,y;

then assign the address of y to the member x.ptr. When the objects go out of scope, their destructors are called. This happens in the reverse order of their construction, so it should be

y.~A()
x.~A()

In your case, you manually call delete in the destructor, and this will call delete on an already deleted object when execution x.~A().

This not the only problem in your code:

You call delete on an object on the stack. Calls to deletes should be matched by calls to new. Even better, you should manually call new or delete and use std::unique_ptr and std::shared_ptr`.
When b's destructor is called it will delete ptr which uses an uninitialized value for ptr. The compiler-generated constructor does not initialize the ptr tp nullptr. I know at least one compiler which does this in debug mode but in release mode.

When you use std::move, it will finally call the move constructor (or move assignmnet operator). Since you did not define one manually but you define a destructor, there will be no compiler-generated move constructor. So in your case, it will eventually call the implicit copy-constructor and copy the member values, which is what you are seeing.

Even if you add

class A
{
    public:
        A(A&& a) = default;
        A& operator=(A&& a) = default;

        A *ptr;
        virtual ~A() 
        { 
            std::cout << "dtor" << std::endl; 
            delete ptr; 
            ptr=nullptr;
        }
};

to request compiler-generated move semantics, it will generate code that does a member-wise move, looking like

A(A&& a): ptr( std::move(a.ptr) ) {}

For a pointer type, this just assigns the value to the new variable and leaves the old variable as is. The standard's requirements on a moved object are quite basic and fulfilled by this.

Destruction of a non-existing object

2 Answers2