Skip to content

Commit f0ddc31

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 f0ddc31

3 files changed

Lines changed: 365 additions & 131 deletions

File tree

‎include/nlohmann/json.hpp‎

Lines changed: 108 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -569,51 +569,124 @@ 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
584+
// affected by a user allocator that throws or
585+
// copies on move.
586+
std::vector<basic_json> stack;
595587

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

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

619692
switch (t)

0 commit comments

Comments
 (0)