Skip to content

Commit 61c78b4

Browse files
authored
Protect the function wrapped by cpp11::function (#395)
* Don't use `cpp11::function` for `static` variable 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. * Protect the function in `cpp11::function` 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. * NEWS bullet
1 parent 16b2177 commit 61c78b4

File tree

2 files changed

+39
-7
lines changed

2 files changed

+39
-7
lines changed

NEWS.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# cpp11 (development version)
22

3+
* `cpp11::function` now protects its underlying function, for maximum safety
4+
(#294).
5+
36
* `cpp11::writable::r_vector<T>::proxy` now implements copy assignment.
47
Practically this means that `x[i] = y[i]` now works when both `x` and `y`
58
are writable vectors (#300, #339).

inst/include/cpp11/function.hpp

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class function {
3131
}
3232

3333
private:
34-
SEXP data_;
34+
sexp data_;
3535

3636
template <typename... Args>
3737
void construct_call(SEXP val, const named_arg& arg, Args&&... args) const {
@@ -71,36 +71,65 @@ class package {
7171
return safe[Rf_findVarInFrame](R_NamespaceRegistry, name_sexp);
7272
}
7373

74+
// Either base env or in namespace registry, so no protection needed
7475
SEXP data_;
7576
};
7677

78+
namespace detail {
79+
80+
// Special internal way to call `base::message()`
81+
//
82+
// - Pure C, so call with `safe[]`
83+
// - Holds a `static SEXP` for the `base::message` function protected with
84+
// `R_PreserveObject()`
85+
//
86+
// We don't use a `static cpp11::function` because that will infinitely retain a cell in
87+
// our preserve list, which can throw off our counts in the preserve list tests.
88+
inline void r_message(const char* x) {
89+
static SEXP fn = NULL;
90+
91+
if (fn == NULL) {
92+
fn = Rf_findFun(Rf_install("message"), R_BaseEnv);
93+
R_PreserveObject(fn);
94+
}
95+
96+
SEXP x_char = PROTECT(Rf_mkCharCE(x, CE_UTF8));
97+
SEXP x_string = PROTECT(Rf_ScalarString(x_char));
98+
99+
SEXP call = PROTECT(Rf_lang2(fn, x_string));
100+
101+
Rf_eval(call, R_GlobalEnv);
102+
103+
UNPROTECT(3);
104+
}
105+
106+
} // namespace detail
107+
77108
inline void message(const char* fmt_arg) {
78-
static auto R_message = cpp11::package("base")["message"];
79109
#ifdef CPP11_USE_FMT
80110
std::string msg = fmt::format(fmt_arg);
81-
R_message(msg.c_str());
111+
safe[detail::r_message](msg.c_str());
82112
#else
83113
char buff[1024];
84114
int msg;
85115
msg = std::snprintf(buff, 1024, "%s", fmt_arg);
86116
if (msg >= 0 && msg < 1024) {
87-
R_message(buff);
117+
safe[detail::r_message](buff);
88118
}
89119
#endif
90120
}
91121

92122
template <typename... Args>
93123
void message(const char* fmt_arg, Args... args) {
94-
static auto R_message = cpp11::package("base")["message"];
95124
#ifdef CPP11_USE_FMT
96125
std::string msg = fmt::format(fmt_arg, args...);
97-
R_message(msg.c_str());
126+
safe[detail::r_message](msg.c_str());
98127
#else
99128
char buff[1024];
100129
int msg;
101130
msg = std::snprintf(buff, 1024, fmt_arg, args...);
102131
if (msg >= 0 && msg < 1024) {
103-
R_message(buff);
132+
safe[detail::r_message](buff);
104133
}
105134
#endif
106135
}

0 commit comments

Comments
 (0)