Skip to content

Commit 4bc9e72

Browse files
IgnitionIgnition
committed
Fix ~basic_json causing std::terminate
During ~basic_json, json_value::destroy() allocated a std::vector to iteratively flatten nested containers. If the allocator threw (e.g. quota violation), the exception propagated through the noexcept destructor boundary, calling std::terminate. Replace with a three-tier strategy. For arrays, children are first promoted into the parent array's spare vector capacity, no heap allocation since the move constructor is noexcept and we stay within capacity. Children that exceed the spare slots overflow to a lazily- allocated std::vector, which is drained iteratively to preserve O(1) call-stack depth. For objects, structured values are moved to the same overflow stack. A try-catch around both tiers ensures that if the overflow stack cannot be allocated, remaining elements are destroyed via RAII recursion instead of calling std::terminate. The overflow stack uses the default allocator to avoid interference from user allocators that may throw or not forward move semantics. Signed-off-by: Gareth Lloyd <gareth.lloyd@memgraph.io>
1 parent 3946872 commit 4bc9e72

3 files changed

Lines changed: 361 additions & 131 deletions

File tree

‎include/nlohmann/json.hpp‎

Lines changed: 106 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -569,51 +569,122 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
569569
}
570570
if (t == value_t::array || t == value_t::object)
571571
{
572-
// flatten the current json_value to a heap-allocated stack
573-
std::vector<basic_json> stack;
574-
575-
// move the top-level items to stack
576-
if (t == value_t::array)
577-
{
578-
stack.reserve(array->size());
579-
std::move(array->begin(), array->end(), std::back_inserter(stack));
580-
}
581-
else
572+
// Iteratively flatten nested containers to prevent
573+
// stack overflow from recursive destruction. Children
574+
// are promoted into the parent array's spare capacity
575+
// (no allocation), with overflow to a heap stack. If
576+
// the heap stack cannot be allocated, the catch block
577+
// lets RAII recurse instead of calling std::terminate.
578+
579+
#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND))
580+
try
582581
{
583-
stack.reserve(object->size());
584-
for (auto&& it : *object)
585-
{
586-
stack.push_back(std::move(it.second));
587-
}
588-
}
589-
590-
while (!stack.empty())
591-
{
592-
// move the last item to a local variable to be processed
593-
basic_json current_item(std::move(stack.back()));
594-
stack.pop_back();
582+
#endif
583+
// Uses the default allocator, we must not be affected
584+
// by a user allocator that throws or copies on move.
585+
std::vector<basic_json> stack;
595586

596-
// if current_item is array/object, move
597-
// its children to the stack to be processed later
598-
if (current_item.is_array())
587+
if (t == value_t::array)
599588
{
600-
std::move(current_item.m_data.m_value.array->begin(), current_item.m_data.m_value.array->end(), std::back_inserter(stack));
601-
602-
current_item.m_data.m_value.array->clear();
589+
// Move children from source into parent, leaving
590+
// `remain` behind. Stays within spare capacity.
591+
auto* parent = array;
592+
auto promote = [parent](basic_json& source, size_type remain)
593+
{
594+
if (source.is_array())
595+
{
596+
auto* src = source.m_data.m_value.array;
597+
auto first = src->begin() + remain;
598+
std::move(first, src->end(), std::back_inserter(*parent));
599+
src->erase(first, src->end());
600+
}
601+
else if (source.is_object())
602+
{
603+
auto* src = source.m_data.m_value.object;
604+
const auto to_move = src->size() - remain;
605+
for (size_type i = 0; i < to_move; ++i)
606+
{
607+
parent->push_back(std::move(src->begin()->second));
608+
src->erase(src->begin());
609+
}
610+
}
611+
};
612+
613+
while (!array->empty())
614+
{
615+
if (array->back().is_structured())
616+
{
617+
const auto spare = parent->capacity() - parent->size();
618+
const auto n = array->back().size();
619+
620+
if (n <= spare + 1)
621+
{
622+
// All fit (+1 from freeing container slot)
623+
basic_json nested(std::move(array->back()));
624+
array->pop_back();
625+
promote(nested, 0);
626+
}
627+
else if (spare > 0)
628+
{
629+
// Partial: revisited once promoted
630+
// children are processed
631+
promote(array->back(), n - spare);
632+
}
633+
else
634+
{
635+
// No capacity: overflow to stack
636+
stack.push_back(std::move(array->back()));
637+
array->pop_back();
638+
}
639+
}
640+
else
641+
{
642+
array->pop_back();
643+
}
644+
}
603645
}
604-
else if (current_item.is_object())
646+
else
605647
{
606-
for (auto&& it : *current_item.m_data.m_value.object)
648+
// Move structured values to the stack so that
649+
// the object's destruction only encounters leaves.
650+
for (auto& it : *object)
607651
{
608-
stack.push_back(std::move(it.second));
652+
if (it.second.is_structured())
653+
{
654+
stack.push_back(std::move(it.second));
655+
}
609656
}
610-
611-
current_item.m_data.m_value.object->clear();
612657
}
613658

614-
// it's now safe that current_item gets destructed
615-
// since it doesn't have any children
659+
while (!stack.empty())
660+
{
661+
basic_json current(std::move(stack.back()));
662+
stack.pop_back();
663+
664+
if (current.is_array())
665+
{
666+
auto* src = current.m_data.m_value.array;
667+
std::move(src->begin(), src->end(), std::back_inserter(stack));
668+
src->clear();
669+
}
670+
else if (current.is_object())
671+
{
672+
auto* src = current.m_data.m_value.object;
673+
for (auto& it : *src)
674+
{
675+
stack.push_back(std::move(it.second));
676+
}
677+
src->clear();
678+
}
679+
}
680+
#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND))
616681
}
682+
catch (...) // NOLINT(bugprone-empty-catch)
683+
{
684+
// Stack allocation failed, RAII cleans up; remaining
685+
// elements are destroyed recursively below.
686+
}
687+
#endif
617688
}
618689

619690
switch (t)

0 commit comments

Comments
 (0)