Using a delegating constructor to avoid leaks

Question

In a talk the following code has been shown to be unsafe because if the constructor throws, the destructor will not be called and leak resources:

class TwoResources {
    TwoResources(int x, int y)
        : m_a(nullptr), m_b(nullptr) {
        m_a = new A(x); m_b = new B(y);
    }
    ~TwoResources() {
        delete m_b; delete m_a;
    }
    A * m_a; B * m_b;
};

A suggestion solution is use a delegating constructor, like so:

class TwoResources {
    TwoResources() : m_a(nullptr), m_b(nullptr) { }
    TwoResources(int x, int y) : TwoResources() {
        m_a = new A(x); m_b = new B(y);
    }
    ~TwoResources() {
        delete m_b; delete m_a;
    }
    A * m_a; B * m_b;
};

This is safe because of:

C++11 15.2 [except.ctor]/2: "if the non-delegating constructor for an object has completed execution and a delegating constructor for that object exists with an exception, the object's destructor will be invoked."

However in the same slide, it says:

Just because you can take advantage of this rule doesn't mean you should!

If this code is guaranteed to be safe, then what are some potential issues with it?

Answer 1

Just because something is safe doesn't mean doing it is a good idea.

For instance, using that delegating constructor call is crucial to exception safety, but that's far from clear to a casual reader of the code who is not familiar with the intricacies of the language. A month later, someone else looking at your code might think "why are you setting it to null if you are setting it in the constructor body again?" and remove it. Ouch.

Moreover, when you are managing lifetime manually, you'll need to write your own copy/move constructors/assignment operators. Miss one and havoc results. If you use a unique_ptr to manage the lifetime, then the compiler-generated move constructor/assignment operator and destructor will do the right thing, and it will complain if you attempt to copy without implementing a copy constructor yourself.