diff --git a/docs/mkdocs/docs/api/basic_json/insert.md b/docs/mkdocs/docs/api/basic_json/insert.md index 12619a2d8..721cb8ae7 100644 --- a/docs/mkdocs/docs/api/basic_json/insert.md +++ b/docs/mkdocs/docs/api/basic_json/insert.md @@ -65,7 +65,12 @@ the same index, which is now a different value. ## Exception safety -Strong exception safety: if an exception occurs, the original value stays intact. +Overload (1) gives the strong guarantee: if an exception is thrown, there are no changes in the JSON value. + +Overloads (2) to (5) give the basic guarantee: if an exception is thrown while the elements or members are copied +(for example, `std::bad_alloc`), the JSON value may be partially modified, but it stays a valid array or object. + +The exceptions listed below are thrown before the JSON value is changed. ## Exceptions diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 8b8d3dbed..28cad76ba 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -4917,6 +4917,30 @@ public: return {it, res.second}; } + private: + /// Sets the parents of a value when it goes out of scope, also when an + /// exception is thrown: a container insertion that throws may already + /// have added elements, whose parents must be set for the value to stay + /// valid (basic exception guarantee). Does nothing without JSON_DIAGNOSTICS. + class set_parents_on_exit + { + public: + explicit set_parents_on_exit(basic_json* v) noexcept : m_value(v) {} + set_parents_on_exit(const set_parents_on_exit&) = delete; + set_parents_on_exit& operator=(const set_parents_on_exit&) = delete; + set_parents_on_exit(set_parents_on_exit&&) = delete; + set_parents_on_exit& operator=(set_parents_on_exit&&) = delete; + + ~set_parents_on_exit() + { + m_value->set_parents(); + } + + private: + basic_json* m_value; + }; + + public: /// Helper for insertion of an iterator /// @note: This uses std::distance to support GCC 4.8, /// see https://github.com/nlohmann/json/pull/1257 @@ -4926,6 +4950,7 @@ public: iterator result(this); JSON_ASSERT(m_data.m_value.array != nullptr); + const set_parents_on_exit parents(this); auto insert_pos = std::distance(m_data.m_value.array->begin(), pos.m_it.array_iterator); m_data.m_value.array->insert(pos.m_it.array_iterator, std::forward(args)...); result.m_it.array_iterator = m_data.m_value.array->begin() + insert_pos; @@ -4934,7 +4959,6 @@ public: // result.m_it.array_iterator = m_data.m_value.array->insert(pos.m_it.array_iterator, cnt, val); // but the return value of insert is missing in GCC 4.8, so it is written this way instead. - set_parents(); return result; } @@ -4954,8 +4978,16 @@ public: throw_iterator_from_other_value(this); } - // insert to array and return iterator - return insert_iterator(pos, val); + // vector::insert has no effects if copying a single element to the + // end throws; elsewhere, it may already have shifted the elements, so + // copy first and insert by move, which cannot throw but by allocating + if (pos.m_it.array_iterator == m_data.m_value.array->end()) + { + return insert_iterator(pos, val); + } + + basic_json tmp(val); + return insert_iterator(pos, std::move(tmp)); } /// @brief inserts element into array @@ -5087,8 +5119,8 @@ public: detail::throw_literal(detail::exception_id::iterator_from_other_value, "iterators first and last must point to objects", this); } + const set_parents_on_exit parents(this); m_data.m_value.object->insert(first.m_it.object_iterator, last.m_it.object_iterator); - set_parents(); } /// @brief updates a JSON object from another object, overwriting existing keys diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index c5c3ec84f..80f938972 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -42747,6 +42747,30 @@ public: return {it, res.second}; } + private: + /// Sets the parents of a value when it goes out of scope, also when an + /// exception is thrown: a container insertion that throws may already + /// have added elements, whose parents must be set for the value to stay + /// valid (basic exception guarantee). Does nothing without JSON_DIAGNOSTICS. + class set_parents_on_exit + { + public: + explicit set_parents_on_exit(basic_json* v) noexcept : m_value(v) {} + set_parents_on_exit(const set_parents_on_exit&) = delete; + set_parents_on_exit& operator=(const set_parents_on_exit&) = delete; + set_parents_on_exit(set_parents_on_exit&&) = delete; + set_parents_on_exit& operator=(set_parents_on_exit&&) = delete; + + ~set_parents_on_exit() + { + m_value->set_parents(); + } + + private: + basic_json* m_value; + }; + + public: /// Helper for insertion of an iterator /// @note: This uses std::distance to support GCC 4.8, /// see https://github.com/nlohmann/json/pull/1257 @@ -42756,6 +42780,7 @@ public: iterator result(this); JSON_ASSERT(m_data.m_value.array != nullptr); + const set_parents_on_exit parents(this); auto insert_pos = std::distance(m_data.m_value.array->begin(), pos.m_it.array_iterator); m_data.m_value.array->insert(pos.m_it.array_iterator, std::forward(args)...); result.m_it.array_iterator = m_data.m_value.array->begin() + insert_pos; @@ -42764,7 +42789,6 @@ public: // result.m_it.array_iterator = m_data.m_value.array->insert(pos.m_it.array_iterator, cnt, val); // but the return value of insert is missing in GCC 4.8, so it is written this way instead. - set_parents(); return result; } @@ -42784,8 +42808,16 @@ public: throw_iterator_from_other_value(this); } - // insert to array and return iterator - return insert_iterator(pos, val); + // vector::insert has no effects if copying a single element to the + // end throws; elsewhere, it may already have shifted the elements, so + // copy first and insert by move, which cannot throw but by allocating + if (pos.m_it.array_iterator == m_data.m_value.array->end()) + { + return insert_iterator(pos, val); + } + + basic_json tmp(val); + return insert_iterator(pos, std::move(tmp)); } /// @brief inserts element into array @@ -42917,8 +42949,8 @@ public: detail::throw_literal(detail::exception_id::iterator_from_other_value, "iterators first and last must point to objects", this); } + const set_parents_on_exit parents(this); m_data.m_value.object->insert(first.m_it.object_iterator, last.m_it.object_iterator); - set_parents(); } /// @brief updates a JSON object from another object, overwriting existing keys diff --git a/tests/src/unit-allocator.cpp b/tests/src/unit-allocator.cpp index 4062870d5..02b145aae 100644 --- a/tests/src/unit-allocator.cpp +++ b/tests/src/unit-allocator.cpp @@ -12,6 +12,7 @@ #include using nlohmann::json; +#include #include #if JSON_HAS_RANGES #include @@ -804,3 +805,222 @@ TEST_CASE("a failed allocation leaves the value unchanged") #endif } #endif + +// the no-exceptions CI job has no std::bad_alloc to catch; with iterator +// debugging (MSVC STL debug builds) containers allocate inside noexcept +// functions, so failing an allocation there terminates the program +#if !defined(JSON_NOEXCEPTION) && !(defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL > 0) +namespace +{ +// whether every child of j names j as its parent (always true without +// JSON_DIAGNOSTICS, where no parents are stored) +template +bool children_name_their_parent(const BasicJsonType& j) +{ +#if JSON_DIAGNOSTICS + if (j.is_array()) + { + for (const auto& element : *j.m_data.m_value.array) + { + if (element.m_parent != &j) + { + return false; + } + } + } + else if (j.is_object()) + { + for (const auto& element : *j.m_data.m_value.object) + { + if (element.second.m_parent != &j) + { + return false; + } + } + } +#else + static_cast(j); +#endif + return true; +} + +enum class guarantee +{ + basic, // the value stays valid and keeps its type + strong // the value is unchanged +}; + +// Runs op on a fresh value make() with the 0th, 1st, 2nd, ... allocation made +// by the library failing in turn, until op completes. Every failed attempt +// must have thrown std::bad_alloc and left the value valid, with all parent +// pointers intact (copying a value asserts the invariant), and for the strong +// guarantee exactly as it was. +template +void check_exception_guarantee(guarantee g, + const std::function& make, + const std::function& op) +{ + fail_at_alloc_call = -1; + + bool completed = false; + std::size_t failures = 0; + for (std::size_t n = 0; !completed; ++n) + { + CAPTURE(n) + REQUIRE(n < 1000); + + BasicJsonType j = make(); + const std::string before = j.dump(); + const auto type_before = j.type(); + + alloc_call_count = 0; + fail_at_alloc_call = static_cast(n); + try + { + op(j); + completed = true; + } + catch (const std::bad_alloc&) + { + ++failures; + } + fail_at_alloc_call = -1; + + if (!completed) + { + CHECK((j.type() == type_before)); + CHECK(children_name_their_parent(j)); + const BasicJsonType copy(j); // NOLINT(performance-unnecessary-copy-initialization): the copy is the point, it allocates + CHECK(copy.dump() == j.dump()); + if (g == guarantee::strong) + { + CHECK(j.dump() == before); + } + } + } + + // the operation allocates, so at least one attempt must have failed + CHECK(failures > 0); +} + +template +void check_insert_exception_guarantees() +{ + using json_t = BasicJsonType; + using make_t = std::function; + + // strings of this length do not fit into the small string buffer, so + // copying them allocates + const json_t val = std::string(64, 'x'); + const json_t source = json_t::array({std::string(64, 'p'), std::string(64, 'q'), std::string(64, 'r')}); + + for (const bool spare_capacity : + { + false, true + }) + { + CAPTURE(spare_capacity) + + // [ "aaa...", "bbb...", "ccc..." ], with a spare slot if requested + const make_t make = [spare_capacity]() + { + json_t j = json_t::array({std::string(64, 'a'), std::string(64, 'b'), std::string(64, 'c')}); + if (spare_capacity) + { + j.push_back(std::string(64, 'd')); + j.erase(3); + } + return json_t(std::move(j)); + }; + + for (const bool at_end : + { + false, true + }) + { + CAPTURE(at_end) + + SECTION("insert(pos, const basic_json&)") + { + check_exception_guarantee(guarantee::strong, make, [&](json_t& j) + { + j.insert(at_end ? j.end() : j.begin() + 1, val); + }); + } + + SECTION("insert(pos, basic_json&&)") + { + check_exception_guarantee(guarantee::strong, make, [&](json_t& j) + { + j.insert(at_end ? j.end() : j.begin() + 1, json_t(val)); + }); + } + + SECTION("insert(pos, size_type, const basic_json&)") + { + check_exception_guarantee(guarantee::basic, make, [&](json_t& j) + { + j.insert(at_end ? j.end() : j.begin() + 1, 3, val); + }); + } + + SECTION("insert(pos, const_iterator, const_iterator)") + { + check_exception_guarantee(guarantee::basic, make, [&](json_t& j) + { + j.insert(at_end ? j.end() : j.begin() + 1, source.begin(), source.end()); + }); + } + + SECTION("insert(pos, initializer_list_t)") + { + check_exception_guarantee(guarantee::basic, make, [&](json_t& j) + { + j.insert(at_end ? j.end() : j.begin() + 1, {val, val}); + }); + } + } + } + + SECTION("insert(const_iterator, const_iterator) for objects") + { + const json_t members = {{"m", std::string(64, 'm')}, {"n", std::string(64, 'n')}, {"o", std::string(64, 'o')}}; + + SECTION("no member exists yet") + { + check_exception_guarantee(guarantee::basic, []() + { + return json_t({{"a", 1}}); + }, [&](json_t& j) + { + j.insert(members.begin(), members.end()); + }); + } + + SECTION("some members exist already") + { + check_exception_guarantee(guarantee::basic, []() + { + return json_t({{"a", 1}, {"n", std::string(64, 'x')}}); + }, [&](json_t& j) + { + j.insert(members.begin(), members.end()); + }); + } + } +} +} // namespace + +TEST_CASE("insert() provides its documented exception guarantees (#5986)") +{ + SECTION("std::map-backed object_t") + { + check_insert_exception_guarantees>(); + } + + SECTION("ordered_map-backed object_t") + { + check_insert_exception_guarantees>(); + } +} +#endif