Skip to content

Commit cd04272

Browse files
IgnitionIgnition
committed
Fix ~basic_json causing std::terminate
If there is an allocation problem in `destroy` fallback to recursion approach. Signed-off-by: Gareth Lloyd <gareth.lloyd@memgraph.io>
1 parent 0b6881a commit cd04272

File tree

3 files changed

+151
-68
lines changed

3 files changed

+151
-68
lines changed

‎include/nlohmann/json.hpp

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -568,51 +568,65 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
568568
}
569569
if (t == value_t::array || t == value_t::object)
570570
{
571-
// flatten the current json_value to a heap-allocated stack
572-
std::vector<basic_json> stack;
573-
574-
// move the top-level items to stack
575-
if (t == value_t::array)
576-
{
577-
stack.reserve(array->size());
578-
std::move(array->begin(), array->end(), std::back_inserter(stack));
579-
}
580-
else
571+
#ifdef __cpp_exceptions
572+
try
581573
{
582-
stack.reserve(object->size());
583-
for (auto&& it : *object)
584-
{
585-
stack.push_back(std::move(it.second));
586-
}
587-
}
588-
589-
while (!stack.empty())
590-
{
591-
// move the last item to local variable to be processed
592-
basic_json current_item(std::move(stack.back()));
593-
stack.pop_back();
574+
#endif
575+
// flatten the current json_value to a heap-allocated stack
576+
std::vector<basic_json, allocator_type> stack;
594577

595-
// if current_item is array/object, move
596-
// its children to the stack to be processed later
597-
if (current_item.is_array())
578+
// move the top-level items to stack
579+
if (t == value_t::array)
598580
{
599-
std::move(current_item.m_data.m_value.array->begin(), current_item.m_data.m_value.array->end(), std::back_inserter(stack));
600-
601-
current_item.m_data.m_value.array->clear();
581+
stack.reserve(array->size());
582+
std::move(array->begin(), array->end(), std::back_inserter(stack));
602583
}
603-
else if (current_item.is_object())
584+
else
604585
{
605-
for (auto&& it : *current_item.m_data.m_value.object)
586+
stack.reserve(object->size());
587+
for (auto&& it : *object)
606588
{
607589
stack.push_back(std::move(it.second));
608590
}
609-
610-
current_item.m_data.m_value.object->clear();
611591
}
612592

613-
// it's now safe that current_item get destructed
614-
// since it doesn't have any children
593+
while (!stack.empty())
594+
{
595+
// move the last item to local variable to be processed
596+
basic_json current_item(std::move(stack.back()));
597+
stack.pop_back();
598+
599+
// if current_item is array/object, move
600+
// its children to the stack to be processed later
601+
if (current_item.is_array())
602+
{
603+
std::move(current_item.m_data.m_value.array->begin(), current_item.m_data.m_value.array->end(), std::back_inserter(stack));
604+
605+
current_item.m_data.m_value.array->clear();
606+
}
607+
else if (current_item.is_object())
608+
{
609+
for (auto&& it : *current_item.m_data.m_value.object)
610+
{
611+
stack.push_back(std::move(it.second));
612+
}
613+
614+
current_item.m_data.m_value.object->clear();
615+
}
616+
617+
// it's now safe that current_item get destructed
618+
// since it doesn't have any children
619+
}
620+
#ifdef __cpp_exceptions
615621
}
622+
catch (...) // NOLINT(bugprone-empty-catch)
623+
{
624+
// Recursion avoidance has issue allocating temporary space. This may have been `std::bad_alloc`
625+
// or any other exception thrown by a custom allocator.
626+
// RAII will correctly clean up anything moved into `stack`.
627+
// Then we continue with regular recursion based destroy, which will not heap allocate.
628+
}
629+
#endif
616630
}
617631

618632
switch (t)

