From 77fccaf0e769ce0256481f70ab738a596b454619 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sun, 4 Oct 2026 22:22:17 +0200 Subject: [PATCH] Test that destroy() performs no allocation, even under memory pressure Update the #4842 regression test: it used to check that destroying a nested array/object made at least one allocation through the provided allocator (the old flattening stack). Now that destroy() does not allocate at all, assert the opposite: zero allocations, deallocations only. Rework the #5135 regression test to use a dedicated failing/counting allocator instead of overriding the process-wide ::operator new and ::operator delete, which affected every allocation in the whole unit-regression2 binary rather than just the values under test. Keep the original small repro as one case, and add deep (100000 levels) and wide-and-deep nested array/object/ordered_json cases, all destroyed while every further allocation is made to fail: the destructor must complete without allocating, without throwing, and without leaking. Co-authored-by: Michael Sam <9461037+michaelsam94@users.noreply.github.com> Signed-off-by: Niels Lohmann --- tests/src/unit-allocator.cpp | 30 ++++-- tests/src/unit-regression2.cpp | 184 +++++++++++++++++++++++++++------ 2 files changed, 173 insertions(+), 41 deletions(-) diff --git a/tests/src/unit-allocator.cpp b/tests/src/unit-allocator.cpp index 9a5bf9db1..eeefd4acd 100644 --- a/tests/src/unit-allocator.cpp +++ b/tests/src/unit-allocator.cpp @@ -606,6 +606,7 @@ TEST_CASE("bad my_allocator::construct") namespace { std::size_t counting_allocator_allocations = 0; +std::size_t counting_allocator_deallocations = 0; template struct counting_allocator : std::allocator @@ -618,6 +619,12 @@ struct counting_allocator : std::allocator return std::allocator::allocate(n); } + void deallocate(T* p, std::size_t n) + { + ++counting_allocator_deallocations; + std::allocator::deallocate(p, n); + } + template struct rebind { @@ -626,9 +633,15 @@ struct counting_allocator : std::allocator }; } // namespace -TEST_CASE("destructor uses the provided allocator") +TEST_CASE("destructor performs no allocation, only deallocation") { - // see https://github.com/nlohmann/json/issues/4842 + // see https://github.com/nlohmann/json/issues/4842 and + // https://github.com/nlohmann/json/issues/5135: destroying nested + // arrays/objects used to allocate a temporary stack (first with + // std::allocator, later - after #4842 - with the provided allocator). + // Since that stack could itself throw bad_alloc from inside the + // noexcept destructor (#5135), destroy() no longer allocates anything: + // it only ever frees what is already there. using counting_json = nlohmann::basic_json before); + CHECK(counting_allocator_allocations == allocations_before); + CHECK(counting_allocator_deallocations > deallocations_before); } SECTION("object") { auto* j = new counting_json({{"a", {{"b", {1, 2}}}}, {"c", 3}}); // NOLINT(cppcoreguidelines-owning-memory) - const auto before = counting_allocator_allocations; + const auto allocations_before = counting_allocator_allocations; + const auto deallocations_before = counting_allocator_deallocations; delete j; // NOLINT(cppcoreguidelines-owning-memory) - CHECK(counting_allocator_allocations > before); + CHECK(counting_allocator_allocations == allocations_before); + CHECK(counting_allocator_deallocations > deallocations_before); } } diff --git a/tests/src/unit-regression2.cpp b/tests/src/unit-regression2.cpp index 3548219f0..2bb6ba9f7 100644 --- a/tests/src/unit-regression2.cpp +++ b/tests/src/unit-regression2.cpp @@ -112,44 +112,79 @@ using float_json = nlohmann::basic_json +struct failing_allocator : std::allocator { - if (fail_next_global_allocation) + using std::allocator::allocator; + + failing_allocator() noexcept = default; + template + failing_allocator(const failing_allocator& /*unused*/) noexcept {} // NOLINT(google-explicit-constructor) + + T* allocate(std::size_t n) { - fail_next_global_allocation = false; - throw std::bad_alloc(); + if (fail_next_allocation) + { + fail_next_allocation = false; + throw std::bad_alloc(); + } + ++failing_allocator_allocations; + return std::allocator::allocate(n); } - if (void* const result = std::malloc(size)) + void deallocate(T* p, std::size_t n) { - return result; + ++failing_allocator_deallocations; + std::allocator::deallocate(p, n); } - throw std::bad_alloc(); + template + struct rebind + { + using other = failing_allocator; + }; +}; + +using failing_json = nlohmann::basic_json; +using failing_ordered_json = nlohmann::basic_json; + +// builds `depth` levels of nesting around a scalar, iteratively (never +// recursing: each wrap only moves the previous, already-built value, which +// is O(1)), each level an array or an object depending on `nest_objects` +template +BasicJsonType make_deep_nest(std::size_t depth, bool nest_objects) +{ + BasicJsonType v = 0; + for (std::size_t i = 0; i < depth; ++i) + { + if (nest_objects) + { + BasicJsonType wrapper = BasicJsonType::object(); + wrapper["x"] = std::move(v); + v = std::move(wrapper); + } + else + { + BasicJsonType wrapper = BasicJsonType::array(); + wrapper.push_back(std::move(v)); + v = std::move(wrapper); + } + } + return v; } } // namespace - -void* operator new (std::size_t size) -{ - return checked_malloc(size); -} - -void* operator new[](std::size_t size) -{ - return checked_malloc(size); -} - -void operator delete (void* ptr) noexcept -{ - std::free(ptr); -} - -void operator delete[](void* ptr) noexcept -{ - std::free(ptr); -} #endif ///////////////////////////////////////////////////////////////////// @@ -986,17 +1021,98 @@ TEST_CASE("regression test - excessive binary container size honors allow_except } #if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND)) && !defined(JSON_NOEXCEPTION) -TEST_CASE("regression test #5135 - destructor tolerates stack allocation failure") +TEST_CASE("regression test #5135 - destructor never allocates, even under memory pressure") { + // Before the fix, ~basic_json() flattened a nested array/object into a + // heap-allocated std::vector to avoid recursing; that allocation could + // itself throw bad_alloc, which escapes a noexcept destructor and + // terminates the program. destroy() no longer allocates anything, so + // none of the sections below ever observe fail_next_allocation being + // consumed: CHECK(fail_next_allocation) confirms it was never touched. + + SECTION("the original report: a small, mixed array/object nest") { - json j = json::array({json::array({1, 2}), json::object({{"key", json::array({3})}})}); - fail_next_global_allocation = true; + failing_allocator_allocations = 0; + failing_allocator_deallocations = 0; + { + failing_json j = failing_json::array( + { + failing_json::array({1, 2}), + failing_json::object({{"key", failing_json::array({3})}}) + }); + fail_next_allocation = true; + } // j is destroyed here, with every further allocation set to fail + + CHECK(fail_next_allocation); + fail_next_allocation = false; + CHECK(failing_allocator_deallocations > 0); } - const bool allocation_failure_was_injected = !fail_next_global_allocation; - fail_next_global_allocation = false; + SECTION("100000-deep nested array") + { + std::size_t allocations_before = 0; + { + failing_json j = make_deep_nest(100000, false); + allocations_before = failing_allocator_allocations; + fail_next_allocation = true; + } - CHECK(allocation_failure_was_injected); + CHECK(fail_next_allocation); + fail_next_allocation = false; + CHECK(failing_allocator_allocations == allocations_before); + } + + SECTION("100000-deep nested object") + { + std::size_t allocations_before = 0; + { + failing_json j = make_deep_nest(100000, true); + allocations_before = failing_allocator_allocations; + fail_next_allocation = true; + } + + CHECK(fail_next_allocation); + fail_next_allocation = false; + CHECK(failing_allocator_allocations == allocations_before); + } + + SECTION("100000-deep nested ordered_json") + { + std::size_t allocations_before = 0; + { + failing_ordered_json j = make_deep_nest(100000, true); + allocations_before = failing_allocator_allocations; + fail_next_allocation = true; + } + + CHECK(fail_next_allocation); + fail_next_allocation = false; + CHECK(failing_allocator_allocations == allocations_before); + } + + SECTION("wide and deep: 1000 arrays of 1000 elements, each a small nested object") + { + std::size_t allocations_before = 0; + { + failing_json wide = failing_json::array(); + for (std::size_t i = 0; i < 1000; ++i) + { + failing_json inner = failing_json::array(); + for (std::size_t k = 0; k < 1000; ++k) + { + inner.push_back(failing_json::object({{"a", 1}, {"b", failing_json::array({1, 2, 3})}})); + } + wide.push_back(std::move(inner)); + } + + allocations_before = failing_allocator_allocations; + fail_next_allocation = true; + } + + CHECK(fail_next_allocation); + fail_next_allocation = false; + CHECK(failing_allocator_allocations == allocations_before); + } } #endif