From 473cdfc572e41d0495506a67148f464f64eed458 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Wed, 29 Apr 2026 10:43:55 -0700 Subject: [PATCH] fix memory leak in Rcpp::warning under tryCatch handlers The variadic Rcpp::warning() template formatted its message into a std::string whose destructor would be skipped if Rf_warning() longjmp-ed (e.g. via tryCatch(warning=...)), leaking the heap buffer. Copy the formatted message into a stack buffer and let the std::string be destroyed before Rf_warning() is called. Closes #1474. --- inst/NEWS.Rd | 5 +++++ inst/include/Rcpp/exceptions.h | 13 ++++++++++++- inst/tinytest/test_dataframe.R | 10 ++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/inst/NEWS.Rd b/inst/NEWS.Rd index 83e557f3f..aae453f48 100644 --- a/inst/NEWS.Rd +++ b/inst/NEWS.Rd @@ -32,6 +32,11 @@ with current R versions (Dirk in \ghpr{1469} fixing \ghit{1468}) \item The \code{Nullable::as()} exporter now uses an explicit cast to the templated type (Dirk in \ghpr{1471} fixing \ghit{1470}) + \item A memory leak in the variadic \code{Rcpp::warning()} template + has been fixed by copying the formatted message into a stack buffer + before \code{Rf_warning()} is invoked, so the \code{std::string} + destructor runs even when an R warning handler triggers a + \code{longjmp} (Kevin fixing \ghit{1474}) } \item Changes in Rcpp Documentation: \itemize{ diff --git a/inst/include/Rcpp/exceptions.h b/inst/include/Rcpp/exceptions.h index 2311442e3..1259b6d20 100644 --- a/inst/include/Rcpp/exceptions.h +++ b/inst/include/Rcpp/exceptions.h @@ -24,6 +24,7 @@ #define Rcpp__exceptions__h #include +#include #ifndef RCPP_DEFAULT_INCLUDE_CALL #define RCPP_DEFAULT_INCLUDE_CALL true @@ -186,7 +187,17 @@ struct LongjumpException { template inline void warning(const char* fmt, Args&&... args ) { - Rf_warning("%s", tfm::format(fmt, std::forward(args)... ).c_str()); + // Rf_warning() may longjmp out of this frame (e.g. when the caller + // installs a warning handler via tryCatch(warning=...)). A longjmp + // skips C++ destructors, so the std::string returned by tfm::format() + // would leak its heap buffer. Copy into a stack buffer and let the + // std::string be destroyed before Rf_warning() is invoked. + char buf[8192]; + { + const std::string msg = tfm::format(fmt, std::forward(args)...); + std::snprintf(buf, sizeof(buf), "%s", msg.c_str()); + } + Rf_warning("%s", buf); } // #nocov end template diff --git a/inst/tinytest/test_dataframe.R b/inst/tinytest/test_dataframe.R index f9571cb1e..d87007d7e 100644 --- a/inst/tinytest/test_dataframe.R +++ b/inst/tinytest/test_dataframe.R @@ -117,3 +117,13 @@ expect_warning( DataFrame_PushZeroLength()) ## issue #1232: push on empty data.frame df <- DataFrame_PushOnEmpty() expect_equal(ncol(df), 3L) + +## issue #1474: warning thrown from DataFrame::push_back used to leak the +## formatted std::string when a tryCatch handler longjmp-ed past its dtor. +## Loop a few iterations so valgrind makes any regression obvious. +got_warning <- FALSE +for (i in 1:5) { + tryCatch(DataFrame_PushWrongSize(), + warning = function(w) { got_warning <<- TRUE }) +} +expect_true(got_warning)