From 1e383a30843f03c92fc5ce383e8f3cbcb5aee164 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Mon, 3 Aug 2026 12:08:10 -0700 Subject: [PATCH 1/2] protect module method and function results while wrapping in invoke --- ChangeLog | 10 ++++++++++ inst/include/Rcpp/module/Module.h | 3 ++- inst/include/Rcpp/module/class.h | 3 ++- inst/tinytest/cpp/Module.cpp | 27 +++++++++++++++++++++++++++ inst/tinytest/test_module.R | 15 +++++++++++++++ 5 files changed, 56 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 15cefd8f8..8d9f03c30 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2026-08-03 Kevin Ushey + + * inst/include/Rcpp/module/class.h (invoke): Protect freshly + computed method results while wrapping them in the result list + (#1493) + * inst/include/Rcpp/module/Module.h (invoke): Idem for module + function results + * inst/tinytest/cpp/Module.cpp: Add regression test + * inst/tinytest/test_module.R: Idem + 2026-07-24 Dirk Eddelbuettel * DESCRIPTION (Version, Date): Roll micro version and date diff --git a/inst/include/Rcpp/module/Module.h b/inst/include/Rcpp/module/Module.h index 43edc20e0..160b53ae5 100644 --- a/inst/include/Rcpp/module/Module.h +++ b/inst/include/Rcpp/module/Module.h @@ -60,8 +60,9 @@ namespace Rcpp { throw std::range_error( "incorrect number of arguments" ) ; } + Shield res( fun->operator()( args ) ) ; return List::create( - _["result"] = fun->operator()( args ), + _["result"] = static_cast(res), _["void"] = fun->is_void() ) ; } diff --git a/inst/include/Rcpp/module/class.h b/inst/include/Rcpp/module/class.h index 7ada6b65e..a8a1a9563 100644 --- a/inst/include/Rcpp/module/class.h +++ b/inst/include/Rcpp/module/class.h @@ -193,7 +193,8 @@ m->operator()( XP(object), args ); return Rcpp::List::create( true ) ; } else { - return Rcpp::List::create( false, m->operator()( XP(object), args ) ) ; + Shield res( m->operator()( XP(object), args ) ) ; + return Rcpp::List::create( false, static_cast(res) ) ; } END_RCPP } diff --git a/inst/tinytest/cpp/Module.cpp b/inst/tinytest/cpp/Module.cpp index 6fa487b0d..b267c18d3 100644 --- a/inst/tinytest/cpp/Module.cpp +++ b/inst/tinytest/cpp/Module.cpp @@ -136,6 +136,27 @@ double Test_get_x_pointer(ModuleTest* x) { return x->value; } +class ModuleGadget { +public: + ModuleGadget() {} + + // void overload: forces dispatch through class_::invoke(), which + // wraps method results as list(voidness, result) + void value(int x) { + (void) x; + } + + // non-void overload: nothing protects the raw SEXP result while + // class_::invoke() allocates the result list + SEXP value() { + SEXP x = Rf_allocVector(REALSXP, 3); + REAL(x)[0] = 1; + REAL(x)[1] = 2; + REAL(x)[2] = 3; + return x; + } +}; + RCPP_MODULE(demoModule) { function("hello", &hello); function("bar" , &bar ); @@ -194,6 +215,12 @@ RCPP_MODULE(demoModule) { .method("get" , &ModuleRandomizer::get) ; + + class_("ModuleGadget") + .constructor() + .method("value", static_cast(&ModuleGadget::value)) + .method("value", static_cast(&ModuleGadget::value)) + ; } // [[Rcpp::export]] diff --git a/inst/tinytest/test_module.R b/inst/tinytest/test_module.R index 960a33e96..afcef2edc 100644 --- a/inst/tinytest/test_module.R +++ b/inst/tinytest/test_module.R @@ -106,3 +106,18 @@ expect_equal(r$get(10), x10) expect_equal( test_reference( seq(0,10) ), 11L ) expect_equal( test_const_reference( seq(0,10) ), 11L ) expect_equal( test_const( seq(0,10) ), 11L ) + +## mixed-voidness method overloads dispatch through class_::invoke(), +## which must protect the freshly allocated method result while it +## wraps it in the result list (#1493) +gadget <- new( ModuleGadget ) +ok <- TRUE +gctorture(TRUE) +for (i in 1:20) { + if (!identical(gadget$value(), c(1, 2, 3))) { + ok <- FALSE + break + } +} +gctorture(FALSE) +expect_true( ok ) From 8a7d8c3dfcce5eb97b279ae5d877fe3351ba5f33 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Mon, 3 Aug 2026 13:39:40 -0700 Subject: [PATCH 2/2] narrow gctorture scope in module regression test --- inst/tinytest/test_module.R | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/inst/tinytest/test_module.R b/inst/tinytest/test_module.R index afcef2edc..5d1c027e4 100644 --- a/inst/tinytest/test_module.R +++ b/inst/tinytest/test_module.R @@ -109,15 +109,11 @@ expect_equal( test_const( seq(0,10) ), 11L ) ## mixed-voidness method overloads dispatch through class_::invoke(), ## which must protect the freshly allocated method result while it -## wraps it in the result list (#1493) +## wraps it in the result list (#1493); under gctorture every +## allocation triggers a collection, so a single call exercises the +## unprotected window deterministically gadget <- new( ModuleGadget ) -ok <- TRUE gctorture(TRUE) -for (i in 1:20) { - if (!identical(gadget$value(), c(1, 2, 3))) { - ok <- FALSE - break - } -} +res <- gadget$value() gctorture(FALSE) -expect_true( ok ) +expect_identical( res, c(1, 2, 3) )