Skip to content

Fix ~basic_json causing std::terminate #4654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Ignition wants to merge 1 commit into
base: develop
Choose a base branch
from
+310 −68
Open

Fix ~basic_json causing std::terminate #4654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 110 additions & 35 deletions include/nlohmann/json.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,51 +569,126 @@ 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<basic_json> 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
// Iteratively flatten nested containers to prevent
// stack overflow from recursive destruction. Children
// are promoted into the parent array's spare capacity
// (no allocation), with overflow to a heap stack. If
// the heap stack cannot be allocated, the catch block
// lets RAII recurse instead of calling std::terminate.

#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 a local variable to be processed
basic_json current_item(std::move(stack.back()));
stack.pop_back();
#endif
// Uses the default allocator, we must not be
// affected by a user allocator that throws or
// copies on move.
std::vector<basic_json> stack;

// if current_item is array/object, move
// its children to the stack to be processed later
if (current_item.is_array())
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();
// Move children from source into parent,
// leaving `remain` behind. Stays within
// spare capacity. Uses resize() rather than
// erase() to avoid instantiating move-
// assignment in MSVC module builds.
auto* parent = array;
auto promote = [parent](basic_json & source, size_type remain)
{
if (source.is_array())
{
auto* src = source.m_data.m_value.array;
auto first = src->begin() + static_cast<typename array_t::difference_type>(remain);
std::move(first, src->end(), std::back_inserter(*parent));
src->resize(remain);
}
else if (source.is_object())
{
auto* src = source.m_data.m_value.object;
const auto to_move = src->size() - remain;
for (size_type i = 0; i < to_move; ++i)
{
parent->push_back(std::move(src->begin()->second));
src->erase(src->begin());
}
}
};

while (!array->empty())
{
if (array->back().is_structured())
{
const auto spare = parent->capacity() - parent->size();
const auto n = array->back().size();

if (n <= spare + 1)
{
// All fit (+1 from freeing container slot)
basic_json nested(std::move(array->back()));
array->pop_back();
promote(nested, 0);
}
else if (spare > 0)
{
// Partial: revisited once promoted
// children are processed
promote(array->back(), n - spare);
}
else
{
// No capacity: overflow to stack
stack.push_back(std::move(array->back()));
Copy link
Copy Markdown
Contributor

@gregmarr gregmarr Apr 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still a memory allocation, so you haven't achieved your goal of not allocating memory in the destructor. I take it you are assuming that any failed allocation would result in a thrown allocation that you can catch. Even if that were the case, then you have converted the potential issue from a failed allocation into a stack overflow. It seems to me that the only way to prevent both the stack overflow and the possibility of an allocation failure is the Morris Traversal using the parent pointer. You could simulate that if the parent pointer isn't available using a local container for the current path from the root, but that is also an allocation that can fail.

Copy link
Copy Markdown
Author

@Ignition Ignition Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My goal is not for no allocations. My goal is to avoid std::terminate if an allocator throws. Which can be for more reasons that running out of memory, in my case its because of a quota is exceeded in a subsystem.

I don't want to change the entire representation of an object to make it zero allocations. This is a bigger change than this fix. I just try to 1) reuse existing buffers to flatten where possible, 2) flatten using heap allocation if needed, 3) allow stack call if required.

Its a best effort approach, without restructuring all of the json library. I've had a patch for over a year that works, I'd just like something delivered upstream so I can cleanup downstream project.

Copy link
Copy Markdown
Contributor

@gregmarr gregmarr Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main point is that you are replacing one terminate with another (stack overflow), which the library had previously avoided due to actual users experiencing it. I'm just a contributor but to me that is a non-starter, especially when it's possible to avoid both with the addition of a single pointer, and when that pointer is already there with a configuration option.

array->pop_back();
}
}
else
{
array->pop_back();
}
}
}
else if (current_item.is_object())
else
{
for (auto&& it : *current_item.m_data.m_value.object)
// Move structured values to the stack so that
// the object's destruction only encounters leaves.
for (auto& it : *object)
{
stack.push_back(std::move(it.second));
if (it.second.is_structured())
{
stack.push_back(std::move(it.second));
}
}

current_item.m_data.m_value.object->clear();
}