‎single_include/nlohmann/json.hpp

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -20567,51 +20567,65 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
2056720567
}
2056820568
if (t == value_t::array || t == value_t::object)
2056920569
{
20570-
// flatten the current json_value to a heap-allocated stack
20571-
std::vector<basic_json> stack;
20572-
20573-
// move the top-level items to stack
20574-
if (t == value_t::array)
20575-
{
20576-
stack.reserve(array->size());
20577-
std::move(array->begin(), array->end(), std::back_inserter(stack));
20578-
}
20579-
else
20580-
{
20581-
stack.reserve(object->size());
20582-
for (auto&& it : *object)
20583-
{
20584-
stack.push_back(std::move(it.second));
20585-
}
20586-
}
20587-
20588-
while (!stack.empty())
20570+
#ifdef __cpp_exceptions
20571+
try
2058920572
{
20590-
// move the last item to local variable to be processed
20591-
basic_json current_item(std::move(stack.back()));
20592-
stack.pop_back();
20573+
#endif
20574+
// flatten the current json_value to a heap-allocated stack
20575+
std::vector<basic_json, allocator_type> stack;
2059320576

20594-
// if current_item is array/object, move
20595-
// its children to the stack to be processed later
20596-
if (current_item.is_array())
20577+
// move the top-level items to stack
20578+
if (t == value_t::array)
2059720579
{
20598-
std::move(current_item.m_data.m_value.array->begin(), current_item.m_data.m_value.array->end(), std::back_inserter(stack));
20599-
20600-
current_item.m_data.m_value.array->clear();
20580+
stack.reserve(array->size());
20581+
std::move(array->begin(), array->end(), std::back_inserter(stack));
2060120582
}
20602-
else if (current_item.is_object())
20583+
else
2060320584
{
20604-
for (auto&& it : *current_item.m_data.m_value.object)
20585+
stack.reserve(object->size());
20586+
for (auto&& it : *object)
2060520587
{
2060620588
stack.push_back(std::move(it.second));
2060720589
}
20608-
20609-
current_item.m_data.m_value.object->clear();
2061020590
}
2061120591

20612-
// it's now safe that current_item get destructed
20613-
// since it doesn't have any children
20592+
while (!stack.empty())
20593+
{
20594+
// move the last item to local variable to be processed
20595+
basic_json current_item(std::move(stack.back()));
20596+
stack.pop_back();
20597+
20598+
// if current_item is array/object, move
20599+
// its children to the stack to be processed later
20600+
if (current_item.is_array())
20601+
{
20602+
std::move(current_item.m_data.m_value.array->begin(), current_item.m_data.m_value.array->end(), std::back_inserter(stack));
20603+
20604+
current_item.m_data.m_value.array->clear();
20605+
}
20606+
else if (current_item.is_object())
20607+
{
20608+
for (auto&& it : *current_item.m_data.m_value.object)
20609+
{
20610+
stack.push_back(std::move(it.second));
20611+
}
20612+
20613+
current_item.m_data.m_value.object->clear();
20614+
}
20615+
20616+
// it's now safe that current_item get destructed
20617+
// since it doesn't have any children
20618+
}
20619+
#ifdef __cpp_exceptions
2061420620
}
20621+
catch (...) // NOLINT(bugprone-empty-catch)
20622+
{
20623+
// Recursion avoidance has issue allocating temporary space. This may have been `std::bad_alloc`
20624+
// or any other exception thrown by a custom allocator.
20625+
// RAII will correctly clean up anything moved into `stack`.
20626+
// Then we continue with regular recursion based destroy, which will not heap allocate.
20627+
}
20628+
#endif
2061520629
}
2061620630

2061720631
switch (t)

‎tests/src/unit-allocator.cpp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,3 +261,58 @@ TEST_CASE("bad my_allocator::construct")
261261
j["test"].push_back("should not leak");
262262
}
263263
}
264+
265+
#ifdef __cpp_exceptions
266+
namespace
267+
{
268+
thread_local bool should_throw = false;
269+
270+
struct QuotaReached : std::exception {};
271+
272+
template<class T>
273+
struct allocator_controlled_throw : std::allocator<T>
274+
{
275+
allocator_controlled_throw() = default;
276+
template <class U>
277+
allocator_controlled_throw(allocator_controlled_throw<U> /*unused*/) {}
278+
279+
template <class U>
280+
struct rebind
281+
{
282+
using other = allocator_controlled_throw<U>;
283+
};
284+
285+
T* allocate(size_t n)
286+
{
287+
if (should_throw)
288+
{
289+
throw QuotaReached{};
290+
}
291+
return std::allocator<T>::allocate(n);
292+
}
293+
};
294+
} // namespace
295+
296+
TEST_CASE("controlled my_allocator::allocate")
297+
{
298+
SECTION("~basic_json tolerant of internal exceptions")
299+
{
300+
using my_alloc_json = nlohmann::basic_json<std::map,
301+
std::vector,
302+
std::string,
303+
bool,
304+
std::int64_t,
305+
std::uint64_t,
306+
double,
307+
allocator_controlled_throw>;
308+
309+
{
310+
auto j = my_alloc_json{1, 2, 3, 4};
311+
should_throw = true;
312+
// `j` is destroyed, ~basic_json is noexcept
313+
// if allocation attempted, exception thrown
314+
// exception should be internally handled
315+
} // should not std::terminate
316+
}
317+
}
318+
#endif

0 commit comments

Comments
 (0)