Fix memory leak with move assignment operator (#366)

Author: DavisVaughan

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

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 <testthat.h>
@@ -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<T>&& 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);
+  }
 }

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<T>&& 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);
+  }
 }

inst/include/cpp11/r_vector.hpp

@@ -843,7 +843,7 @@ inline r_vector<T>& r_vector<T>::operator=(r_vector<T>&& rhs) {
   SEXP old_protect = protect_;
 
   data_ = rhs.data_;
-  protect_ = detail::store::insert(data_);
+  protect_ = rhs.protect_;
 
   detail::store::release(old_protect);