[Rcpp-devel] R_PreserveObject and R_ReleaseObject

[Kevin Ushey]

I think the main problem is that it's easy to use your class incorrectly.
For example:

- What happens if your map is destructed? The SEXPs it is handling are
effectively leaked.
- What if you copy your map? Since SEXPs are pointers, you now have two
maps containing the same pointers, and if you were to attempt to remove a
SEXP from both maps problems would occur.

Rather than attempting to manage the lifetime of these R objects in your
map's methods, it's better to have wrapper classes that manage the lifetime
of a single SEXP -- ie, what Rcpp does with its classes.

You might consider just using e.g. std::unordered_map<T, Rcpp::RObject> if
you need to map some arbitrary types to R objects.

Best,
Kevin

On Thu, Aug 16, 2018 at 1:11 AM Simon Dirmeier <simon.dirmeier at web.de>
wrote:

> Dear all,
> I have a question about *R_PreserveObject *and* R_ReleaseObject *and thought
> this would be good place to post it.
> I apologize if the question is not suitable or naive.
>
> Suppose I have something like the class below for which I use Rcpp-modules
> to extend it to R (code not shown).
> I use "insert" to add key-value pair to a map and "remove" to remove an
> entry.
>
> I am not having memory issues or so, but the solution seems wrong to me
> and I am concerned that it causes problems.
> Could you please give me some advice if this implementation is wrong or
> unefficient?
>
> Thank you in advance.
> Best,
> Simon
>
> template<typename T>
> class map
> {
> public:
>     map() = default;
>
>     size_t size()
>     {
>         return map_.size();
>     }
>
>     void insert(T& t, SEXP u)
>     {
>         SEXP s = Rf_duplicate(VECTOR_ELT(u, i));
>         R_PreserveObject(s);
>
>         map_.insert(std::pair<T, SEXP>(t[i], s));
>     }
>
>     void remove(T& t)
>     {
>             auto iter = map_.equal_range(t);
>             for (auto it = iter.first; it != iter.second; ++it)
>             {
>                 R_ReleaseObject(it->second);
>             }
>             map_.erase(t);
>     }
>
> private:
>     std::unordered_map<T, SEXP> map_;
> };
>
>
> _______________________________________________
> Rcpp-devel mailing list
> Rcpp-devel at lists.r-forge.r-project.org
> https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.r-forge.r-project.org/pipermail/rcpp-devel/attachments/20180816/f1c08446/attachment-0001.html>