Skip to content

returning a pointer from a module method hands ownership to r, so returning this double-frees #1533

Open

Description

@kevinushey

When a module function or method returns a raw pointer to an exposed class (T*), Rcpp wraps it in a new R object that owns the pointer. module_wrap() sends pointers through wrap(object<T>(ptr)) → make_new_object<T>(ptr), which creates XPtr<T>(ptr, true), i.e. with a finalizer that deletes the object when the R object is garbage collected:

template <typename Class>
SEXP make_new_object( Class* ptr ){
Rcpp::XPtr<Class> xp( ptr, true ) ;
Function maker = Environment::Rcpp_namespace()[ "cpp_object_maker"] ;
return maker( typeid(Class).name() , xp ) ;
}

That is the right behavior for factory-style functions returning new T(...), and it's what code doing that relies on. But it is not documented anywhere (the Rcpp-modules vignette only mentions pointers for .factory()), and it means any pointer to an object that something else owns is deleted behind its owner's back. Common patterns that crash:

  • returning this to allow chaining (obj$inc()$inc()): each call creates another R object owning the same this, so the object is deleted several times;
  • returning a pointer to a member, or to an element of a container held by the object;
  • returning the same pointer from more than one call.

The result is a double delete (or a delete of memory that was never newed) when the R objects are collected, usually far from the call that caused it.

Reproducible example

repro.cpp:

#include <Rcpp.h>

using namespace Rcpp;

class Counter {
public:
    Counter() : n(0) {}

    // returns this, to allow chaining calls
    Counter* inc() {
        n++;
        return this;
    }

    int n;
};

RCPP_EXPOSED_CLASS_NODECL(Counter)

RCPP_MODULE(counter_module) {
    class_<Counter>("Counter")
        .constructor()
        .method("inc", &Counter::inc)
        .field("n", &Counter::n);
}

repro.R:

Rcpp::sourceCpp("repro.cpp")

counter <- new(Counter)
counter$inc()$inc()
cat("n:", counter$n, "\n")

rm(counter)
invisible(gc())
cat("survived gc()\n")

Output with R 4.6.1 (macOS 27, Rcpp 1.1.2.6 / current master):

C++ object <0x7c18d1d8d0> of class 'Counter' <0x7c1b004200>
n: 2
Abort trap: 6              Rscript repro.R

The chained calls work, but counter, the object returned by the first inc(), and the one returned by the second all own the same Counter, so gc() deletes it three times and the session aborts (exit code 134).

Changing the ownership semantics would break code that returns freshly allocated objects, so I think the fix is to document them. I'll follow up with a PR that adds a section to the Rcpp-modules vignette explaining that a returned T* is owned (and eventually deleted) by R, which pointers are safe to return, and the alternatives (return by value, which copies, or return void and chain in R), plus a test that pins the documented behavior.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions