Skip to content

Commit 683c809

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 683c809

File tree

3 files changed

+141
-68
lines changed

3 files changed

+141
-68
lines changed

‎include/nlohmann/json.hpp

+44-34
Original file line numberDiff line numberDiff line change
@@ -568,50 +568,60 @@ 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
581-
{
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())
571+
try
590572
{
591-
// move the last item to local variable to be processed
592-
basic_json current_item(std::move(stack.back()));
593-
stack.pop_back();
573+
// flatten the current json_value to a heap-allocated stack
574+
std::vector<basic_json, allocator_type> stack;
594575

595-
// if current_item is array/object, move
596-
// its children to the stack to be processed later
597-
if (current_item.is_array())
576+
// move the top-level items to stack
577+
if (t == value_t::array)
598578
{
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();
579+
stack.reserve(array->size());
580+
std::move(array->begin(), array->end(), std::back_inserter(stack));
602581
}
603-
else if (current_item.is_object())
582+
else
604583
{
605-
for (auto&& it : *current_item.m_data.m_value.object)
584+
stack.reserve(object->size());
585+
for (auto&& it : *object)
606586
{
607587
stack.push_back(std::move(it.second));
608588
}
609-
610-
current_item.m_data.m_value.object->clear();
611589
}
612590

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

‎single_include/nlohmann/json.hpp

+44-34
Original file line numberDiff line numberDiff line change
@@ -20567,50 +20567,60 @@ 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
20570+
try
2058020571
{
20581-
stack.reserve(object->size());
20582-
for (auto&& it : *object)
20583-
{
20584-
stack.push_back(std::move(it.second));
20585-
}
20586-
}
20572+
// flatten the current json_value to a heap-allocated stack
20573+
std::vector<basic_json, allocator_type> stack;
2058720574

20588-
while (!stack.empty())
20589-
{
20590-
// move the last item to local variable to be processed
20591-
basic_json current_item(std::move(stack.back()));
20592-
stack.pop_back();
20593-
20594-
// if current_item is array/object, move
20595-
// its children to the stack to be processed later
20596-
if (current_item.is_array())
20575+
// move the top-level items to stack
20576+
if (t == value_t::array)
2059720577
{
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();
20578+
stack.reserve(array->size());
20579+
std::move(array->begin(), array->end(), std::back_inserter(stack));
2060120580
}
20602-
else if (current_item.is_object())
20581+
else
2060320582
{
20604-
for (auto&& it : *current_item.m_data.m_value.object)
20583+
stack.reserve(object->size());
20584+
for (auto&& it : *object)
2060520585
{
2060620586
stack.push_back(std::move(it.second));
2060720587
}
20608-
20609-
current_item.m_data.m_value.object->clear();
2061020588
}
2061120589

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

‎tests/src/unit-allocator.cpp

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

0 commit comments

Comments
 (0)