Is "this" pointer valid during data member destruction?

Question

Let's consider classes below. I am not asking if this is a good practice or a bad one. Naming of the classes are also arbitrary so it doesn't show any pattern or design.

class Proxy;

class ProxyImplementation
{
public:
    Proxy * proxy_;

    ProxyImplementation(Proxy * proxy): proxy_(proxy)
    {
    }

    ~ProxyImplementation()
    {
        // Is proxy_ valid here? or is it undefined behavior? 
        // is proxy_->number_ still valid here?  
    }


};

class Proxy
{
public:
    int number_;
    shared_ptr<ProxyImplementation> proxyImplementation_;
    Proxy()
    {
        number_ = 10;
        proxyImplementation_.reset(new ProxyImplementation(this));
    }

    ~Proxy() {}
};

In main

   Proxy *p = new Proxy();
   delete p;

Proxy passes this to ProxyImplementation in its constructor, when delete is called, Proxy destructor is called, then its member destructors in order of definition.

Is this valid during member destructors? In other words, is it valid to access proxy_ pointer in ~ProxyImplementation?

Answer 1

The standard guarantees that members are destroyed in reverse order of declaration. All members must be destroyed before the object that contains them is destroyed (this is obvious by inspection; you can't destroy the contents of something that no longer exists). So, when it comes time for a Proxy instance to self-destruct, it must:

1. Call the Proxy destructor (does nothing)
2. Perform cleanup for proxyImplementation_
3. Perform cleanup for number_ (nothing to do for simple primitive)
4. Perform final cleanup for Proxy object itself

As such, if proxyImplementation_ was a unique_ptr (and the pointer always pointed to the original Proxy), this would be enough to say that both number_ and the Proxy object itself must still exist; they're not allowed to be destroyed until proxyImplementation_ is fully destroyed.

The problem is that you used a shared_ptr, which implies that somewhere, there may be other shared_ptrs to the same ProxyImplementation object. So when a given Proxy is destroyed, there is no guarantee the ProxyImplementation it holds will get destroyed at that time. If it gets destroyed later, proxy_ won't exist, and terrible things will happen.

In short, this is safe if you change to using a unique_ptr that you never replace after construction and unsafe if you actually share a shared_ptr. If you do use a unique_ptr to make it safe, since you declared it as a public member, I'd recommend making it a properly initialized const unique_ptr, which guarantees neither you, nor any consumer of your class can (without evil const_casts) replace the unique_ptr's owned pointer, so it will always be the original value. Fixed up code would be (comments on changed lines):

class Proxy;

class ProxyImplementation
{
public:
    Proxy *const proxy_; // Explicitly declare constant pointer; 1-1 permanent
                         // relationship between Proxy and Implementation that
                         // consumers of your class can't change

    ProxyImplementation(Proxy *proxy): proxy_(proxy) {}

    ~ProxyImplementation()
    {
        // proxy_ is valid here
        // proxy_->number_ is valid here
    }
};

class Proxy
{
public:
    int number_;
    // Changed from shared_ptr to const unique_ptr: initialize once, can't be
    // reassigned, guaranteed to perform cleanup during destruction (no possibility
    // of shared_ptr copied by class consumer keeping it alive after Proxy is gone)
    const unique_ptr<ProxyImplementation> proxyImplementation_;

    // Use initializers, not assignment in constructor body, so const unique_ptr
    // can be assigned precisely once
    Proxy() : number_(10), proxyImplementation_(new ProxyImplementation(this)) {}
    // Or if you can guarantee C++14 or higher, and want new/delete-free code:
    Proxy() : number_(10),
              proxyImplementation_(std::make_unique<ProxyImplementation>(this)) {}

    // Got rid of explicitly declared empty destructor; let compiler generate it
};

Answer 2

// Is proxy_ valid here? or is it undefined behavior? 

The pointer is "valid" but refers to the storage of an object whose lifetime has ended.

The lifetime of an object o of type T ends when:

...

• if T is a class type, the destructor call starts, or

See https://eel.is/c++draft/basic.life#1.4

after the lifetime of an object has ended and before the storage which the object occupied is reused or released, any pointer that represents the address of the storage location where the object will be or was located may be used but only in limited ways

See https://eel.is/c++draft/basic.life#6

// is proxy_->number_ still valid here?

It maybe shouldn't be, but it is (assuming this was the last outstanding reference to the ProxyImplementation, and so its destructor runs inside the shared_ptr member destructor. See @ShadowRanger's answer for more details on why that's important). The rules presented below allow access to all member subobjects up until the moment the whole containing object finishes destruction.

The program has undefined behavior if:

...

• the pointer is used to access a non-static data member

See https://eel.is/c++draft/basic.life#6.2

However, there is an escape clause written in, namely

For an object under construction or destruction, see [class.cdtor].

And the rules we find there seem to indicate it will be allowed:

For an object with a non-trivial destructor, referring to any non-static member or base class of the object after the destructor finishes execution results in undefined behavior.

See https://eel.is/c++draft/class.cdtor#1

The destructor body is trivial, but the entire destructor, including compiler-generated calls to member destructors, is non-trivial. It's the latter that matters. But the latter also hasn't finished execution, so by this rule we're still ok.

To form a pointer to (or access the value of) a direct non-static member of an object obj, the construction of obj shall have started and its destruction shall not have completed, otherwise the computation of the pointer value (or accessing the member value) results in undefined behavior.

See https://eel.is/c++draft/class.cdtor#3

obj is the Proxy instance, and its destructor is still in progress, so accessing the member value is still ok according to this rule as well.

Answer 3

1. An object of a class type in the middle of destruction is still an object. You can refer to its members that are not destroyed yet. ref
2. Destruction of subobjects happens in the reverse order of their construction. ref
3. Subojects are destroyed after the main object's destructor body finishes. ibid

In your example, *proxy_ is being destroyed. Its destructor body has finished, and destruction of its subobjects is taking place. The one that is currently being destroyed is proxy_->proxyImplementation_. Destruction of proxy_->number_ has not started yet. It is thus OK to touch proxy_->number_ from the destructor body of proxy_->proxyImplementation_.

Answer 4

To answer the questions in the quoted code:

~ProxyImplementation()
{
    // Is proxy_ valid here? or is it undefined behavior? 
    // is proxy_->number_ still valid here?  
}

At the time ~ProxyImplementation() executes, the Proxy object that proxy_ is pointing to is in an "interesting" partially-valid state:

• The ~Proxy() destructor has already finished executing, which means that any resources that were explicitly freed up inside that destructor are no longer available

• The member-variables in the Proxy object that were declared after proxyImplementation_ in the Proxy class-header have already been destroyed. (the member-variables declared before proxyImplementation_, like number_, and proxyImplementation_ itself, are still valid)

• Perhaps needless to say, any virtual-method-calls to the Proxy class are no longer going to call their method-overrides in Proxy's subclasses, since those subclasses' destructors have also already finished executing and the Proxy object's vtable-pointer has been modified to reflect that.

My recommendation is that ~ProxyImplementation() try to avoid dereferencing its proxy_ pointer if at all possible... or if it does dereference it, you'll need to be very careful to only access the still-valid member-variables (and which ones are still-valid could change if/when someone reorders the member-variable declarations in the Proxy class declaration, so watch out!)