run the module body only once per dll (#1528)

* turn exceptions during module initialization into r errors

* run the module body only once per dll

* Minor ChangeLog cleanup

* 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.

---------

Co-authored-by: Dirk Eddelbuettel <edd@debian.org>

Author: kevinushey

ChangeLog

@@ -41,6 +41,20 @@
 	* inst/tinytest/cpp/Module.cpp: Add a class deriving from another
 	* inst/tinytest/test_module.R: Test the new checks
 
+	* 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/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
 	module initializes raises an R error instead of terminating the session

inst/include/Rcpp/Module.h

@@ -637,17 +637,29 @@ 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>( & _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, 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 ) ;                               \
+    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 ) ; \
+            try {                                                                     \
+                _rcpp_module_##name##_init( ) ;                                       \
+            } catch( ... ) {                                                          \
+                _rcpp_module_##name.clear() ;                                         \
+                throw ;                                                               \
+            }                                                                         \
+            _rcpp_module_##name##_initialized = true ;                                \
+        }                                                                             \
+        return Rcpp::XPtr<Rcpp::Module>( & _rcpp_module_##name , false ) ;           \
+        END_RCPP                                                                      \
+    }                                                                                 \
     void _rcpp_module_##name##_init()
 
 // silly little dance to suppress a 'defined but not used variable' warning

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() ;
         }

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 <http://www.gnu.org/licenses/>.
+
+#include <Rcpp.h>
+
+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>("ModuleInitRetry")
+        .constructor()
+        .method("value", &ModuleInitRetry::value)
+        ;
+    function("retry_add_one", &retry_add_one);
+
+    if (++boot_count == 1)
+        throw std::runtime_error("first boot fails");
+}

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 )
@@ -189,3 +201,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 )