// it's now safe that current_item gets destructed
// since it doesn't have any children
while (!stack.empty())
{
basic_json current(std::move(stack.back())); // NOLINT(misc-const-correctness)
stack.pop_back();

if (current.is_array())
{
auto* src = current.m_data.m_value.array;
std::move(src->begin(), src->end(), std::back_inserter(stack));
src->clear();
}
else if (current.is_object())
{
auto* src = current.m_data.m_value.object;
for (auto& it : *src)
{
stack.push_back(std::move(it.second));
}
src->clear();
}
}
#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND))
}
catch (...) // NOLINT(bugprone-empty-catch)
{
// Stack allocation failed, RAII cleans up; remaining
// elements are destroyed recursively below.
}
#endif
}

switch (t)
Expand Down
141 changes: 108 additions & 33 deletions single_include/nlohmann/json.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20806,51 +20806,126 @@ 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<basic_json> stack;
// Iteratively flatten nested containers to prevent
// stack overflow from recursive destruction. Children
// are promoted into the parent array's spare capacity
// (no allocation), with overflow to a heap stack. If
// the heap stack cannot be allocated, the catch block
// lets RAII recurse instead of calling std::terminate.

// move the top-level items to stack
if (t == value_t::array)
#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND))
try
{
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())
{
// move the last item to a local variable to be processed
basic_json current_item(std::move(stack.back()));
stack.pop_back();
#endif
// Uses the default allocator, we must not be
// affected by a user allocator that throws or
// copies on move.
std::vector<basic_json> stack;

// if current_item is array/object, move
// its children to the stack to be processed later
if (current_item.is_array())
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));
// Move children from source into parent,
// leaving `remain` behind. Stays within
// spare capacity. Uses resize() rather than
// erase() to avoid instantiating move-
// assignment in MSVC module builds.
auto* parent = array;
auto promote = [parent](basic_json & source, size_type remain)
{
if (source.is_array())
{
auto* src = source.m_data.m_value.array;
auto first = src->begin() + static_cast<typename array_t::difference_type>(remain);
std::move(first, src->end(), std::back_inserter(*parent));
src->resize(remain);
}
else if (source.is_object())
{
auto* src = source.m_data.m_value.object;
const auto to_move = src->size() - remain;
for (size_type i = 0; i < to_move; ++i)
{
parent->push_back(std::move(src->begin()->second));
src->erase(src->begin());
}
}
};

while (!array->empty())
{
if (array->back().is_structured())
{
const auto spare = parent->capacity() - parent->size();
const auto n = array->back().size();

current_item.m_data.m_value.array->clear();
if (n <= spare + 1)
{
// All fit (+1 from freeing container slot)
basic_json nested(std::move(array->back()));
array->pop_back();
promote(nested, 0);
}
else if (spare > 0)
{
// Partial: revisited once promoted
// children are processed
promote(array->back(), n - spare);
}
else
{
// No capacity: overflow to stack
stack.push_back(std::move(array->back()));
array->pop_back();
}
}
else
{
array->pop_back();
}
}
}
else if (current_item.is_object())
else
{
for (auto&& it : *current_item.m_data.m_value.object)
// Move structured values to the stack so that
// the object's destruction only encounters leaves.
for (auto& it : *object)
{
stack.push_back(std::move(it.second));
if (it.second.is_structured())
{
stack.push_back(std::move(it.second));
}
}

current_item.m_data.m_value.object->clear();
}

// it's now safe that current_item gets destructed
// since it doesn't have any children
while (!stack.empty())
{
basic_json current(std::move(stack.back())); // NOLINT(misc-const-correctness)
stack.pop_back();

if (current.is_array())
{
auto* src = current.m_data.m_value.array;
std::move(src->begin(), src->end(), std::back_inserter(stack));
src->clear();
}
else if (current.is_object())
{
auto* src = current.m_data.m_value.object;
for (auto& it : *src)
{
stack.push_back(std::move(it.second));
}
src->clear();
}
}
#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND))
}
catch (...) // NOLINT(bugprone-empty-catch)
{
// Stack allocation failed, RAII cleans up; remaining
// elements are destroyed recursively below.
}
#endif
}

switch (t)
Expand Down
Loading
Loading