Merge pull request #2025 from ArtemSarmini/issue-1971-basic_json-push…

…_back Fixes #1971 (memory leak in basic_json::push_back)
nlohmann · Apr 10, 2020 · ea0a7c7 · ea0a7c7
2 parents d01193a + 8db02bc
commit ea0a7c7
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 6 deletions.
diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp
@@ -4875,9 +4875,7 @@ class basic_json
 
         // add element to array (move semantics)
         m_value.array->push_back(std::move(val));
-        // invalidate object: mark it null so we do not call the destructor
-        // cppcheck-suppress accessMoved
-        val.m_type = value_t::null;
+        // if val is moved from, basic_json move constructor marks it null so we do not call the destructor
     }
 
     /*!

diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp
@@ -19734,9 +19734,7 @@ class basic_json
 
         // add element to array (move semantics)
         m_value.array->push_back(std::move(val));
-        // invalidate object: mark it null so we do not call the destructor
-        // cppcheck-suppress accessMoved
-        val.m_type = value_t::null;
+        // if val is moved from, basic_json move constructor marks it null so we do not call the destructor
     }
 
     /*!

diff --git a/test/src/unit-allocator.cpp b/test/src/unit-allocator.cpp
@@ -234,3 +234,45 @@ TEST_CASE("controlled bad_alloc")
         }
     }
 }
+
+namespace
+{
+template<class T>
+struct allocator_no_forward : std::allocator<T>
+{
+    allocator_no_forward() {}
+    template <class U>
+    allocator_no_forward(allocator_no_forward<U>) {}
+
+    template <class U>
+    struct rebind {
+        using other =  allocator_no_forward<U>;
+    };
+
+    template <class... Args>
+    void construct(T* p, const Args&... args)
+    {
+        // force copy even if move is available
+        ::new (static_cast<void*>(p)) T(args...);
+    }
+};
+}
+
+TEST_CASE("bad my_allocator::construct")
+{
+    SECTION("my_allocator::construct doesn't forward")
+    {
+        using bad_alloc_json = nlohmann::basic_json<std::map,
+                                                    std::vector,
+                                                    std::string,
+                                                    bool,
+                                                    std::int64_t,
+                                                    std::uint64_t,
+                                                    double,
+                                                    allocator_no_forward>;
+
+      bad_alloc_json json;
+      json["test"] = bad_alloc_json::array_t();
+      json["test"].push_back("should not leak");
+    }
+}