36

I've got a non-trivial type that owns multiple resources. How do I construct it in an exception safe manner?

For example, here is a demo class X that holds an array of A:

#include "A.h"

class X
{
    unsigned size_ = 0;
    A* data_ = nullptr;

public:
    ~X()
    {
        for (auto p = data_; p < data_ + size_; ++p)
            p->~A();
        ::operator delete(data_);
    }

    X() = default;
    // ...
};

Now the obvious answer for this particular class is to use std::vector<A>. And that's good advice. But X is only a stand-in for more complicated scenarios where X must own multiple resources and it isn't convenient to use the good advice of "use the std::lib." I've chosen to communicate the question with this data structure simply because it is familiar.

To be crystal clear: If you can design your X such that a defaulted ~X() properly cleans everything up ("the rule of zero"), or if ~X() only has to release a single resource, then that is best. However, there are times in real life when ~X() has to deal with multiple resources, and this question addresses those circumstances.

So this type already has a good destructor, and a good default constructor. My question centers on a non-trivial constructor that takes two A's, allocates space for them, and constructs them:

X::X(const A& x, const A& y)
    : size_{2}
    , data_{static_cast<A*>(::operator new (size_*sizeof(A)))}
{
    ::new(data_) A{x};
    ::new(data_ + 1) A{y};
}

I have a fully instrumented test class A and if no exceptions are thrown from this constructor it works perfectly well. For example with this test driver:

int
main()
{
    A a1{1}, a2{2};
    try
    {
        std::cout << "Begin\n";
        X x{a1, a2};
        std::cout << "End\n";
    }
    catch (...)
    {
        std::cout << "Exceptional End\n";
    }
}

The output is:

A(int state): 1
A(int state): 2
Begin
A(A const& a): 1
A(A const& a): 2
End
~A(1)
~A(2)
~A(2)
~A(1)

I have 4 constructions, and 4 destructions, and each destruction has a matching constructor. All is well.

However if the copy constructor of A{2} throws an exception, I get this output:

A(int state): 1
A(int state): 2
Begin
A(A const& a): 1
Exceptional End
~A(2)
~A(1)

Now I have 3 constructions but only 2 destructions. The A resulting from A(A const& a): 1 has been leaked!

One way to solve this problem is to lace the constructor with try/catch. However this approach is not scalable. After every single resource allocation, I need yet another nested try/catch to test the next resource allocation and deallocate what has already been allocated. Holds nose:

X(const A& x, const A& y)
    : size_{2}
    , data_{static_cast<A*>(::operator new (size_*sizeof(A)))}
{
    try
    {
        ::new(data_) A{x};
        try
        {
            ::new(data_ + 1) A{y};
        }
        catch (...)
        {
            data_->~A();
            throw;
        }
    }
    catch (...)
    {
        ::operator delete(data_);
        throw;
    }
}

This correctly outputs:

A(int state): 1
A(int state): 2
Begin
A(A const& a): 1
~A(1)
Exceptional End
~A(2)
~A(1)

But this is ugly! What if there are 4 resources? Or 400?! What if the number of resources is not known at compile time?!

Is there a better way?

TemplateRex
  • 69,038
  • 19
  • 164
  • 304
Howard Hinnant
  • 206,506
  • 52
  • 449
  • 577
  • Your code isn't correct. You never have to explicitly call a destructor. You should allocate `data_` via `new []`, and destroy it via `delete[]`. That will call the destructors for you. – user207421 Aug 05 '16 at 03:34
  • 5
    @EJP: You miss the entire point of this post. With this exact data structure, even `new/delete` is ridiculous. This example is a simplistic example of more complex types that need to acquire multiple resources in a constructor. If you need to do *exactly* what I show above, don't use this code, or `new/delete`. **Use vector.** – Howard Hinnant Aug 05 '16 at 03:39
  • 8
    My general guideline is, don't have objects with multiple responsibilities. Even dynamically-allocated memory is a responsibility, which can be delegated to `std::unique_ptr`. An object should either have a single responsibility, or form a single abstraction over a "product" of simple objects. In your case, you can easily create internal classes inside of `X` which have their own destructors to handle their unique responsibility. – KABoissonneault Aug 05 '16 at 03:47
  • 1
    @KABoissonneault: That's a good guideline, and one I use regularly. But life can get complicated. This post is about an under-appreciated technique for dealing with that complication. – Howard Hinnant Aug 05 '16 at 03:49
  • 2
    Use one class to manage each resource, and then give your class multiple members of such class types. – M.M Aug 05 '16 at 07:08
  • 1
    @HowardHinnant Even if isolating the resource-management responsibility from class X is non-trivial, it is still the main problem, and **it** may well be worth a Stack Overflow post. Covering it up with more bad design (e.g. declaring a default constructor that may not have any valid meaning and just serves the purpose of ensuring the destructor gets called) is not a solution. – Victor Savu Aug 05 '16 at 08:31

