Is calling destructor from a catch block in constructor safe?

Question

In my constructor, I have to destroy any remaining resources if any code in it throws. I'd like to avoid writing duplicate code so I just call the destructor in the catch block which than frees any resource that has been created. Is this safe?

I'm aware that the destructor is not called if the constructor throws, so I tried compiling some code in msvc and nothing seems wrong, but I'm not sure if this is just luck.

Object::Object(){
    try{
        // Initialize multiple resources here.
    }catch(...){
        this->~Object(); // Is this safe?
        throw;
    }
}

Object::~Object(){
    // release multiple resources, if initialized.
}

I suspect this is UB, but am too lazy to do the research. It would be safer to have a `cleanup()` function and call it from both constructor and destructor. Better still, hold "multiple resources" in RAII wrappers, so that they are released automatically. — Igor Tandetnik, Feb 18 '19 at 23:23
*I have to destroy any remaining resources if any code in it throws* -- RAII is what you should be using, not a dubious call of the destructor explicitly. That type of call of the destructor is almost always reserved for `placement-new` cleanup. — PaulMcKenzie, Feb 18 '19 at 23:24
It's UB due to calling non-trivial destructor on an object whose lifetime has not begun — M.M, Feb 18 '19 at 23:25
In other words: `{ YourResource r1; YourOtherResource r2; }` If `r1` and `r2` can't automatically clean themselves up after the closing curly brace, then reconsider how you've written your classes (or as already mentioned, wrap those types into a class where the code I have shown works correctly). — PaulMcKenzie, Feb 18 '19 at 23:28

benrg · Accepted Answer · 2019-02-19T00:33:14.980

Despite the fact that destructors look like ordinary methods, and the explicit destruction syntax looks like a call of that method, it does not actually just call that method. Among other implementation-specific things, it also calls the destructors of base classes and data members. Throwing an exception from the constructor also causes all of those destructors to be called. So, ~Object() followed by throw will call them twice, likely with disastrous consequences.

Just move the cleanup code to an ordinary method, as someone suggested in a comment.

There's a similar syntactic problem with the function-call syntax for constructing a temporary, and with new/delete and operator new/operator delete. None of them just call the functions with the same names, even though it looks like they should.

score 1 · Answer 2 · answered Feb 18 '19 at 23:34

Firstly, invoking a member function here is fine:

Member functions, including virtual functions ([class.virtual]), can be called during construction or destruction ([class.base.init]).

(Your constructor has begun execution.)

But then there's this about destructors specifically:

Once a destructor is invoked for an object, the object no longer exists; the behavior is undefined if the destructor is invoked for an object whose lifetime has ended ([basic.life]). [ Example: If the destructor for an automatic object is explicitly invoked, and the block is subsequently left in a manner that would ordinarily invoke implicit destruction of the object, the behavior is undefined. — end example ]

So, although we do know that your destructor wouldn't be implicitly invoked "again", the question is whether the subsequent rethrowing results in a scenario where the object is "again" destructed in the sense described by this passage.

I actually gave up on the standardese at this point and I wonder whether this is slightly underspecified. My point is that this in itself is probably enough reason to avoid this well-intentioned pattern, and just put your cleanup in a nice private member function instead, to be shared between your catch block and your destructor.

Is calling destructor from a catch block in constructor safe?

2 Answers2