Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protect the function wrapped by cpp11::function #395

Merged
merged 3 commits into from
Aug 26, 2024

Conversation

DavisVaughan
Copy link
Member

Closes #294

In theory this is as simple as changing SEXP to sexp in cpp11::function.

In practice, doing just that breaks some protection list related "count" tests. In the tests we assert that the size of the protection list starts at 0 at the beginning of the test, and we'd like to keep that nice assertion / property. However, if we just change to sexp then the count won't start at 0 if we've run the message() related tests first. That's because message() previously maintained a static cpp11::function variable holding base::message. Switching to sexp means that we now always maintains a protection list count of >0, because it never releases base::message. My solution is to switch away from static cpp11::function to a lower level method through r_message() that doesn't use the cpp11 protection list.

When we switch to `sexp`, this would force a cell to always exist in our protection list (because it would never release the `sexp`). While that isn't really a bad thing, it messes up our count related protection list tests.
It is technically possibly to conceive of situations where we could be wrapping a function that is temporary on the R side, and the C++ side of things outlives that R temporary function.
@DavisVaughan DavisVaughan merged commit 61c78b4 into r-lib:main Aug 26, 2024
16 checks passed
@DavisVaughan DavisVaughan deleted the feature/protect-function-data branch August 26, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cpp11::function does not protect its underlying SEXP
1 participant