CODEWITHSUNDEEP
c++exceptiontry-catchcontainersthrow
Pixelchemist
Pixelchemist

Reputation: 24926

How do I handle thows/exceptions in a container class safely?

I am currently working on a container class that uses allocators to manage the resources.I'll try to give a short version of what I currently do to resize the container. (The real one is not one-dimensional but the scheme is identical since the allocated data is contiguous.)

Everything that is not clear to me is marked [[[ x ]]].

Example code

template<typename T>
class example
// ...

Notes:

Source of resize(size_t):

void resize (size_type const & new_size)
{
  if (new_size == 0U) 
  { // we resize to zero, so we just remove all data
    clear();
  }
  else if (new_size != size())
  { // we don't go to zero and don't remain the same size
    size_type const old_size = size();
    pointer new_mem(nullptr);
    try
    {
      new_mem = _Allocate(new_size);
    }
    catch (std::bad_alloc e)
    {
      // [[[ 1 ]]]
    }
    size_type counter(0);
    for (size_type i=0; i<new_size; ++i)
    {
      try
      {
        if (i<size()) 
         _A.construct(new_mem + i, const_cast<const_reference>(*(_begin+i)));
         // [[[ 2 ]]]
        else 
          _A.construct(new_mem + i);
        ++counter;
      }
      catch (...) // [[[ 3 ]]]
      {
        // [[[ 4 ]]]
      }
    }
    clear();
    _begin = new_mem;
    _end = _begin + new_size;
  }
}

Questions:

[[[ 1 ]]]

Should I call clear() and re-throw here or is the destructor of the current object called anyway, if I don't catch here?

[[[ 2 ]]]

What about casting to an rvalue-reference here using const_cast() or std::move()? Would this break exception safty?

If I move construct, let's say 9 out of 10 elements, and element 10 throws something on move-construction, I'll loose 9 out of 10 objects!?

[[[ 3 ]]]

I read that catch (...) should be avoided. Nevertheless, I don't know whether there is any other possibility. Is there a way to avoid using a universal catch without knowing whether or what the constructor will throw at me?

[[[ 4 ]]]

I believe that the proper steps here are:

Is this correct?

Upvotes: 2

Views: 308

Answers (2)

CB Bailey
CB Bailey

Reputation: 791421

You really want to avoid all the try/catch stuff and use RAII to ensure proper resource cleanup. E.g.:

void resize (size_type const & new_size)
{
    example<T> tmp(_A); // assuming I can construct with an allocator

    // If the allocation throws then the exception can propogate without
    // affecting the original contents of the container.
    tmp._end = tmp._begin = tmp._A.allocate(new_size);


    for (size_type i =  0; i < std::min(size(), new_size); ++i)
    {
        tmp._A.construct(tmp._begin + i, _begin[i]);
        ++tmp._end; // construction successful, increment _end so this
                    // object is destroyed if something throws later
    }
    for (size_type i = size(); i < new_size; ++i)
    {
        tmp._A.construct(tmp._begin + i);
        ++tmp._end; // as above
    }

    // OK, the copy of old objects and construction of new objects succeeded
    // now take ownership of the new memory and give our old contents to the
    // temporary container to be destroyed at the end of the function.

    std::swap(_begin, tmp._begin);
    std::swap(_end, tmp._end);
}

Notes:

  • You say that "clear() calls _A.destroy() for all elements in [_begin,_end) and _A.deallocate(_begin,size())". For simplicity I've assumed that deallocate doesn't really care about the size() parameter which is true of some allocators. If this is important then you probably want example to have a concept of "capacity" and a _capacity or _end_of_storage member. Separating size from capacity will make clean up simpler to write and more robust.

  • You've already written correct clean-up code in your destructor (and/or functions that it calls). By using a temporary container, I can reuse that code and don't have to duplicate it.

  • By using a local object, I can avoid all try/catch blocks and rely on the automatic destruction of local objects to clean up resources.

Upvotes: 1

danielschemmel
danielschemmel

Reputation: 11116

  1. If your memory allocation fails, you will never have constructed any new objects. (Where would they go?) However, it usually makes sense to rethrow, since the only way to continue after a bad_alloc would be to retry.

  2. The safest way to proceed here is to only move construct only if the move constructor is noexcept and copy construct otherwise. If your compiler does not support ::std::is_nothrow_move_constructible<T> you can either require the implementers of your types to only implement move constructors that are at least transaction safe, or copy construct always.

  3. No. The code can throw anything at you - something derived from ::std::exception, a unsigned long long or even an void*. This is exactly what the universal catch is intended for.

  4. Pretty much. Calling clear should be unnecessary though - as far as I can see you are performing a correct rollback, such that your object is in a consistent state.

Other notes:

  • You should always throw by value and catch by reference (you are catching the std::bad_alloc by value).

  • Be aware that the types provided by your allocator (e.g. size_type) may not be quite what you expect them to be, if the allocator type is a template parameter of your class. (Leading to questions such as whether comparing them holds a nothrow guarantee.)

  • You are relying on clear() to be nothrow. Consider the case where you properly created your new memory and constructed all your objects. You now attempt to clear your old data - if this throws, you are leaking new_mem and all objects. If you move constructed them, this will leave you with a memory and object leak and leave your data structure in an unusable state, even if clear() is transaction safe!

Upvotes: 1

Related Questions