Merge pull request #1602 from rstudio/fix-interrupt

Fix segfaults after interrupt.

NEWS.md

@@ -1,5 +1,7 @@
 # reticulate (development version)
 
+- Interrupting Python no longer leads to segfaults (#1601, fixed in #1602).
+
 - `virtualenv_starter()` no longer warns when encountering broken symlinks (#1598).
 
 # reticulate 1.36.1

R/RcppExports.R

@@ -86,6 +86,10 @@ r_to_py_impl <- function(object, convert) {
     .Call(`_reticulate_r_to_py_impl`, object, convert)
 }
 
+install_interrupt_handlers <- function() {
+    invisible(.Call(`_reticulate_install_interrupt_handlers`))
+}
+
 py_activate_virtualenv <- function(script) {
     invisible(.Call(`_reticulate_py_activate_virtualenv`, script))
 }
@@ -307,10 +311,6 @@ r_convert_date <- function(dates, convert) {
     .Call(`_reticulate_r_convert_date`, dates, convert)
 }
 
-py_set_interrupt_impl <- function() {
-    invisible(.Call(`_reticulate_py_set_interrupt_impl`))
-}
-
 py_list_length <- function(x) {
     .Call(`_reticulate_py_list_length`, x)
 }
@@ -369,11 +369,3 @@ readline <- function(prompt) {
     .Call(`_reticulate_readline`, prompt)
 }
 
-py_register_interrupt_handler <- function() {
-    invisible(.Call(`_reticulate_py_register_interrupt_handler`))
-}
-
-py_interrupts_pending <- function(reset) {
-    .Call(`_reticulate_py_interrupts_pending`, reset)
-}
-

R/package.R

@@ -86,13 +86,6 @@ ensure_python_initialized <- function(required_module = NULL) {
   if (is.function(callback))
     callback()
 
-  # set up a Python signal handler
-  signals <- import("rpytools.signals")
-  signals$initialize(py_interrupts_pending)
-
-  # register C-level interrupt handler
-  py_register_interrupt_handler()
-
   # call init hooks
   call_init_hooks()
 

R/python.R

@@ -1378,16 +1378,7 @@ py_inject_hooks <- function() {
 
   input <- function(prompt = "") {
 
-    response <- tryCatch(
-      readline(prompt),
-      interrupt = identity
-    )
-
-    if (inherits(response, "interrupt"))
-      stop("KeyboardInterrupt", call. = FALSE)
-
-    r_to_py(response)
-
+    readline(prompt)
   }
 
   # override input function
@@ -1487,9 +1478,6 @@ chooseOpsMethod.python.builtin.object <- function(x, y, mx, my, cl, reverse) {
   identical(environment(my), parent.env(environment()))
 }
 
-py_set_interrupt <- function() {
-  py_set_interrupt_impl()
-}
 
 #' @export
 format.python.builtin.traceback <- function(x, ..., limit = NULL) {

src/RcppExports.cpp

@@ -161,6 +161,15 @@ BEGIN_RCPP
     return rcpp_result_gen;
 END_RCPP
 }
+// install_interrupt_handlers
+void install_interrupt_handlers();
+RcppExport SEXP _reticulate_install_interrupt_handlers() {
+BEGIN_RCPP
+    Rcpp::RNGScope rcpp_rngScope_gen;
+    install_interrupt_handlers();
+    return R_NilValue;
+END_RCPP
+}
 // py_activate_virtualenv
 void py_activate_virtualenv(const std::string& script);
 RcppExport SEXP _reticulate_py_activate_virtualenv(SEXP scriptSEXP) {
@@ -685,15 +694,6 @@ BEGIN_RCPP
     return rcpp_result_gen;
 END_RCPP
 }
-// py_set_interrupt_impl
-void py_set_interrupt_impl();
-RcppExport SEXP _reticulate_py_set_interrupt_impl() {
-BEGIN_RCPP
-    Rcpp::RNGScope rcpp_rngScope_gen;
-    py_set_interrupt_impl();
-    return R_NilValue;
-END_RCPP
-}
 // py_list_length
 SEXP py_list_length(PyObjectRef x);
 RcppExport SEXP _reticulate_py_list_length(SEXP xSEXP) {
@@ -823,26 +823,6 @@ BEGIN_RCPP
     return rcpp_result_gen;
 END_RCPP
 }
-// py_register_interrupt_handler
-void py_register_interrupt_handler();
-RcppExport SEXP _reticulate_py_register_interrupt_handler() {
-BEGIN_RCPP
-    Rcpp::RNGScope rcpp_rngScope_gen;
-    py_register_interrupt_handler();
-    return R_NilValue;
-END_RCPP
-}
-// py_interrupts_pending
-bool py_interrupts_pending(bool reset);
-RcppExport SEXP _reticulate_py_interrupts_pending(SEXP resetSEXP) {
-BEGIN_RCPP
-    Rcpp::RObject rcpp_result_gen;
-    Rcpp::RNGScope rcpp_rngScope_gen;
-    Rcpp::traits::input_parameter< bool >::type reset(resetSEXP);
-    rcpp_result_gen = Rcpp::wrap(py_interrupts_pending(reset));
-    return rcpp_result_gen;
-END_RCPP
-}
 
 static const R_CallMethodDef CallEntries[] = {
     {"_reticulate_write_stdout", (DL_FUNC) &_reticulate_write_stdout, 1},
@@ -859,6 +839,7 @@ static const R_CallMethodDef CallEntries[] = {
     {"_reticulate_py_to_r_cpp", (DL_FUNC) &_reticulate_py_to_r_cpp, 1},
     {"_reticulate_py_get_formals", (DL_FUNC) &_reticulate_py_get_formals, 1},
     {"_reticulate_r_to_py_impl", (DL_FUNC) &_reticulate_r_to_py_impl, 2},
+    {"_reticulate_install_interrupt_handlers", (DL_FUNC) &_reticulate_install_interrupt_handlers, 0},
     {"_reticulate_py_activate_virtualenv", (DL_FUNC) &_reticulate_py_activate_virtualenv, 1},
     {"_reticulate_main_process_python_info", (DL_FUNC) &_reticulate_main_process_python_info, 0},
     {"_reticulate_py_clear_error", (DL_FUNC) &_reticulate_py_clear_error, 0},
@@ -904,7 +885,6 @@ static const R_CallMethodDef CallEntries[] = {
     {"_reticulate_py_convert_pandas_df", (DL_FUNC) &_reticulate_py_convert_pandas_df, 1},
     {"_reticulate_r_convert_dataframe", (DL_FUNC) &_reticulate_r_convert_dataframe, 2},
     {"_reticulate_r_convert_date", (DL_FUNC) &_reticulate_r_convert_date, 2},
-    {"_reticulate_py_set_interrupt_impl", (DL_FUNC) &_reticulate_py_set_interrupt_impl, 0},
     {"_reticulate_py_list_length", (DL_FUNC) &_reticulate_py_list_length, 1},
     {"_reticulate_py_len_impl", (DL_FUNC) &_reticulate_py_len_impl, 2},
     {"_reticulate_py_bool_impl", (DL_FUNC) &_reticulate_py_bool_impl, 2},
@@ -916,8 +896,6 @@ static const R_CallMethodDef CallEntries[] = {
     {"_reticulate_py_iter_next", (DL_FUNC) &_reticulate_py_iter_next, 2},
     {"_reticulate_py_iterate", (DL_FUNC) &_reticulate_py_iterate, 3},
     {"_reticulate_readline", (DL_FUNC) &_reticulate_readline, 1},
-    {"_reticulate_py_register_interrupt_handler", (DL_FUNC) &_reticulate_py_register_interrupt_handler, 0},
-    {"_reticulate_py_interrupts_pending", (DL_FUNC) &_reticulate_py_interrupts_pending, 1},
     {NULL, NULL, 0}
 };
 

src/event_loop.cpp

@@ -104,6 +104,22 @@ int pollForEvents(void*) {
     R_ToplevelExec(processEvents, NULL);
   }
 
+  // Check if we need to set a python interrupt.
+  // This is a fallback in case:
+  //
+  // 1. User code overwrote the C handler reticulate registered and we received
+  // a SIGINT. This can easily happen if python code called signal.signal()
+  //
+  // 2. An R interrupt was only simulated by calling R_onintr() directly (e.g,
+  // rlang::interrupt()), and not actually received as an OS process signal.
+  //
+  // As a fail-safe for the C handler not being called, we also ensure here, in
+  // the background polling worker, that if R_interrupts_pending=1, then a Python
+  // interrupt is pending too.
+  if(reticulate::signals::getInterruptsPending()) {
+    PyErr_SetInterrupt();
+  }
+
   // Success!
   return 0;
 

src/libpython.cpp

@@ -154,6 +154,16 @@ void initialize_type_objects(bool python3) {
   Py_Complex = PyComplex_FromDoubles(0.0, 0.0);
   Py_ByteArray = PyByteArray_FromStringAndSize("a", 1);
   Py_DictClass = PyObject_Type(Py_Dict);
+
+  PyObject* builtins = PyImport_AddModule(python3 ? "builtins" : "__builtin__"); // borrowed ref
+  if (builtins == NULL) goto error;
+  PyExc_KeyboardInterrupt = PyObject_GetAttrString(builtins, "KeyboardInterrupt"); // new ref
+
+  if (PyErr_Occurred()) { error:
+     // Should never happen. If you see this please report a bug
+     printf("initialize_type_objects() raised a Python exception.\n");
+     PyErr_Print();
+  }
 }
 
 #define LOAD_PYTHON_SYMBOL_AS(name, as)             \
@@ -182,14 +192,12 @@ bool LibPython::loadSymbols(bool python3, std::string* pError)
 {
   bool is64bit = sizeof(size_t) >= 8;
 
-  LOAD_PYTHON_SYMBOL(Py_Initialize)
+  LOAD_PYTHON_SYMBOL(Py_InitializeEx)
   LOAD_PYTHON_SYMBOL(Py_IsInitialized)
   LOAD_PYTHON_SYMBOL(Py_GetVersion)
   LOAD_PYTHON_SYMBOL(Py_AddPendingCall)
   LOAD_PYTHON_SYMBOL(PyErr_SetInterrupt)
   LOAD_PYTHON_SYMBOL(PyErr_CheckSignals)
-  LOAD_PYTHON_SYMBOL(PyExc_KeyboardInterrupt)
-  LOAD_PYTHON_SYMBOL(PyExc_ValueError)
   LOAD_PYTHON_SYMBOL(Py_IncRef)
   LOAD_PYTHON_SYMBOL(Py_DecRef)
   LOAD_PYTHON_SYMBOL(PyObject_Size)
@@ -274,6 +282,7 @@ bool LibPython::loadSymbols(bool python3, std::string* pError)
   LOAD_PYTHON_SYMBOL(PyType_IsSubtype)
   LOAD_PYTHON_SYMBOL(PyType_GetFlags)
   LOAD_PYTHON_SYMBOL(PyMapping_Items)
+  LOAD_PYTHON_SYMBOL(PyOS_setsig)
   LOAD_PYTHON_SYMBOL(PySys_WriteStderr)
   LOAD_PYTHON_SYMBOL(PySys_GetObject)
   LOAD_PYTHON_SYMBOL(PyEval_SetProfile)

src/libpython.h

@@ -95,6 +95,7 @@ typedef int (*inquiry)(PyObject *);
 typedef int (*visitproc)(PyObject *, void *);
 typedef int (*traverseproc)(PyObject *, visitproc, void *);
 typedef void (*freefunc)(void *);
+typedef void (*PyOS_sighandler_t)(int);
 
 typedef struct PyModuleDef_Base {
   PyObject_HEAD3
@@ -193,7 +194,7 @@ void initialize_type_objects(bool python3);
 #define PyFunction_Check(op) ((PyTypeObject*)(Py_TYPE(op)) == PyFunction_Type)
 #define PyMethod_Check(op)   ((PyTypeObject *)(Py_TYPE(op)) == PyMethod_Type)
 
-LIBPYTHON_EXTERN void (*Py_Initialize)();
+LIBPYTHON_EXTERN void (*Py_InitializeEx)(int);
 LIBPYTHON_EXTERN int (*Py_IsInitialized)();
 LIBPYTHON_EXTERN const char* (*Py_GetVersion)();
 LIBPYTHON_EXTERN char* (*Py_GetProgramFullPath_v2)();
@@ -375,6 +376,8 @@ LIBPYTHON_EXTERN void (*Py_SetProgramName_v3)(wchar_t *);
 LIBPYTHON_EXTERN void (*Py_SetPythonHome)(char *);
 LIBPYTHON_EXTERN void (*Py_SetPythonHome_v3)(wchar_t *);
 
+LIBPYTHON_EXTERN PyOS_sighandler_t (*PyOS_setsig)(int i, PyOS_sighandler_t h);
+
 LIBPYTHON_EXTERN void (*PySys_SetArgv)(int, char **);
 LIBPYTHON_EXTERN void (*PySys_SetArgv_v3)(int, wchar_t **);