Skip to content

Commit

Permalink
Fix memory leak with move assignment operator
Browse files Browse the repository at this point in the history
  • Loading branch information
DavisVaughan committed Jul 28, 2024
1 parent 5f563b6 commit 276059b
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 1 deletion.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# cpp11 (development version)

* Fixed a memory leak with the `cpp11::writable::r_vector` move assignment
operator (#338).

* Dropped support for gcc 4.8, mainly an issue for extremely old CentOS 7
systems which used that as their default compiler. As of June 2024, CentOS 7
is past its Vendor end of support date and therefore also out of scope for
Expand Down
18 changes: 18 additions & 0 deletions cpp11test/src/test-integers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -207,4 +208,21 @@ 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::preserved.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::preserved.count();

expect_true(before == 0);
expect_true(after - before == 1);
}
}
18 changes: 18 additions & 0 deletions cpp11test/src/test-list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -162,4 +163,21 @@ 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::preserved.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::preserved.count();

expect_true(before == 0);
expect_true(after - before == 1);
}
}
5 changes: 5 additions & 0 deletions inst/include/cpp11/protect.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,11 @@ static struct {
REprintf("---\n");
}

R_xlen_t count() {
// Elements in list = `Rf_xlength()` - `head` - `tail`
return Rf_xlength(get_preserve_list()) - 2;
}

void release(SEXP cell) {
if (cell == R_NilValue) {
return;
Expand Down
2 changes: 1 addition & 1 deletion inst/include/cpp11/r_vector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ inline r_vector<T>& r_vector<T>::operator=(r_vector<T>&& rhs) {
SEXP old_protect = protect_;

data_ = rhs.data_;
protect_ = preserved.insert(data_);
protect_ = rhs.protect_;

preserved.release(old_protect);

Expand Down

0 comments on commit 276059b

Please sign in to comment.