Not exception-safe default move constructors (demonstrated on std::pair)

Clang Frontend
#1

Hi!

It turns out that default move constructors are not exception-safe -
e.g. when an object is constructed with move constructor, it may happen
that some of its members are constructed with move constuctors, then
others with copy constructors. In case of such a copy constructor
throwing an exception, already moved members will be freed, and original
object will be left with a set of empty memebers.

The problem is described here:
http://cpp-next.com/archive/2009/10/exceptionally-moving/

and may be demonstrated on std::pair:

- You have two classes:
  A, which has a proper move constructor which doesn't throw
  B, which which only has copy constructor which may throw
- You have an std::pair<A, B>
- You move a pair
  std::pair<A, B> pair1;
  try {
    std::pair<A, B> pair2(std::move(a));
  }

What happens:
- A part is constructed with move ctor. This is obviously destructive
  to pair1.
- B part is constructed with copy ctor, as it doesn't have move constuctor.
- Now, that copy constructor throws (which is pretty expectable is it
likely allocates resources). After this, as A part was already moved,
we're left with corrupted pair1 (with empty A part).

The same problem applies to most containers, but, for example, in
std::vector it is solved by checking whether a template type has
non-throwing move constructor.

However, with pair this does not apply:

--- utility header from libc++
struct pair {
    ...
    pair(pair&&) = default;

#2

Hi!

It turns out that default move constructors are not exception-safe -
e.g. when an object is constructed with move constructor, it may happen
that some of its members are constructed with move constuctors, then
others with copy constructors. In case of such a copy constructor
throwing an exception, already moved members will be freed, and original
object will be left with a set of empty memebers.

The problem is described here:
http://cpp-next.com/archive/2009/10/exceptionally-moving/

and may be demonstrated on std::pair:

- You have two classes:
A, which has a proper move constructor which doesn't throw
B, which which only has copy constructor which may throw
- You have an std::pair<A, B>
- You move a pair
std::pair<A, B> pair1;
try {
   std::pair<A, B> pair2(std::move(a));
}

What happens:
- A part is constructed with move ctor. This is obviously destructive
to pair1.
- B part is constructed with copy ctor, as it doesn't have move constuctor.
- Now, that copy constructor throws (which is pretty expectable is it
likely allocates resources). After this, as A part was already moved,
we're left with corrupted pair1 (with empty A part).

The same problem applies to most containers, but, for example, in
std::vector it is solved by checking whether a template type has
non-throwing move constructor.

However, with pair this does not apply:

--- utility header from libc++
struct pair {
   ...
   pair(pair&&) = default;
---

Either the default constructor generated by clang is unsafe, or there
should be custom, safe move constuctor with corresponding enable_if.

I lean towards the former, as safe default move constuctors should be
always generated probably, and specific ones may be really hard to
implement for more complex types (tuples).

I've written a small program which demonstrates the problem:

https://gist.github.com/AMDmi3/5005557

tested it on FreeBSD current with clang-3.2 and libc++.

The same problem also exists with gcc/libstdc++.

You have rediscovered the motivation behind:

template <class T>
constexpr
typename conditional
<
   !is_nothrow_move_constructible<T>::value &&
   is_copy_constructible<T>::value,
     const T&,
     T&&

::type

move_if_noexcept(T& x) noexcept;

If you are in a situation where you need the strong exception guarantee, but prefer move over copy if you can get away it, use std::move_if_noexcept.

If move is noexcept, you'll get a move, else you'll get a copy, unless T is not copy constructible, in which case you'll got back to a move.

Howard

#3

It turns out that default move constructors are not exception-safe -
e.g. when an object is constructed with move constructor, it may happen
that some of its members are constructed with move constuctors, then
others with copy constructors. In case of such a copy constructor
throwing an exception, already moved members will be freed, and original
object will be left with a set of empty memebers.

The problem is described here:
http://cpp-next.com/archive/2009/10/exceptionally-moving/

and may be demonstrated on std::pair:

- You have two classes:
A, which has a proper move constructor which doesn't throw
B, which which only has copy constructor which may throw
- You have an std::pair<A, B>
- You move a pair
std::pair<A, B> pair1;
try {
   std::pair<A, B> pair2(std::move(a));
}

What happens:
- A part is constructed with move ctor. This is obviously destructive
to pair1.
- B part is constructed with copy ctor, as it doesn't have move constuctor.
- Now, that copy constructor throws (which is pretty expectable is it
likely allocates resources). After this, as A part was already moved,
we're left with corrupted pair1 (with empty A part).

In addition to Howard's response, I just want to point out that pair1 isn't
really "corrupted"; operations on it will still work correctly, including its
eventual destruction. It's just been partially "emptied", so the operations
won't reflect the original value before the partial move.

Your complaint is really that move construction isn't *transactional*:
it does not promise to either fully succeed or else have no effect other
than to fail. This would be a very strong guarantee for the language to
make; it would involve a huge amount of overhead (move-assigning
moved components back into their source) and is probably not possible
in general.

Either the default constructor generated by clang is unsafe, or there
should be custom, safe move constuctor with corresponding enable_if.

Well, clang and libc++'s behavior here is not up for debate; they're both
directly dictated by the standard, which is why gcc and libstdc++ agree.
If you don't agree with this, the appropriate action is to take it to the
C++ committee.

John.