Simplifying code and achieving exception safety using unique_ptr

Recently, I had a bug report against libc++’s implementation of deque.

The code in question was (simplified):

    #ifndef _LIBCPP_NO_EXCEPTIONS
        try
        {
    #endif  // _LIBCPP_NO_EXCEPTIONS
            __buf.push_back(__alloc_traits::allocate(__a, __base::__block_size));
    #ifndef _LIBCPP_NO_EXCEPTIONS
        }
        catch (...)
        {
            __alloc_traits::deallocate(__a, __buf.front(), __base::__block_size);
            throw;
        }
    #endif  // _LIBCPP_NO_EXCEPTIONS

and the bug report read “If the allocation fails, then the pointer never gets
added to __buf, and so the call to deallocate will free the wrong thing.”

Thanks to Tavian Barnes for the bug report and the diagnosis.

There are two operations here that can fail; the first is the allocation,
and the second is the call to push_back. We need to deal with both.

Tavian suggested declaring a variable named __block (since this is allocating
block of memory for the deque to store objects into), and then attempting to
add that to __buf. If that fails, then deallocate __block.

I dismissed this solution immediately, because __block has a special meaning
in Objective-C, and there are people who use libc++ and Objective-C/C++ together.

However, I did try writing with __ablock instead:

    #ifndef _LIBCPP_NO_EXCEPTIONS
        pointer __ablock;
        try
        {
    #endif  // _LIBCPP_NO_EXCEPTIONS
            __ablock = __alloc_traits::allocate(__a, __base::__block_size);
            __buf.push_back(__ablock);
    #ifndef _LIBCPP_NO_EXCEPTIONS
        }
        catch (...)
        {
            __alloc_traits::deallocate(__ablock);
            throw;
        }
    #endif  // _LIBCPP_NO_EXCEPTIONS

Testing this, I found that this solved the problem. Yay!
But the code was still ugly. All those ifdefs and the catch (...) bothered me.

Then I remembered unique_ptr, and realized that it did exactly what I wanted.

Libc++ has an internal class called __allocator_destructor, whose constructor
takes an allocator (by reference) and a size_t, and whose operator() takes a
pointer and deletes it, using the allocator’s deallocate function (passing in
the size). This is used in several places in libc++.

Suddenly, the code got much simpler:

    typedef __allocator_destructor<_Allocator> _Dp;
    unique_ptr<pointer, _Dp> __hold(
        __alloc_traits::allocate(__a, __base::__block_size),
            _Dp(__a, __base::__block_size));
    __buf.push_back(__hold.get());
    __hold.release();

This looks very different – what’s going on here?

This code allocates the block of memory and immediately stuffs it into a
unique_ptr (__hold). Then it adds it to __buf as before. Then we release
it from __hold, which says that it doesn’t need to be deallocated when __hold
goes out of scope.

What about when an exception is thrown?
* If the allocation throws, the __hold is never constructed. Nothing was
allocated, and nothing needs to be deleted.
* If the push_back throws, then __hold‘s destructor deallocates the memory for us.

In either case, the right thing happens.

The general pattern for this is:

    unique_ptr<T> holder(new T{/* params */});
    // set up some stuff that uses holder.get()
    holder.release();

If the “stuff” throws, then holder cleans up. Simple, and no need for try/catch
blocks to handle deallocations.

If you’re writing code that does allocations and other operations that can throw,
this pattern could simplify your code a lot.

Advertisements