From 157c066f2b127d478291636b07f31872032486bf Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Thu, 1 Nov 2018 16:17:11 -0400 Subject: [PATCH] Fix memory leak in Json move assignment operator * When move assignment is creates a cyclic ownership pattern memory was being leaked. This patch breaks the cycle. * Fixes: #2572 --- src/ripple/json/impl/json_value.cpp | 11 ++++++----- src/ripple/json/json_value.h | 4 ++-- src/test/json/json_value_test.cpp | 12 ++++++++++++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/ripple/json/impl/json_value.cpp b/src/ripple/json/impl/json_value.cpp index 7fee5a95a8..88e0a429e9 100644 --- a/src/ripple/json/impl/json_value.cpp +++ b/src/ripple/json/impl/json_value.cpp @@ -348,10 +348,10 @@ Value::~Value () } Value& -Value::operator= ( const Value& other ) +Value::operator=(Value const& other) { - Value temp ( other ); - swap ( temp ); + Value tmp(other); + swap(tmp); return *this; } @@ -365,9 +365,10 @@ Value::Value ( Value&& other ) noexcept } Value& -Value::operator= ( Value&& other ) noexcept +Value::operator=(Value&& other) { - swap ( other ); + Value tmp(std::move(other)); + swap(tmp); return *this; } diff --git a/src/ripple/json/json_value.h b/src/ripple/json/json_value.h index 1e5ca69109..242c22004b 100644 --- a/src/ripple/json/json_value.h +++ b/src/ripple/json/json_value.h @@ -233,10 +233,10 @@ public: Value ( const Value& other ); ~Value (); - Value& operator= ( const Value& other ); + Value& operator= ( Value const& other ); + Value& operator= ( Value&& other ); Value ( Value&& other ) noexcept; - Value& operator= ( Value&& other ) noexcept; /// Swap values. /// \note Currently, comments are intentionally not swapped, for diff --git a/src/test/json/json_value_test.cpp b/src/test/json/json_value_test.cpp index b20204d14a..82de38f473 100644 --- a/src/test/json/json_value_test.cpp +++ b/src/test/json/json_value_test.cpp @@ -296,6 +296,17 @@ struct json_value_test : beast::unit_test::suite } } + void + test_leak() + { + // When run with the address sanitizer, this test confirms there is no + // memory leak with the scenario below. + Json::Value a; + a[0u] = 1; + a = std::move(a[0u]); + pass(); + } + void run () override { test_bool (); @@ -306,6 +317,7 @@ struct json_value_test : beast::unit_test::suite test_comparisons (); test_compact (); test_nest_limits (); + test_leak(); } };