diff --git a/NEWS.md b/NEWS.md index d4f21208..b5f9cb61 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # cpp11 (development version) +* Fixed a memory leak with the `cpp11::writable::r_vector` move assignment + operator (#338). + * The approach for the protection list managed by cpp11 has been tweaked slightly. In 0.4.6, we changed to an approach that creates one protection list per compilation unit, but we now believe we've found an approach that is diff --git a/cpp11test/src/test-integers.cpp b/cpp11test/src/test-integers.cpp index 6f876c57..a7aec08f 100644 --- a/cpp11test/src/test-integers.cpp +++ b/cpp11test/src/test-integers.cpp @@ -2,6 +2,7 @@ #include "cpp11/doubles.hpp" #include "cpp11/function.hpp" #include "cpp11/integers.hpp" +#include "cpp11/protect.hpp" #include "cpp11/strings.hpp" #include @@ -207,4 +208,22 @@ context("integers-C++") { int y = NA_INTEGER; expect_true(cpp11::is_na(y)); } + + test_that("writable integer vector temporary isn't leaked (#338)") { + R_xlen_t before = cpp11::detail::store::count(); + + // +1 from `x` allocation + cpp11::writable::integers x(1); + + // Calls move assignment operator `operator=(r_vector&& rhs)` + // +1 from `rhs` allocation and move into `x` + // -1 from old `x` release + x = cpp11::writable::integers(1); + + R_xlen_t after = cpp11::detail::store::count(); + + expect_true(before == 0); + // TODO: This should be 1 but writable vectors are being double protected + expect_true(after - before == 2); + } } diff --git a/cpp11test/src/test-list.cpp b/cpp11test/src/test-list.cpp index 0abf6002..9226039e 100644 --- a/cpp11test/src/test-list.cpp +++ b/cpp11test/src/test-list.cpp @@ -2,6 +2,7 @@ #include "cpp11/integers.hpp" #include "cpp11/list.hpp" #include "cpp11/logicals.hpp" +#include "cpp11/protect.hpp" #include "cpp11/raws.hpp" #include "cpp11/strings.hpp" @@ -162,4 +163,22 @@ context("list-C++") { expect_true(Rf_xlength(y) == 0); expect_true(y != R_NilValue); } + + test_that("writable list vector temporary isn't leaked (#338)") { + R_xlen_t before = cpp11::detail::store::count(); + + // +1 from `x` allocation + cpp11::writable::list x(1); + + // Calls move assignment operator `operator=(r_vector&& rhs)` + // +1 from `rhs` allocation and move into `x` + // -1 from old `x` release + x = cpp11::writable::list(1); + + R_xlen_t after = cpp11::detail::store::count(); + + expect_true(before == 0); + // TODO: This should be 1 but writable vectors are being double protected + expect_true(after - before == 2); + } } diff --git a/inst/include/cpp11/r_vector.hpp b/inst/include/cpp11/r_vector.hpp index 22739af2..90758d32 100644 --- a/inst/include/cpp11/r_vector.hpp +++ b/inst/include/cpp11/r_vector.hpp @@ -843,7 +843,7 @@ inline r_vector& r_vector::operator=(r_vector&& rhs) { SEXP old_protect = protect_; data_ = rhs.data_; - protect_ = detail::store::insert(data_); + protect_ = rhs.protect_; detail::store::release(old_protect);