diff --git a/README.md b/README.md index 2cb8603b..1d065407 100644 --- a/README.md +++ b/README.md @@ -974,6 +974,7 @@ I deeply appreciate the help of the following people. - [johnfb](https://github.com/johnfb) found a bug in the implementation of CBOR's indefinite length strings. - [Paul Fultz II](https://github.com/pfultz2) added a note on the cget package manager. - [Wilson Lin](https://github.com/wla80) made the integration section of the README more concise. +- [RalfBielig](https://github.com/ralfbielig) detected and fixed a memory leak in the parser callback. Thanks a lot for helping out! Please [let me know](mailto:mail@nlohmann.me) if I forgot someone. diff --git a/include/nlohmann/detail/input/parser.hpp b/include/nlohmann/detail/input/parser.hpp index 5c0c2e94..8346cd4b 100644 --- a/include/nlohmann/detail/input/parser.hpp +++ b/include/nlohmann/detail/input/parser.hpp @@ -434,6 +434,7 @@ class parser if (keep and callback and not callback(depth, parse_event_t::value, result)) { + result.m_value.destroy(result.m_type); result.m_type = value_t::discarded; } } diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 3a52ec8c..3b3ab74b 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -955,7 +955,7 @@ class basic_json /// constructor for rvalue strings json_value(string_t&& value) { - string = create(std::move(value)); + string = create(std::forward < string_t&& > (value)); } /// constructor for objects @@ -967,7 +967,7 @@ class basic_json /// constructor for rvalue objects json_value(object_t&& value) { - object = create(std::move(value)); + object = create(std::forward < object_t&& > (value)); } /// constructor for arrays @@ -979,7 +979,7 @@ class basic_json /// constructor for rvalue arrays json_value(array_t&& value) { - array = create(std::move(value)); + array = create(std::forward < array_t&& > (value)); } void destroy(value_t t) noexcept diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index ff992269..4439829a 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -3911,6 +3911,7 @@ class parser if (keep and callback and not callback(depth, parse_event_t::value, result)) { + result.m_value.destroy(result.m_type); result.m_type = value_t::discarded; } } @@ -11018,7 +11019,7 @@ class basic_json /// constructor for rvalue strings json_value(string_t&& value) { - string = create(std::move(value)); + string = create(std::forward < string_t&& > (value)); } /// constructor for objects @@ -11030,7 +11031,7 @@ class basic_json /// constructor for rvalue objects json_value(object_t&& value) { - object = create(std::move(value)); + object = create(std::forward < object_t&& > (value)); } /// constructor for arrays @@ -11042,7 +11043,7 @@ class basic_json /// constructor for rvalue arrays json_value(array_t&& value) { - array = create(std::move(value)); + array = create(std::forward < array_t&& > (value)); } void destroy(value_t t) noexcept diff --git a/test/src/unit-class_parser.cpp b/test/src/unit-class_parser.cpp index fc7563cd..39753d68 100644 --- a/test/src/unit-class_parser.cpp +++ b/test/src/unit-class_parser.cpp @@ -1527,91 +1527,6 @@ TEST_CASE("parser class") }); CHECK(j_empty_array == json()); } - - /* - SECTION("skip in GeoJSON") - { - auto geojsonExample = R"( - { "type": "FeatureCollection", - "features": [ - { "type": "Feature", - "geometry": {"type": "Point", "coordinates": [102.0, 0.5]}, - "properties": {"prop0": "value0"} - }, - { "type": "Feature", - "geometry": { - "type": "LineString", - "coordinates": [ - [102.0, 0.0], [103.0, 1.0], [104.0, 0.0], [105.0, 1.0] - ] - }, - "properties": { - "prop0": "value0", - "prop1": 0.0 - } - }, - { "type": "Feature", - "geometry": { - "type": "Polygon", - "coordinates": [ - [ [100.0, 0.0], [101.0, 0.0], [101.0, 1.0], - [100.0, 1.0], [100.0, 0.0] ] - ] - }, - "properties": { - "prop0": "value0", - "prop1": {"this": "that"} - } - } - ] - })"; - - json::parser_callback_t cb = [&](int, json::parse_event_t event, json & parsed) - { - // skip uninteresting events - if (event == json::parse_event_t::value and !parsed.is_primitive()) - { - return false; - } - - switch (event) - { - case json::parse_event_t::key: - { - return true; - } - case json::parse_event_t::value: - { - return false; - } - case json::parse_event_t::object_start: - { - return true; - } - case json::parse_event_t::object_end: - { - return false; - } - case json::parse_event_t::array_start: - { - return true; - } - case json::parse_event_t::array_end: - { - return false; - } - - default: - { - return true; - } - } - }; - - auto j = json::parse(geojsonExample, cb, true); - CHECK(j == json()); - } - */ } SECTION("constructing from contiguous containers") diff --git a/test/src/unit-regression.cpp b/test/src/unit-regression.cpp index c84c2750..604def6c 100644 --- a/test/src/unit-regression.cpp +++ b/test/src/unit-regression.cpp @@ -1514,4 +1514,87 @@ TEST_CASE("regression tests") CHECK(ff.x == 3); nlohmann::json nj = lj; // This line works as expected } + + SECTION("issue #1001 - Fix memory leak during parser callback") + { + auto geojsonExample = R"( + { "type": "FeatureCollection", + "features": [ + { "type": "Feature", + "geometry": {"type": "Point", "coordinates": [102.0, 0.5]}, + "properties": {"prop0": "value0"} + }, + { "type": "Feature", + "geometry": { + "type": "LineString", + "coordinates": [ + [102.0, 0.0], [103.0, 1.0], [104.0, 0.0], [105.0, 1.0] + ] + }, + "properties": { + "prop0": "value0", + "prop1": 0.0 + } + }, + { "type": "Feature", + "geometry": { + "type": "Polygon", + "coordinates": [ + [ [100.0, 0.0], [101.0, 0.0], [101.0, 1.0], + [100.0, 1.0], [100.0, 0.0] ] + ] + }, + "properties": { + "prop0": "value0", + "prop1": {"this": "that"} + } + } + ] + })"; + + json::parser_callback_t cb = [&](int, json::parse_event_t event, json & parsed) + { + // skip uninteresting events + if (event == json::parse_event_t::value and !parsed.is_primitive()) + { + return false; + } + + switch (event) + { + case json::parse_event_t::key: + { + return true; + } + case json::parse_event_t::value: + { + return false; + } + case json::parse_event_t::object_start: + { + return true; + } + case json::parse_event_t::object_end: + { + return false; + } + case json::parse_event_t::array_start: + { + return true; + } + case json::parse_event_t::array_end: + { + return false; + } + + default: + { + return true; + } + } + }; + + auto j = json::parse(geojsonExample, cb, true); + CHECK(j == json()); + } }