Skip to content

Commit b4da845

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 b4da845

File tree

3 files changed

+161
-68
lines changed

3 files changed

+161
-68
lines changed

‎include/nlohmann/json.hpp

+51-34
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
#include <memory> // unique_ptr
3030
#include <string> // string, stoi, to_string
3131
#include <utility> // declval, forward, move, pair, swap
32+
#ifdef _MSC_VER
33+
#include <vcruntime.h> // _HAS_EXCEPTIONS
34+
#endif
3235
#include <vector> // vector
3336

3437
#include <nlohmann/adl_serializer.hpp>
@@ -568,51 +571,65 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
568571
}
569572
if (t == value_t::array || t == value_t::object)
570573
{
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())
574+
#if defined(__cpp_exceptions) || defined(__EXCEPTIONS) || (_HAS_EXCEPTIONS)
575+
try
590576
{
591-
// move the last item to local variable to be processed
592-
basic_json current_item(std::move(stack.back()));
593-
stack.pop_back();
577+
#endif
578+
// flatten the current json_value to a heap-allocated stack
579+
std::vector<basic_json, allocator_type> stack;
594580

595-
// if current_item is array/object, move
596-
// its children to the stack to be processed later
597-
if (current_item.is_array())
581+
// move the top-level items to stack
582+
if (t == value_t::array)
598583
{
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();
584+
stack.reserve(array->size());
585+
std::move(array->begin(), array->end(), std::back_inserter(stack));
602586
}
603-
else if (current_item.is_object())
587+
else
604588
{
605-
for (auto&& it : *current_item.m_data.m_value.object)
589+
stack.reserve(object->size());
590+
for (auto&& it : *object)
606591
{
607592
stack.push_back(std::move(it.second));
608593
}
609-
610-
current_item.m_data.m_value.object->clear();
611594
}
612595

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

618635
switch (t)

‎single_include/nlohmann/json.hpp

+51-34
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
#include <memory> // unique_ptr
3030
#include <string> // string, stoi, to_string
3131
#include <utility> // declval, forward, move, pair, swap
32+
#ifdef _MSC_VER
33+
#include <vcruntime.h> // _HAS_EXCEPTIONS
34+
#endif
3235
#include <vector> // vector
3336

3437
// #include <nlohmann/adl_serializer.hpp>
@@ -20567,51 +20570,65 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
2056720570
}
2056820571
if (t == value_t::array || t == value_t::object)
2056920572
{
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
20573+
#if defined(__cpp_exceptions) || defined(__EXCEPTIONS) || (_HAS_EXCEPTIONS)
20574+
try
2058020575
{
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())
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();
20576+
#endif
20577+
// flatten the current json_value to a heap-allocated stack
20578+
std::vector<basic_json, allocator_type> stack;
2059320579

20594-
// if current_item is array/object, move
20595-
// its children to the stack to be processed later
20596-
if (current_item.is_array())
20580+
// move the top-level items to stack
20581+
if (t == value_t::array)
2059720582
{
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();
20583+
stack.reserve(array->size());
20584+
std::move(array->begin(), array->end(), std::back_inserter(stack));
2060120585
}
20602-
else if (current_item.is_object())
20586+
else
2060320587
{
20604-
for (auto&& it : *current_item.m_data.m_value.object)
20588+
stack.reserve(object->size());
20589+
for (auto&& it : *object)
2060520590
{
2060620591
stack.push_back(std::move(it.second));
2060720592
}
20608-
20609-
current_item.m_data.m_value.object->clear();
2061020593
}
2061120594

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

2061720634
switch (t)

‎tests/src/unit-allocator.cpp

+59
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88

99
#include "doctest_compatibility.h"
1010

11+
#ifdef _MSC_VER
12+
#include <vcruntime.h> // _HAS_EXCEPTIONS
13+
#endif
14+
1115
#define JSON_TESTS_PRIVATE
1216
#include <nlohmann/json.hpp>
1317
using nlohmann::json;
@@ -261,3 +265,58 @@ TEST_CASE("bad my_allocator::construct")
261265
j["test"].push_back("should not leak");
262266
}
263267
}
268+
269+
#if defined(__cpp_exceptions) || defined(__EXCEPTIONS) || (_HAS_EXCEPTIONS)
270+
namespace
271+
{
272+
thread_local bool should_throw = false;
273+
274+
struct QuotaReached : std::exception {};
275+
276+
template<class T>
277+
struct allocator_controlled_throw : std::allocator<T>
278+
{
279+
allocator_controlled_throw() = default;
280+
template <class U>
281+
allocator_controlled_throw(allocator_controlled_throw<U> /*unused*/) {}
282+
283+
template <class U>
284+
struct rebind
285+
{
286+
using other = allocator_controlled_throw<U>;
287+
};
288+
289+
T* allocate(size_t n)
290+
{
291+
if (should_throw)
292+
{
293+
throw QuotaReached{};
294+
}
295+
return std::allocator<T>::allocate(n);
296+
}
297+
};
298+
} // namespace
299+
300+
TEST_CASE("controlled my_allocator::allocate")
301+
{
302+
SECTION("~basic_json tolerant of internal exceptions")
303+
{
304+
using my_alloc_json = nlohmann::basic_json<std::map,
305+
std::vector,
306+
std::string,
307+
bool,
308+
std::int64_t,
309+
std::uint64_t,
310+
double,
311+
allocator_controlled_throw>;
312+
313+
{
314+
auto j = my_alloc_json{1, 2, 3, 4};
315+
should_throw = true;
316+
// `j` is destroyed, ~basic_json is noexcept
317+
// if allocation attempted, exception thrown
318+
// exception should be internally handled
319+
} // should not std::terminate
320+
}
321+
}
322+
#endif

0 commit comments

Comments
 (0)