restore object storage on failed range insert

Author: grisumbras

include/boost/json/impl/object.hpp

@@ -154,6 +154,7 @@ class object::revert_construct
 class object::revert_insert
 {
     object* obj_;
+    table* t_ = nullptr;
     std::size_t size_;
 
     BOOST_JSON_DECL
@@ -163,24 +164,38 @@ class object::revert_insert
 public:
     explicit
     revert_insert(
-        object& obj) noexcept
+        object& obj,
+        std::size_t capacity)
         : obj_(&obj)
         , size_(obj_->size())
     {
+        if( capacity > obj_->capacity() )
+            t_ = obj_->reserve_impl(capacity);
     }
 
     ~revert_insert()
     {
         if(! obj_)
             return;
+
         destroy();
-        obj_->t_->size = static_cast<
-            index_t>(size_);
+        if( t_ )
+        {
+            table::deallocate( obj_->t_, obj_->sp_ );
+            obj_->t_ = t_;
+        }
+        else
+        {
+            obj_->t_->size = static_cast<index_t>(size_);
+        }
     }
 
     void
     commit() noexcept
     {
+        BOOST_ASSERT(obj_);
+        if( t_ )
+            table::deallocate( t_, obj_->sp_ );
         obj_ = nullptr;
     }
 };
@@ -329,6 +344,16 @@ capacity() const noexcept ->
     return t_->capacity;
 }
 
+void
+object::
+reserve(std::size_t new_capacity)
+{
+    if( new_capacity <= capacity() )
+        return;
+    table* const old_table = reserve_impl(new_capacity);
+    table::deallocate( old_table, sp_ );
+}
+
 //----------------------------------------------------------
 //
 // Lookup
@@ -491,8 +516,7 @@ insert(
         BOOST_STATIC_CONSTEXPR source_location loc = BOOST_CURRENT_LOCATION;
         detail::throw_system_error( error::object_too_large, &loc );
     }
-    reserve(n0 + n);
-    revert_insert r(*this);
+    revert_insert r( *this, n0 + n );
     while(first != last)
     {
         insert(*first);

include/boost/json/impl/object.ipp

@@ -452,8 +452,7 @@ insert(
         BOOST_STATIC_CONSTEXPR source_location loc = BOOST_CURRENT_LOCATION;
         detail::throw_system_error( error::object_too_large, &loc );
     }
-    reserve(n0 + init.size());
-    revert_insert r(*this);
+    revert_insert r( *this, n0 + init.size() );
     if(t_->is_small())
     {
         for(auto& iv : init)
@@ -708,10 +707,10 @@ insert_impl(
     return pv;
 }
 
-// rehash to at least `n` buckets
-void
+// allocate new table, copy elements there, and rehash them
+object::table*
 object::
-rehash(std::size_t new_capacity)
+reserve_impl(std::size_t new_capacity)
 {
     BOOST_ASSERT(
         new_capacity > t_->capacity);
@@ -726,8 +725,7 @@ rehash(std::size_t new_capacity)
             size() * sizeof(
                 key_value_pair));
     t->size = t_->size;
-    table::deallocate(t_, sp_);
-    t_ = t;
+    std::swap(t_, t);
 
     if(! t_->is_small())
     {
@@ -744,6 +742,8 @@ rehash(std::size_t new_capacity)
             head = i;
         }
     }
+
+    return t;
 }
 
 bool

include/boost/json/object.hpp

@@ -898,13 +898,9 @@ class object
 
         @throw system_error `new_capacity > max_size()`
     */
+    inline
     void
-    reserve(std::size_t new_capacity)
-    {
-        if(new_capacity <= capacity())
-            return;
-        rehash(new_capacity);
-    }
+    reserve(std::size_t new_capacity);
 
     //------------------------------------------------------
     //
@@ -982,9 +978,9 @@ class object
         are two keys within the range that are equal to each other, only the
         first will be inserted.
 
-        If the size necessary to accomodate elements from the range exceeds
-        @ref capacity(), a rehashing can occur. In that case all iterators and
-        references are invalidated. Otherwise, they are not affected.
+        Insertion may result in rehashing of the container. In that case all
+        iterators and references are invalidated. Otherwise, they are not
+        affected.
 
         @par Precondition
         `first` and `last` are not iterators into `*this`.
@@ -999,7 +995,8 @@ class object
         Linear in `std::distance(first, last)`.
 
         @par Exception Safety
-        Basic guarantee.
+        Strong guarantee for forward iterators, basic guarantee for input
+        iterators.
         Calls to `memory_resource::allocate` may throw.
 
         @param first An input iterator pointing to the first
@@ -1033,8 +1030,7 @@ class object
         are two keys within the initializer list that are equal to each other,
         only the first will be inserted.
 
-        If the size necessary to accomodate elements from the initializer list
-        exceeds @ref capacity(), a rehashing can occur. In that case all
+        Insertion may result in rehashing of the container. In that case all
         iterators and references are invalidated. Otherwise, they are not
         affected.
 
@@ -1607,8 +1603,8 @@ class object
         std::size_t hash);
 
     BOOST_JSON_DECL
-    void
-    rehash(std::size_t new_capacity);
+    table*
+    reserve_impl(std::size_t new_capacity);
 
     BOOST_JSON_DECL
     bool

test/object.cpp

@@ -28,6 +28,28 @@
 namespace boost {
 namespace json {
 
+namespace {
+
+struct throws_on_convert
+{
+    // this member only exists due to MSVC code analysis bug that marks lines
+    // in callers of the type's operator key_value_pair() as unreachable (due
+    // to exception thrown), even if that caller is a function template, and
+    // the line is reachable in other instantiations
+    bool should_throw = true;
+
+    throws_on_convert() = default;
+
+    operator key_value_pair()
+    {
+        if( should_throw )
+            throw std::invalid_argument("");
+        return key_value_pair( "", nullptr);
+    }
+};
+
+} // namespace
+
 BOOST_STATIC_ASSERT( std::is_nothrow_destructible<object>::value );
 BOOST_STATIC_ASSERT( std::is_nothrow_move_constructible<object>::value );
 
@@ -1620,6 +1642,19 @@ class object_test
 
         o.insert( key_value_pair("0", nullptr) );
         BOOST_TEST( capacity == o.capacity() );
+
+        // Check that insertion rolls back reserve when cannot insert all
+        // elements.
+        std::array<throws_on_convert, 10> input;
+        try
+        {
+            o.insert( input.begin(), input.end() );
+        }
+        catch( ... )
+        {
+            // ignore
+        }
+        BOOST_TEST( capacity == o.capacity() );
     }
 
     void