Make Rcpp::exception thread-safe

jpritikin · jpritikin · commit 2815ae87b86d · 2020-02-09T10:54:32.000-05:00
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,11 @@
+2020-02-09  Joshua Pritikin  <jpritikin@pobox.com>
+
+	* inst/include/Rcpp.h: Include exceptions_impl.h
+	* inst/include/Rcpp/exceptions.h: Make thread-safe
+	* inst/include/Rcpp/exceptions_impl.h: New home for code moved
+	from inst/include/Rcpp/routines.h and src/api.cpp
+	* src/rcpp_init.cpp: stack_trace no longer exported as an R API
+
 2020-01-05  Dirk Eddelbuettel  <edd@debian.org>
 
 	* inst/include/Rcpp/exceptions.h: A few more #nocov tags
diff --git a/inst/NEWS.Rd b/inst/NEWS.Rd
@@ -7,14 +7,15 @@
   \itemize{
     \item Changes in Rcpp API:
     \itemize{
+      \item \code{Rcpp::exception} is now thread-safe
       \item Safer \code{Rcpp_list*}, \code{Rcpp_lang*} and
       \code{Function.operator()} (Romain in \ghpr{1014}, \ghit{1015}).
       \item A number of \code{#nocov} markers were added (Dirk in
       \ghpr{1036} and \ghpr{1042}).
       \item Finalizer calls clear external pointer first (Kirill Müller and
       Dirk in \ghpr{1038}).
       \item Scalar operations with a rhs matrix no longer change the
-      matrix value (Qiang in \ghpr{1040} fixing (again) \ghit{365}). 
+      matrix value (Qiang in \ghpr{1040} fixing (again) \ghit{365}).
     }
     \item Changes in Rcpp Attributes:
     \itemize{
diff --git a/inst/include/Rcpp.h b/inst/include/Rcpp.h
@@ -56,6 +56,8 @@
 #include <Rcpp/Formula.h>
 #include <Rcpp/DataFrame.h>
 
+#include <Rcpp/exceptions_impl.h>
+
 #if !defined(RCPP_FORCE_OLD_DATE_DATETIME_VECTORS)
   #define RCPP_NEW_DATE_DATETIME_VECTORS 1
 #endif
diff --git a/inst/include/Rcpp/exceptions.h b/inst/include/Rcpp/exceptions.h
@@ -27,21 +27,22 @@
 #define RCPP_DEFAULT_INCLUDE_CALL true
 #endif
 
-#define GET_STACKTRACE() stack_trace( __FILE__, __LINE__ )
-
 namespace Rcpp {
 
+    // Throwing an exception must be thread-safe to avoid surprises w/ OpenMP.
     class exception : public std::exception {
     public:
         explicit exception(const char* message_, bool include_call = RCPP_DEFAULT_INCLUDE_CALL) :	// #nocov start
             message(message_),
-            include_call_(include_call){
-            rcpp_set_stack_trace(Shield<SEXP>(stack_trace()));
+            include_call_(include_call)
+        {
+            record_stack_trace();
         }
         exception(const char* message_, const char*, int, bool include_call = RCPP_DEFAULT_INCLUDE_CALL) :
             message(message_),
-            include_call_(include_call){
-            rcpp_set_stack_trace(Shield<SEXP>(stack_trace()));
+            include_call_(include_call)
+        {
+            record_stack_trace();
         }
         bool include_call() const {
             return include_call_;
@@ -50,9 +51,12 @@ namespace Rcpp {
         virtual const char* what() const throw() {
             return message.c_str();					// #nocov end
         }
+        inline void copy_stack_trace_to_r() const;
     private:
         std::string message;
         bool include_call_;
+        std::vector<std::string> stack;
+        inline void record_stack_trace();
     };
 
     // simple helper
@@ -340,7 +344,8 @@ inline SEXP exception_to_condition_template( const Exception& ex, bool include_c
 }
 
 inline SEXP rcpp_exception_to_r_condition(const Rcpp::exception& ex) {
-  return exception_to_condition_template(ex, ex.include_call());
+    ex.copy_stack_trace_to_r();
+    return exception_to_condition_template(ex, ex.include_call());
 }
 
 inline SEXP exception_to_r_condition( const std::exception& ex){
diff --git a/inst/include/Rcpp/exceptions_impl.h b/inst/include/Rcpp/exceptions_impl.h
@@ -0,0 +1,69 @@
+namespace Rcpp {
+#if defined(__GNUC__) || defined(__clang__)
+    #if defined(_WIN32) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__CYGWIN__) || defined(__sun) || defined(_AIX) || defined(__MUSL__) || defined(__HAIKU__) || defined(__ANDROID__)
+        // do nothing
+    #else
+    #include <execinfo.h>
+
+    // Extract mangled name e.g. ./test(baz+0x14)[0x400962]
+    static std::string demangler_one(const char* input) {
+        static std::string buffer;
+        buffer = input;
+        size_t last_open = buffer.find_last_of('(');
+        size_t last_close = buffer.find_last_of(')');
+        if (last_open == std::string::npos ||
+            last_close == std::string::npos) {
+            return input;     // #nocov
+        }
+        std::string function_name = buffer.substr(last_open + 1, last_close - last_open - 1);
+        // Strip the +0x14 (if it exists, which it does not in earlier versions of gcc)
+        size_t function_plus = function_name.find_last_of('+');
+        if (function_plus != std::string::npos) {
+            function_name.resize(function_plus);
+        }
+        buffer.replace(last_open + 1, function_name.size(), demangle(function_name));
+        return buffer;
+    }
+    #endif
+#endif
+
+    // thread-safe; invoked prior to throwing the exception
+    inline void exception::record_stack_trace()
+    {
+    #if defined(__GNUC__) || defined(__clang__)
+        #if defined(_WIN32) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__CYGWIN__) || defined(__sun) || defined(_AIX) || defined(__MUSL__) || defined(__HAIKU__) || defined(__ANDROID__)
+            // C++ stack not available on this system
+        #else // ! (defined(_WIN32) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__CYGWIN__) || defined(__sun) || defined(_AIX) || defined(__ANDROID__)
+
+        /* inspired from http://tombarta.wordpress.com/2008/08/01/c-stack-traces-with-gcc/  */
+        const size_t max_depth = 100;
+        int stack_depth;
+        void *stack_addrs[max_depth];
+
+        stack_depth = backtrace(stack_addrs, max_depth);
+        char **stack_strings = backtrace_symbols(stack_addrs, stack_depth);
+
+        std::transform(stack_strings + 1, stack_strings + stack_depth,
+                       std::back_inserter(stack), demangler_one);
+        free(stack_strings); // malloc()ed by backtrace_symbols
+        #endif
+    #endif
+    }
+
+    // not thread-safe; invoked after catching the exception
+    inline void exception::copy_stack_trace_to_r() const
+    {
+        if (!stack.size()) {
+            rcpp_set_stack_trace(R_NilValue);
+            return;
+        }
+
+        CharacterVector res(stack.size());
+        std::copy(stack.begin(), stack.end(), res.begin());
+        List trace = List::create(_["file" ] = "",
+                                  _["line" ] = -1,
+                                  _["stack"] = res);
+        trace.attr("class") = "Rcpp_stack_trace";
+        rcpp_set_stack_trace(trace);
+    }
+};
diff --git a/inst/include/Rcpp/routines.h b/inst/include/Rcpp/routines.h
@@ -45,7 +45,6 @@ SEXP          rcpp_set_stack_trace(SEXP);
 std::string   demangle(const std::string& name);
 const char*   short_file_name(const char* );
 int*          get_cache(int n);
-SEXP          stack_trace( const char *file = "", int line = -1);
 SEXP          get_string_elt(SEXP s, R_xlen_t i);
 const char*   char_get_string_elt(SEXP s, R_xlen_t i);
 void          set_string_elt(SEXP s, R_xlen_t i, SEXP v);
@@ -157,12 +156,6 @@ inline attribute_hidden const char* short_file_name(const char* file) {
     return fun(file);
 }
 
-inline attribute_hidden SEXP stack_trace( const char *file = "", int line = -1){
-    typedef SEXP (*Fun)(const char*, int);
-    static Fun fun = GET_CALLABLE("stack_trace");
-    return fun(file, line);
-}
-
 inline attribute_hidden SEXP get_string_elt(SEXP s, R_xlen_t i){
     typedef SEXP (*Fun)(SEXP, R_xlen_t);
     static Fun fun = GET_CALLABLE("get_string_elt");
diff --git a/src/api.cpp b/src/api.cpp
@@ -32,35 +32,6 @@ using namespace Rcpp;
 #include <cxxabi.h>
 #endif
 
-#if defined(__GNUC__) || defined(__clang__)
-    #if defined(_WIN32) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__CYGWIN__) || defined(__sun) || defined(_AIX) || defined(__MUSL__) || defined(__HAIKU__) || defined(__ANDROID__)
-        // do nothing
-    #else
-        #include <execinfo.h>
-
-        // Extract mangled name e.g. ./test(baz+0x14)[0x400962]
-        static std::string demangler_one(const char* input) {
-            static std::string buffer;
-            buffer = input;
-            size_t last_open = buffer.find_last_of('(');
-            size_t last_close = buffer.find_last_of(')');
-            if (last_open == std::string::npos ||
-                last_close == std::string::npos) {
-              return input;     // #nocov
-            }
-            std::string function_name = buffer.substr(last_open + 1, last_close - last_open - 1);
-            // Strip the +0x14 (if it exists, which it does not in earlier versions of gcc)
-            size_t function_plus = function_name.find_last_of('+');
-            if (function_plus != std::string::npos) {
-              function_name.resize(function_plus);
-            }
-            buffer.replace(last_open + 1, function_name.size(), demangle(function_name));
-            return buffer;
-        }
-    #endif
-#endif
-
-
 namespace Rcpp {
 
     namespace internal {
@@ -280,44 +251,6 @@ SEXP rcpp_can_use_cxx11() {				// #nocov start
 }							// #nocov end
 
 
-// [[Rcpp::register]]
-SEXP stack_trace(const char* file, int line) {
-    #if defined(__GNUC__) || defined(__clang__)
-        #if defined(_WIN32) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__CYGWIN__) || defined(__sun) || defined(_AIX) || defined(__MUSL__) || defined(__HAIKU__) || defined(__ANDROID__)
-            // Simpler version for Windows and *BSD
-            List trace = List::create(_["file"] = file,
-                                      _[ "line"  ] = line,
-                                      _[ "stack" ] = "C++ stack not available on this system");
-            trace.attr("class") = "Rcpp_stack_trace";
-            return trace;
-        #else // ! (defined(_WIN32) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__CYGWIN__) || defined(__sun) || defined(_AIX) || defined(__ANDROID__)
-
-            /* inspired from http://tombarta.wordpress.com/2008/08/01/c-stack-traces-with-gcc/  */
-            const size_t max_depth = 100;
-            int stack_depth;
-            void *stack_addrs[max_depth];
-            char **stack_strings;
-
-            stack_depth = backtrace(stack_addrs, max_depth);
-            stack_strings = backtrace_symbols(stack_addrs, stack_depth);
-
-            std::string current_line;
-
-            CharacterVector res(stack_depth - 1);
-            std::transform(stack_strings + 1, stack_strings + stack_depth, res.begin(), demangler_one);
-            free(stack_strings); // malloc()ed by backtrace_symbols
-
-            List trace = List::create(_["file" ] = file,
-                                      _["line" ] = line,
-                                      _["stack"] = res);
-            trace.attr("class") = "Rcpp_stack_trace";
-            return trace;
-        #endif
-    #else /* !defined( __GNUC__ ) */
-        return R_NilValue;
-    #endif
-}
-
 // // [ [ Rcpp::register ] ]
 // void print(SEXP s) {
 //     Rf_PrintValue(s);           // defined in Rinternals.h
diff --git a/src/rcpp_init.cpp b/src/rcpp_init.cpp
@@ -100,7 +100,6 @@ void registerFunctions(){
     RCPP_REGISTER(endSuspendRNGSynchronization);
     RCPP_REGISTER(get_Rcpp_namespace)
     RCPP_REGISTER(get_cache)
-    RCPP_REGISTER(stack_trace)
     RCPP_REGISTER(get_string_elt)
     RCPP_REGISTER(char_get_string_elt)
     RCPP_REGISTER(set_string_elt)