3 Answers3

41

Is there a better way?

YES

C++11 delivers a new feature called delegating constructors which deals with this situation very gracefully. But it is a bit subtle.

The problem with throwing exceptions in constructors is to realize that the destructor of the object you're constructing doesn't run until the constructor is complete. Though the destructors of sub objects (bases and members) will run if an exception is thrown, as soon as those sub objects are fully constructed.

The key here is to fully construct X before you start adding resources to it, and then add resources one at a time, keeping the X in a valid state as you add each resource. Once the X is fully constructed, ~X() will clean up any mess as you add resources. Prior to C++11 this might look like:

X x;  // no resources
x.push_back(A(1));  // add a resource
x.push_back(A(2));  // add a resource
// ...

But in C++11 you can write the multi-resource-acquizition constructor like this:

X(const A& x, const A& y)
    : X{}
{
    data_ = static_cast<A*>(::operator new (2*sizeof(A)));
    ::new(data_) A{x};
    ++size_;
    ::new(data_ + 1) A{y};
    ++size_;
}

This is pretty much like writing code completely ignorant of exception safety. The difference is this line:

    : X{}

This says: Construct me a default X. After this construction, *this is fully constructed and if an exception is thrown in subsequent operations, ~X() gets run. This is revolutionary!

Note that in this case, a default-constructed X acquires no resources. Indeed, it is even implicitly noexcept. So that part won't throw. And it sets *this to a valid X that holds an array of size 0. ~X() knows how to deal with that state.

Now add the resource of the uninitialized memory. If that throws, you still have a default constructed X and ~X() correctly deals with that by doing nothing.

Now add the second resource: A constructed copy of x. If that throws, ~X() will still deallocate the data_ buffer, but without running any ~A().

If the second resource succeeds, set the X to a valid state by incrementing size_ which is a noexcept operation. If anything after this throws, ~X() will correctly clean up a buffer of length 1.

Now try the third resource: A constructed copy of y. If that construction throws, ~X() will correctly clean up your buffer of length 1. If it doesn't throw, inform *this that it now owns a buffer of length 2.

Use of this technique does not require X to be default constructible. For example the default constructor could be private. Or you could use some other private constructor that puts X into a resourceless state:

: X{moved_from_tag{}}

In C++11, it is generally a good idea if your X can have a resourceless state as this enables you to have a noexcept move constructor which comes bundled with all kinds of goodness (and is the subject of a different post).

C++11 delegating constructors is a very good (scalable) technique for writing exception safe constructors as long as you have a resource-less state to construct to in the beginning (e.g. a noexcept default constructor).

Yes, there are ways of doing this in C++98/03, but they aren't as pretty. You have to create an implementation-detail base class of X that contains the destruction logic of X, but not the construction logic. Been there, done that, I love delegating constructors.

Howard Hinnant
  • 206,506
  • 52
  • 449
  • 577
7

I think the problem stems from a violation of the Single Responsibility Principle: Class X has to deal with managing the lifetimes of multiple objects (and that is probably not even its main responsibility).

The destructor of a class should only free the resources that the class has directly acquired. If the class is just a composite (i.e. an instance of the class owns instances of other classes) it should ideally rely on automatic memory management (via RAII) and just use the default destructor. If the class has to manage some specialized resources manually (e.g. opens a file descriptor or a connection, acquires a lock or allocates memory) I would recommend factoring out the responsibility of managing those resources to a class dedicated for this purpose and then using instances of that class as members.

Using the standard template library would in fact help because it contains data structures (such as smart pointers and std::vector<T>) that exclusively handle this problem. They can also be composed, so even if your X has to contain multiple instances of objects with complicated resource acquisition strategies, the problem of resource management in an exception safe manner is solved both for each member as well as for the containing composite class X.

Victor Savu
  • 936
  • 14
  • 19
2

In C++11, maybe try something like this:

#include "A.h"
#include <vector>

class X
{
    std::vector<A> data_;

public:
    X() = default;

    X(const A& x, const A& y)
        : data_{x, y}
    {
    }

    // ...
};
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770