Skip to content

Commit 7ce2f82

Browse files
committed
protect module method and function results while wrapping in invoke (#1494)
* protect module method and function results while wrapping in invoke * narrow gctorture scope in module regression test
1 parent 06492e7 commit 7ce2f82

5 files changed

Lines changed: 51 additions & 2 deletions

File tree

ChangeLog

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
2026-08-03 Kevin Ushey <kevinushey@gmail.com>
2+
3+
* inst/include/Rcpp/module/class.h (invoke): Protect freshly
4+
computed method results while wrapping them in the result list
5+
(#1493)
6+
* inst/include/Rcpp/module/Module.h (invoke): Idem for module
7+
function results
8+
* inst/tinytest/cpp/Module.cpp: Add regression test
9+
* inst/tinytest/test_module.R: Idem
110
2026-08-03 Kevin Ushey <kevinushey@gmail.com>
211

312
* inst/include/Rcpp/api/meat/proxy.h: Protect fresh SEXPs returned

inst/include/Rcpp/module/Module.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,9 @@ namespace Rcpp {
6060
throw std::range_error( "incorrect number of arguments" ) ;
6161
}
6262

63+
Shield<SEXP> res( fun->operator()( args ) ) ;
6364
return List::create(
64-
_["result"] = fun->operator()( args ),
65+
_["result"] = static_cast<SEXP>(res),
6566
_["void"] = fun->is_void()
6667
) ;
6768
}

inst/include/Rcpp/module/class.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,8 @@
193193
m->operator()( XP(object), args );
194194
return Rcpp::List::create( true ) ;
195195
} else {
196-
return Rcpp::List::create( false, m->operator()( XP(object), args ) ) ;
196+
Shield<SEXP> res( m->operator()( XP(object), args ) ) ;
197+
return Rcpp::List::create( false, static_cast<SEXP>(res) ) ;
197198
}
198199
END_RCPP
199200
}

inst/tinytest/cpp/Module.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,27 @@ double Test_get_x_pointer(ModuleTest* x) {
136136
return x->value;
137137
}
138138

139+
class ModuleGadget {
140+
public:
141+
ModuleGadget() {}
142+
143+
// void overload: forces dispatch through class_::invoke(), which
144+
// wraps method results as list(voidness, result)
145+
void value(int x) {
146+
(void) x;
147+
}
148+
149+
// non-void overload: nothing protects the raw SEXP result while
150+
// class_::invoke() allocates the result list
151+
SEXP value() {
152+
SEXP x = Rf_allocVector(REALSXP, 3);
153+
REAL(x)[0] = 1;
154+
REAL(x)[1] = 2;
155+
REAL(x)[2] = 3;
156+
return x;
157+
}
158+
};
159+
139160
RCPP_MODULE(demoModule) {
140161
function("hello", &hello);
141162
function("bar" , &bar );
@@ -194,6 +215,12 @@ RCPP_MODULE(demoModule) {
194215

195216
.method("get" , &ModuleRandomizer::get)
196217
;
218+
219+
class_<ModuleGadget>("ModuleGadget")
220+
.constructor()
221+
.method("value", static_cast<void (ModuleGadget::*)(int)>(&ModuleGadget::value))
222+
.method("value", static_cast<SEXP (ModuleGadget::*)()>(&ModuleGadget::value))
223+
;
197224
}
198225

199226
// [[Rcpp::export]]

inst/tinytest/test_module.R

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,14 @@ expect_equal(r$get(10), x10)
106106
expect_equal( test_reference( seq(0,10) ), 11L )
107107
expect_equal( test_const_reference( seq(0,10) ), 11L )
108108
expect_equal( test_const( seq(0,10) ), 11L )
109+
110+
## mixed-voidness method overloads dispatch through class_::invoke(),
111+
## which must protect the freshly allocated method result while it
112+
## wraps it in the result list (#1493); under gctorture every
113+
## allocation triggers a collection, so a single call exercises the
114+
## unprotected window deterministically
115+
gadget <- new( ModuleGadget )
116+
gctorture(TRUE)
117+
res <- gadget$value()
118+
gctorture(FALSE)
119+
expect_identical( res, c(1, 2, 3) )

0 commit comments

Comments
 (0)