From d5ac35142622f35841a78b356f33618f03c85629 Mon Sep 17 00:00:00 2001 From: mistersandman Date: Mon, 14 Jan 2019 21:16:43 +0100 Subject: [PATCH 1/3] Explicit iterative destruction of JSON containers (objects and arrays) instead of implicit recursive destruction in order to avoid stack overflows for deeply nested hierarchies --- include/nlohmann/json.hpp | 75 +++++++++++++++++++++++++++----- single_include/nlohmann/json.hpp | 75 +++++++++++++++++++++++++++----- 2 files changed, 130 insertions(+), 20 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 3b3da50fff..b2b41c76b7 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -997,18 +997,73 @@ class basic_json switch (t) { case value_t::object: - { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, object); - std::allocator_traits::deallocate(alloc, object, 1); - break; - } - case value_t::array: { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, array); - std::allocator_traits::deallocate(alloc, array, 1); + if ((t == value_t::object && !this->object->empty()) || (t == value_t::array && !this->array->empty())) + { + std::vector> stack; + stack.push_back(std::make_pair(this, t)); + while (!stack.empty()) + { + json_value* value; + value_t type; + std::tie(value, type) = stack.back(); + if (type == value_t::object) + { + while (!value->object->empty()) + { + basic_json& inner_value = value->object->begin()->second; + value_t inner_type = inner_value.type(); + if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) + { + stack.push_back(std::make_pair(&inner_value.m_value, inner_type)); + break; + } + else + { + value->object->erase(value->object->begin()); + } + } + if (value->object->empty()) + { + stack.pop_back(); + } + } + else + { + while (!value->array->empty()) + { + basic_json& inner_value = value->array->back(); + value_t inner_type = inner_value.type(); + if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) + { + stack.push_back(std::make_pair(&inner_value.m_value, inner_type)); + break; + } + else + { + value->array->pop_back(); + } + } + if (value->array->empty()) + { + stack.pop_back(); + } + } + } + } + if (t == value_t::object) + { + AllocatorType alloc; + std::allocator_traits::destroy(alloc, object); + std::allocator_traits::deallocate(alloc, object, 1); + } + else + { + AllocatorType alloc; + std::allocator_traits::destroy(alloc, array); + std::allocator_traits::deallocate(alloc, array, 1); + } break; } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 70b6c8a855..a90ae27361 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -13465,18 +13465,73 @@ class basic_json switch (t) { case value_t::object: - { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, object); - std::allocator_traits::deallocate(alloc, object, 1); - break; - } - case value_t::array: { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, array); - std::allocator_traits::deallocate(alloc, array, 1); + if ((t == value_t::object && !this->object->empty()) || (t == value_t::array && !this->array->empty())) + { + std::vector> stack; + stack.push_back(std::make_pair(this, t)); + while (!stack.empty()) + { + json_value* value; + value_t type; + std::tie(value, type) = stack.back(); + if (type == value_t::object) + { + while (!value->object->empty()) + { + basic_json& inner_value = value->object->begin()->second; + value_t inner_type = inner_value.type(); + if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) + { + stack.push_back(std::make_pair(&inner_value.m_value, inner_type)); + break; + } + else + { + value->object->erase(value->object->begin()); + } + } + if (value->object->empty()) + { + stack.pop_back(); + } + } + else + { + while (!value->array->empty()) + { + basic_json& inner_value = value->array->back(); + value_t inner_type = inner_value.type(); + if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) + { + stack.push_back(std::make_pair(&inner_value.m_value, inner_type)); + break; + } + else + { + value->array->pop_back(); + } + } + if (value->array->empty()) + { + stack.pop_back(); + } + } + } + } + if (t == value_t::object) + { + AllocatorType alloc; + std::allocator_traits::destroy(alloc, object); + std::allocator_traits::deallocate(alloc, object, 1); + } + else + { + AllocatorType alloc; + std::allocator_traits::destroy(alloc, array); + std::allocator_traits::deallocate(alloc, array, 1); + } break; } From 17f59bf56e8df08595fb2d1fa6f588c2de946e84 Mon Sep 17 00:00:00 2001 From: mistersandman Date: Tue, 15 Jan 2019 11:45:10 +0100 Subject: [PATCH 2/3] Use emplace_back instead of push_back --- include/nlohmann/json.hpp | 6 +++--- single_include/nlohmann/json.hpp | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index b2b41c76b7..b8afa6d37a 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -1002,7 +1002,7 @@ class basic_json if ((t == value_t::object && !this->object->empty()) || (t == value_t::array && !this->array->empty())) { std::vector> stack; - stack.push_back(std::make_pair(this, t)); + stack.emplace_back(this, t); while (!stack.empty()) { json_value* value; @@ -1016,7 +1016,7 @@ class basic_json value_t inner_type = inner_value.type(); if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) { - stack.push_back(std::make_pair(&inner_value.m_value, inner_type)); + stack.emplace_back(&inner_value.m_value, inner_type); break; } else @@ -1037,7 +1037,7 @@ class basic_json value_t inner_type = inner_value.type(); if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) { - stack.push_back(std::make_pair(&inner_value.m_value, inner_type)); + stack.emplace_back(&inner_value.m_value, inner_type); break; } else diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index a90ae27361..6f6515cad5 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -13470,7 +13470,7 @@ class basic_json if ((t == value_t::object && !this->object->empty()) || (t == value_t::array && !this->array->empty())) { std::vector> stack; - stack.push_back(std::make_pair(this, t)); + stack.emplace_back(this, t); while (!stack.empty()) { json_value* value; @@ -13484,7 +13484,7 @@ class basic_json value_t inner_type = inner_value.type(); if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) { - stack.push_back(std::make_pair(&inner_value.m_value, inner_type)); + stack.emplace_back(&inner_value.m_value, inner_type); break; } else @@ -13505,7 +13505,7 @@ class basic_json value_t inner_type = inner_value.type(); if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) { - stack.push_back(std::make_pair(&inner_value.m_value, inner_type)); + stack.emplace_back(&inner_value.m_value, inner_type); break; } else From df42d482597a0a272ebe1bad280fe1ca58b01697 Mon Sep 17 00:00:00 2001 From: mistersandman Date: Thu, 21 Feb 2019 22:34:40 +0100 Subject: [PATCH 3/3] Improve readability by restructuring nested code blocks into their own functions --- include/nlohmann/json.hpp | 151 ++++++++++++++++++------------- single_include/nlohmann/json.hpp | 151 ++++++++++++++++++------------- 2 files changed, 178 insertions(+), 124 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index b8afa6d37a..0430f7beeb 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -997,73 +997,28 @@ class basic_json switch (t) { case value_t::object: - case value_t::array: { - if ((t == value_t::object && !this->object->empty()) || (t == value_t::array && !this->array->empty())) - { - std::vector> stack; - stack.emplace_back(this, t); - while (!stack.empty()) - { - json_value* value; - value_t type; - std::tie(value, type) = stack.back(); - if (type == value_t::object) - { - while (!value->object->empty()) - { - basic_json& inner_value = value->object->begin()->second; - value_t inner_type = inner_value.type(); - if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) - { - stack.emplace_back(&inner_value.m_value, inner_type); - break; - } - else - { - value->object->erase(value->object->begin()); - } - } - if (value->object->empty()) - { - stack.pop_back(); - } - } - else - { - while (!value->array->empty()) - { - basic_json& inner_value = value->array->back(); - value_t inner_type = inner_value.type(); - if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) - { - stack.emplace_back(&inner_value.m_value, inner_type); - break; - } - else - { - value->array->pop_back(); - } - } - if (value->array->empty()) - { - stack.pop_back(); - } - } - } - } - if (t == value_t::object) + if (!this->object->empty()) { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, object); - std::allocator_traits::deallocate(alloc, object, 1); + destroy_subelements_iterative(t); } - else + + AllocatorType alloc; + std::allocator_traits::destroy(alloc, object); + std::allocator_traits::deallocate(alloc, object, 1); + break; + } + + case value_t::array: + { + if (!this->array->empty()) { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, array); - std::allocator_traits::deallocate(alloc, array, 1); + destroy_subelements_iterative(t); } + + AllocatorType alloc; + std::allocator_traits::destroy(alloc, array); + std::allocator_traits::deallocate(alloc, array, 1); break; } @@ -1081,6 +1036,78 @@ class basic_json } } } + +private: + + /// destroys all children in an iterative depth-first traversal using an explicit stack + /// to avoid stack overflows in a recursive destruction of deeply nested hierarchies + void destroy_subelements_iterative(value_t t) noexcept + { + std::vector> stack; + stack.emplace_back(this, t); + while (!stack.empty()) + { + json_value* value; + value_t type; + std::tie(value, type) = stack.back(); + switch (type) + { + case value_t::object: + destroy_object_subelements(stack, value); + break; + case value_t::array: + destroy_array_subelements(stack, value); + break; + default: + // this should never happen + break; + } + } + } + + void destroy_object_subelements(std::vector>& stack, json_value* value) noexcept + { + while (!value->object->empty()) + { + basic_json& inner_value = value->object->begin()->second; + value_t inner_type = inner_value.type(); + if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) + { + // the current inner value has children, push it onto the stack and return to continue the depth-first + // traversal with the children, this limits the maximal stack size exactly to the maximal hierarchy depth + stack.emplace_back(&inner_value.m_value, inner_type); + return; + } + // the current inner value does not have any children (anymore), remove it from this object + value->object->erase(value->object->begin()); + } + // the object does not have any children anymore, remove it from the stack + // the object itself will be destroyed later + stack.pop_back(); + } + + void destroy_array_subelements(std::vector>& stack, json_value* value) noexcept + { + while (!value->array->empty()) + { + // start removing children from the back of the array, to guarantee O(1) removal complexity + basic_json& inner_value = value->array->back(); + value_t inner_type = inner_value.type(); + if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) + { + // the current inner value has children, push it onto the stack and return to continue the depth-first + // traversal with the children, this limits the maximal stack size exactly to the maximal hierarchy depth + stack.emplace_back(&inner_value.m_value, inner_type); + return; + } + // the current inner value does not have any children (anymore), remove it from this array + value->array->pop_back(); + } + // the array does not have any children anymore, remove it from the stack + // the array itself will be destroyed later + stack.pop_back(); + } + }; /*! diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 6f6515cad5..d51494d940 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -13465,73 +13465,28 @@ class basic_json switch (t) { case value_t::object: - case value_t::array: { - if ((t == value_t::object && !this->object->empty()) || (t == value_t::array && !this->array->empty())) - { - std::vector> stack; - stack.emplace_back(this, t); - while (!stack.empty()) - { - json_value* value; - value_t type; - std::tie(value, type) = stack.back(); - if (type == value_t::object) - { - while (!value->object->empty()) - { - basic_json& inner_value = value->object->begin()->second; - value_t inner_type = inner_value.type(); - if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) - { - stack.emplace_back(&inner_value.m_value, inner_type); - break; - } - else - { - value->object->erase(value->object->begin()); - } - } - if (value->object->empty()) - { - stack.pop_back(); - } - } - else - { - while (!value->array->empty()) - { - basic_json& inner_value = value->array->back(); - value_t inner_type = inner_value.type(); - if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) - { - stack.emplace_back(&inner_value.m_value, inner_type); - break; - } - else - { - value->array->pop_back(); - } - } - if (value->array->empty()) - { - stack.pop_back(); - } - } - } - } - if (t == value_t::object) + if (!this->object->empty()) { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, object); - std::allocator_traits::deallocate(alloc, object, 1); + destroy_subelements_iterative(t); } - else + + AllocatorType alloc; + std::allocator_traits::destroy(alloc, object); + std::allocator_traits::deallocate(alloc, object, 1); + break; + } + + case value_t::array: + { + if (!this->array->empty()) { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, array); - std::allocator_traits::deallocate(alloc, array, 1); + destroy_subelements_iterative(t); } + + AllocatorType alloc; + std::allocator_traits::destroy(alloc, array); + std::allocator_traits::deallocate(alloc, array, 1); break; } @@ -13549,6 +13504,78 @@ class basic_json } } } + +private: + + /// destroys all children in an iterative depth-first traversal using an explicit stack + /// to avoid stack overflows in a recursive destruction of deeply nested hierarchies + void destroy_subelements_iterative(value_t t) noexcept + { + std::vector> stack; + stack.emplace_back(this, t); + while (!stack.empty()) + { + json_value* value; + value_t type; + std::tie(value, type) = stack.back(); + switch (type) + { + case value_t::object: + destroy_object_subelements(stack, value); + break; + case value_t::array: + destroy_array_subelements(stack, value); + break; + default: + // this should never happen + break; + } + } + } + + void destroy_object_subelements(std::vector>& stack, json_value* value) noexcept + { + while (!value->object->empty()) + { + basic_json& inner_value = value->object->begin()->second; + value_t inner_type = inner_value.type(); + if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) + { + // the current inner value has children, push it onto the stack and return to continue the depth-first + // traversal with the children, this limits the maximal stack size exactly to the maximal hierarchy depth + stack.emplace_back(&inner_value.m_value, inner_type); + return; + } + // the current inner value does not have any children (anymore), remove it from this object + value->object->erase(value->object->begin()); + } + // the object does not have any children anymore, remove it from the stack + // the object itself will be destroyed later + stack.pop_back(); + } + + void destroy_array_subelements(std::vector>& stack, json_value* value) noexcept + { + while (!value->array->empty()) + { + // start removing children from the back of the array, to guarantee O(1) removal complexity + basic_json& inner_value = value->array->back(); + value_t inner_type = inner_value.type(); + if ((inner_type == value_t::object || inner_type == value_t::array) && !inner_value.empty()) + { + // the current inner value has children, push it onto the stack and return to continue the depth-first + // traversal with the children, this limits the maximal stack size exactly to the maximal hierarchy depth + stack.emplace_back(&inner_value.m_value, inner_type); + return; + } + // the current inner value does not have any children (anymore), remove it from this array + value->array->pop_back(); + } + // the array does not have any children anymore, remove it from the stack + // the array itself will be destroyed later + stack.pop_back(); + } + }; /*!