From 810328368547e918a52161314dcd2a0a2c8a10ed Mon Sep 17 00:00:00 2001 From: Gareth Lloyd Date: Mon, 17 Feb 2025 21:50:12 +0000 Subject: [PATCH] Fix ~basic_json causing std::terminate If there is an allocation problem in `destroy` fallback to recursion approach. Signed-off-by: Gareth Lloyd --- include/nlohmann/json.hpp | 82 +++++++++++++++++++------------- single_include/nlohmann/json.hpp | 82 +++++++++++++++++++------------- tests/src/unit-allocator.cpp | 55 +++++++++++++++++++++ 3 files changed, 151 insertions(+), 68 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 7a857e655f..55feb11242 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -568,51 +568,65 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } if (t == value_t::array || t == value_t::object) { - // flatten the current json_value to a heap-allocated stack - std::vector stack; - - // move the top-level items to stack - if (t == value_t::array) - { - stack.reserve(array->size()); - std::move(array->begin(), array->end(), std::back_inserter(stack)); - } - else +#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND)) + try { - stack.reserve(object->size()); - for (auto&& it : *object) - { - stack.push_back(std::move(it.second)); - } - } - - while (!stack.empty()) - { - // move the last item to local variable to be processed - basic_json current_item(std::move(stack.back())); - stack.pop_back(); +#endif + // flatten the current json_value to a heap-allocated stack + std::vector stack; - // if current_item is array/object, move - // its children to the stack to be processed later - if (current_item.is_array()) + // move the top-level items to stack + if (t == value_t::array) { - std::move(current_item.m_data.m_value.array->begin(), current_item.m_data.m_value.array->end(), std::back_inserter(stack)); - - current_item.m_data.m_value.array->clear(); + stack.reserve(array->size()); + std::move(array->begin(), array->end(), std::back_inserter(stack)); } - else if (current_item.is_object()) + else { - for (auto&& it : *current_item.m_data.m_value.object) + stack.reserve(object->size()); + for (auto&& it : *object) { stack.push_back(std::move(it.second)); } - - current_item.m_data.m_value.object->clear(); } - // it's now safe that current_item get destructed - // since it doesn't have any children + while (!stack.empty()) + { + // move the last item to local variable to be processed + basic_json current_item(std::move(stack.back())); + stack.pop_back(); + + // if current_item is array/object, move + // its children to the stack to be processed later + if (current_item.is_array()) + { + std::move(current_item.m_data.m_value.array->begin(), current_item.m_data.m_value.array->end(), std::back_inserter(stack)); + + current_item.m_data.m_value.array->clear(); + } + else if (current_item.is_object()) + { + for (auto&& it : *current_item.m_data.m_value.object) + { + stack.push_back(std::move(it.second)); + } + + current_item.m_data.m_value.object->clear(); + } + + // it's now safe that current_item get destructed + // since it doesn't have any children + } +#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND)) } + catch (...) // NOLINT(bugprone-empty-catch) + { + // Recursion avoidance has issue allocating temporary space. This may have been `std::bad_alloc` + // or any other exception thrown by a custom allocator. + // RAII will correctly clean up anything moved into `stack`. + // Then we continue with regular recursion based destroy, which will not heap allocate. + } +#endif } switch (t) diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 07dea3c4ab..f947169007 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -20567,51 +20567,65 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } if (t == value_t::array || t == value_t::object) { - // flatten the current json_value to a heap-allocated stack - std::vector stack; - - // move the top-level items to stack - if (t == value_t::array) - { - stack.reserve(array->size()); - std::move(array->begin(), array->end(), std::back_inserter(stack)); - } - else - { - stack.reserve(object->size()); - for (auto&& it : *object) - { - stack.push_back(std::move(it.second)); - } - } - - while (!stack.empty()) +#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND)) + try { - // move the last item to local variable to be processed - basic_json current_item(std::move(stack.back())); - stack.pop_back(); +#endif + // flatten the current json_value to a heap-allocated stack + std::vector stack; - // if current_item is array/object, move - // its children to the stack to be processed later - if (current_item.is_array()) + // move the top-level items to stack + if (t == value_t::array) { - std::move(current_item.m_data.m_value.array->begin(), current_item.m_data.m_value.array->end(), std::back_inserter(stack)); - - current_item.m_data.m_value.array->clear(); + stack.reserve(array->size()); + std::move(array->begin(), array->end(), std::back_inserter(stack)); } - else if (current_item.is_object()) + else { - for (auto&& it : *current_item.m_data.m_value.object) + stack.reserve(object->size()); + for (auto&& it : *object) { stack.push_back(std::move(it.second)); } - - current_item.m_data.m_value.object->clear(); } - // it's now safe that current_item get destructed - // since it doesn't have any children + while (!stack.empty()) + { + // move the last item to local variable to be processed + basic_json current_item(std::move(stack.back())); + stack.pop_back(); + + // if current_item is array/object, move + // its children to the stack to be processed later + if (current_item.is_array()) + { + std::move(current_item.m_data.m_value.array->begin(), current_item.m_data.m_value.array->end(), std::back_inserter(stack)); + + current_item.m_data.m_value.array->clear(); + } + else if (current_item.is_object()) + { + for (auto&& it : *current_item.m_data.m_value.object) + { + stack.push_back(std::move(it.second)); + } + + current_item.m_data.m_value.object->clear(); + } + + // it's now safe that current_item get destructed + // since it doesn't have any children + } +#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND)) } + catch (...) // NOLINT(bugprone-empty-catch) + { + // Recursion avoidance has issue allocating temporary space. This may have been `std::bad_alloc` + // or any other exception thrown by a custom allocator. + // RAII will correctly clean up anything moved into `stack`. + // Then we continue with regular recursion based destroy, which will not heap allocate. + } +#endif } switch (t) diff --git a/tests/src/unit-allocator.cpp b/tests/src/unit-allocator.cpp index 2d27be9608..817e6de6b7 100644 --- a/tests/src/unit-allocator.cpp +++ b/tests/src/unit-allocator.cpp @@ -261,3 +261,58 @@ TEST_CASE("bad my_allocator::construct") j["test"].push_back("should not leak"); } } + +#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND)) +namespace +{ +thread_local bool should_throw = false; + +struct QuotaReached : std::exception {}; + +template +struct allocator_controlled_throw : std::allocator +{ + allocator_controlled_throw() = default; + template + allocator_controlled_throw(allocator_controlled_throw /*unused*/) {} + + template + struct rebind + { + using other = allocator_controlled_throw; + }; + + T* allocate(size_t n) + { + if (should_throw) + { + throw QuotaReached{}; + } + return std::allocator::allocate(n); + } +}; +} // namespace + +TEST_CASE("controlled my_allocator::allocate") +{ + SECTION("~basic_json tolerant of internal exceptions") + { + using my_alloc_json = nlohmann::basic_json; + + { + auto j = my_alloc_json{1, 2, 3, 4}; + should_throw = true; + // `j` is destroyed, ~basic_json is noexcept + // if allocation attempted, exception thrown + // exception should be internally handled + } // should not std::terminate + } +} +#endif