From b501ab8639255915a13d40b1806c45d932b59013 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Thu, 29 Oct 2026 11:04:10 -0700 Subject: [PATCH 1/4] turn exceptions during module initialization into r errors --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index 02f61fba4..5f5259ec3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -41,12 +41,15 @@ * inst/tinytest/cpp/Module.cpp: Add a class deriving from another * inst/tinytest/test_module.R: Test the new checks +2026-10-29 Kevin Ushey + * inst/include/Rcpp/Module.h (RCPP_MODULE): Wrap the module boot function in BEGIN_RCPP / END_RCPP so that an exception thrown while the module initializes raises an R error instead of terminating the session (#1525) * inst/include/Rcpp/Module.h (internal::ModuleScope): New RAII guard restoring the previous module scope however the module body exits + clearing the current module scope however the module body exits * inst/tinytest/cpp/ModuleInitError.cpp: New module whose initialization throws * inst/tinytest/cpp/Module.cpp (Module_scope_is_cleared): New From 0f6dad8b54c80723e9954f02e0f4eb843b5236c8 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Thu, 29 Oct 2026 11:06:49 -0700 Subject: [PATCH 2/4] run the module body only once per dll --- ChangeLog | 8 ++++++++ inst/include/Rcpp/Module.h | 28 +++++++++++++++++----------- inst/tinytest/test_module.R | 10 ++++++++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5f5259ec3..7a5e90a75 100644 --- a/ChangeLog +++ b/ChangeLog @@ -41,6 +41,14 @@ * inst/tinytest/cpp/Module.cpp: Add a class deriving from another * inst/tinytest/test_module.R: Test the new checks +2026-10-29 Kevin Ushey + + * inst/include/Rcpp/Module.h (RCPP_MODULE): Run the module body only the + first time the boot function is called, since it adds to the static + module and every Module() call boots the module again, which duplicated + constructors and methods (#1527) + * inst/tinytest/test_module.R: Test loading a module repeatedly + 2026-10-29 Kevin Ushey * inst/include/Rcpp/Module.h (RCPP_MODULE): Wrap the module boot diff --git a/inst/include/Rcpp/Module.h b/inst/include/Rcpp/Module.h index cada7d800..ee8a1a66b 100644 --- a/inst/include/Rcpp/Module.h +++ b/inst/include/Rcpp/Module.h @@ -637,17 +637,23 @@ namespace Rcpp { #define RCPP_MODULE_BOOT(name) _rcpp_module_boot_##name // the boot function is called from R via .Call(), so exceptions thrown by -// the module body must be turned into R errors -#define RCPP_MODULE(name) \ - void _rcpp_module_##name##_init() ; \ - static Rcpp::Module _rcpp_module_##name( # name ) ; \ - extern "C" SEXP _rcpp_module_boot_##name(){ \ - BEGIN_RCPP \ - Rcpp::internal::ModuleScope _rcpp_module_scope( & _rcpp_module_##name ) ; \ - _rcpp_module_##name##_init( ) ; \ - return Rcpp::XPtr( & _rcpp_module_##name , false ) ; \ - END_RCPP \ - } \ +// the module body must be turned into R errors; it runs every time the +// module is loaded, but the body adds to the static module, so it may only +// run once +#define RCPP_MODULE(name) \ + void _rcpp_module_##name##_init() ; \ + static Rcpp::Module _rcpp_module_##name( # name ) ; \ + static bool _rcpp_module_##name##_initialized = false ; \ + extern "C" SEXP _rcpp_module_boot_##name(){ \ + BEGIN_RCPP \ + if( ! _rcpp_module_##name##_initialized ){ \ + Rcpp::internal::ModuleScope _rcpp_module_scope( & _rcpp_module_##name ) ; \ + _rcpp_module_##name##_init( ) ; \ + _rcpp_module_##name##_initialized = true ; \ + } \ + return Rcpp::XPtr( & _rcpp_module_##name , false ) ; \ + END_RCPP \ + } \ void _rcpp_module_##name##_init() // silly little dance to suppress a 'defined but not used variable' warning diff --git a/inst/tinytest/test_module.R b/inst/tinytest/test_module.R index 4186cf1e1..8eeef0ea3 100644 --- a/inst/tinytest/test_module.R +++ b/inst/tinytest/test_module.R @@ -189,3 +189,13 @@ expect_identical( res, c(1, 2, 3) ) ## rather than an error (which used to surface at garbage collection) r <- new( ModuleRandomizer ) expect_identical( r$finalize(), NULL ) + +## loading a module again must not re-run its body, which used to add +## every constructor and method to the static module once more (#1527); +## this redefines the module's classes, so it comes last +is_demo_module <- function(dll) is.loaded("_rcpp_module_boot_demoModule", PACKAGE = dll[["name"]]) +dll <- Filter(is_demo_module, getLoadedDLLs())[[1L]] +mod <- Module( "demoModule", PACKAGE = dll, mustStart = TRUE ) +mod <- Module( "demoModule", PACKAGE = dll, mustStart = TRUE ) +expect_equal( length( mod$ModuleWorld@constructors ), 1L ) +expect_equal( length( mod$ModuleWorld@methods$greet@nargs ), 1L ) From 711ac20667dfef0ccf3b8689092db28091637287 Mon Sep 17 00:00:00 2001 From: Dirk Eddelbuettel Date: Tue, 3 Nov 2026 16:30:32 -0600 Subject: [PATCH 3/4] Minor ChangeLog cleanup --- ChangeLog | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7a5e90a75..22626718e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -41,23 +41,18 @@ * inst/tinytest/cpp/Module.cpp: Add a class deriving from another * inst/tinytest/test_module.R: Test the new checks -2026-10-29 Kevin Ushey - * inst/include/Rcpp/Module.h (RCPP_MODULE): Run the module body only the first time the boot function is called, since it adds to the static module and every Module() call boots the module again, which duplicated constructors and methods (#1527) * inst/tinytest/test_module.R: Test loading a module repeatedly -2026-10-29 Kevin Ushey - * inst/include/Rcpp/Module.h (RCPP_MODULE): Wrap the module boot function in BEGIN_RCPP / END_RCPP so that an exception thrown while the module initializes raises an R error instead of terminating the session (#1525) * inst/include/Rcpp/Module.h (internal::ModuleScope): New RAII guard restoring the previous module scope however the module body exits - clearing the current module scope however the module body exits * inst/tinytest/cpp/ModuleInitError.cpp: New module whose initialization throws * inst/tinytest/cpp/Module.cpp (Module_scope_is_cleared): New From 78f5ac3d6c72e2bd5df61b379d204626aa606e5f Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Wed, 4 Nov 2026 08:57:18 -0800 Subject: [PATCH 4/4] clear the module when its body throws A body that registers some classes or functions and then throws left them in the static module, and the initialized flag stayed false, so a later successful load appended a second copy of each. Module::clear() drops the registrations in the catch block so the body always runs against an empty module. The C callables registered by Add() cannot be removed, but running the body again just registers them again. --- ChangeLog | 10 +++++- inst/include/Rcpp/Module.h | 10 ++++-- inst/include/Rcpp/module/Module.h | 16 +++++++++ inst/tinytest/cpp/ModuleInitRetry.cpp | 50 +++++++++++++++++++++++++++ inst/tinytest/test_module.R | 12 +++++++ 5 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 inst/tinytest/cpp/ModuleInitRetry.cpp diff --git a/ChangeLog b/ChangeLog index 22626718e..79737d708 100644 --- a/ChangeLog +++ b/ChangeLog @@ -45,7 +45,15 @@ first time the boot function is called, since it adds to the static module and every Module() call boots the module again, which duplicated constructors and methods (#1527) - * inst/tinytest/test_module.R: Test loading a module repeatedly + * inst/include/Rcpp/module/Module.h (Module::clear): New, removes the + registered classes and functions + * inst/include/Rcpp/Module.h (RCPP_MODULE): Clear the module when the + body throws, so that a later load runs the body against an empty module + instead of appending to what the failed run registered + * inst/tinytest/cpp/ModuleInitRetry.cpp: New module whose first load + fails after registering a class and a function + * inst/tinytest/test_module.R: Test loading a module repeatedly, and + loading one again after its body threw * inst/include/Rcpp/Module.h (RCPP_MODULE): Wrap the module boot function in BEGIN_RCPP / END_RCPP so that an exception thrown while the diff --git a/inst/include/Rcpp/Module.h b/inst/include/Rcpp/Module.h index ee8a1a66b..2b57cea27 100644 --- a/inst/include/Rcpp/Module.h +++ b/inst/include/Rcpp/Module.h @@ -639,7 +639,8 @@ namespace Rcpp { // the boot function is called from R via .Call(), so exceptions thrown by // the module body must be turned into R errors; it runs every time the // module is loaded, but the body adds to the static module, so it may only -// run once +// run once, and a body that throws part-way leaves nothing behind so that +// the next load does not append to what it registered #define RCPP_MODULE(name) \ void _rcpp_module_##name##_init() ; \ static Rcpp::Module _rcpp_module_##name( # name ) ; \ @@ -648,7 +649,12 @@ namespace Rcpp { BEGIN_RCPP \ if( ! _rcpp_module_##name##_initialized ){ \ Rcpp::internal::ModuleScope _rcpp_module_scope( & _rcpp_module_##name ) ; \ - _rcpp_module_##name##_init( ) ; \ + try { \ + _rcpp_module_##name##_init( ) ; \ + } catch( ... ) { \ + _rcpp_module_##name.clear() ; \ + throw ; \ + } \ _rcpp_module_##name##_initialized = true ; \ } \ return Rcpp::XPtr( & _rcpp_module_##name , false ) ; \ diff --git a/inst/include/Rcpp/module/Module.h b/inst/include/Rcpp/module/Module.h index a8539d7f2..08537b4e7 100644 --- a/inst/include/Rcpp/module/Module.h +++ b/inst/include/Rcpp/module/Module.h @@ -203,6 +203,22 @@ namespace Rcpp { classes.insert( CLASS_PAIR( name_ , cptr ) ) ; } + // drops everything the module body registered; used when the body + // throws, so that running it again starts from an empty module (the + // C callables registered by Add() cannot be removed, but running the + // body again simply registers them again) + inline void clear(){ + for( MAP::iterator it = functions.begin(); it != functions.end(); ++it ){ + delete it->second ; + } + functions.clear() ; + + for( CLASS_MAP::iterator it = classes.begin(); it != classes.end(); ++it ){ + delete it->second ; + } + classes.clear() ; + } + inline bool has_function( const std::string& m){ // #nocov start return functions.find(m) != functions.end() ; } diff --git a/inst/tinytest/cpp/ModuleInitRetry.cpp b/inst/tinytest/cpp/ModuleInitRetry.cpp new file mode 100644 index 000000000..6829027ba --- /dev/null +++ b/inst/tinytest/cpp/ModuleInitRetry.cpp @@ -0,0 +1,50 @@ +// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; indent-tabs-mode: nil; -*- +// +// ModuleInitRetry.cpp: Rcpp R/C++ interface class library -- module whose +// initialization fails the first time only +// +// Copyright (C) 2026 Kevin Ushey +// +// This file is part of Rcpp. +// +// Rcpp is free software: you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 2 of the License, or +// (at your option) any later version. +// +// Rcpp is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Rcpp. If not, see . + +#include + +using namespace Rcpp; + +class ModuleInitRetry { +public: + ModuleInitRetry() {} + int value() { return 42; } +}; + +int retry_add_one(int x) { + return x + 1; +} + +static int boot_count = 0; + +// registers a class and a function, then throws on the first boot only, so +// the second boot must not find what the first one registered +RCPP_MODULE(moduleInitRetry) { + class_("ModuleInitRetry") + .constructor() + .method("value", &ModuleInitRetry::value) + ; + function("retry_add_one", &retry_add_one); + + if (++boot_count == 1) + throw std::runtime_error("first boot fails"); +} diff --git a/inst/tinytest/test_module.R b/inst/tinytest/test_module.R index 8eeef0ea3..165d43884 100644 --- a/inst/tinytest/test_module.R +++ b/inst/tinytest/test_module.R @@ -167,6 +167,18 @@ expect_error( r$get(10L), "not initialized" ) expect_error( Rcpp::sourceCpp("cpp/ModuleInitError.cpp"), "no such class" ) expect_true( Module_scope_is_cleared() ) +## a body that throws after registering some of its classes and functions +## leaves nothing behind, so loading the module again does not end up with +## two copies of what the failed run registered +expect_error( Rcpp::sourceCpp("cpp/ModuleInitRetry.cpp"), "first boot fails" ) +is_retry_module <- function(dll) is.loaded("_rcpp_module_boot_moduleInitRetry", PACKAGE = dll[["name"]]) +dll <- Filter(is_retry_module, getLoadedDLLs())[[1L]] +mod <- Module( "moduleInitRetry", PACKAGE = dll, mustStart = TRUE ) +expect_equal( length( mod$ModuleInitRetry@constructors ), 1L ) +expect_equal( length( mod$ModuleInitRetry@methods$value@nargs ), 1L ) +expect_equal( mod$retry_add_one( 41L ), 42L ) +expect_equal( new( mod$ModuleInitRetry )$value(), 42L ) + # test.Module.flexible.semantics <- function( ){ expect_equal( test_reference( seq(0,10) ), 11L ) expect_equal( test_const_reference( seq(0,10) ), 11L )