Merge pull request #1753 from rstudio/bugfix/windows-interrupts

Fix interrupts on Windows in RStudio

R/package.R

@@ -87,6 +87,11 @@ ensure_python_initialized <- function(required_module = NULL) {
   if (is.function(callback))
     callback()
 
+  # re-install interrupt handler -- note that RStudio tries to re-install its
+  # own interrupt handler when reticulate is initialized, but reticulate needs
+  # to handle interrupts itself (and it can do so compatibly with RStudio)
+  install_interrupt_handlers()
+
   # call init hooks
   call_init_hooks()
 

src/libpython.cpp

@@ -3,12 +3,16 @@
 #include "libpython.h"
 
 #ifndef _WIN32
-#include <dlfcn.h>
+# include <dlfcn.h>
 #else
-#define WIN32_LEAN_AND_MEAN 1
-#include <windows.h>
+# define WIN32_LEAN_AND_MEAN 1
+# include <windows.h>
 #endif
 
+#include <R.h>
+#include <Rinternals.h>
+
+
 #include <string>
 #include <vector>
 #include <iostream>
@@ -453,39 +457,54 @@ bool import_numpy_api(bool python3, std::string* pError) {
   return true;
 }
 
+// returns 'true' if the buffer was flushed, or if the stdout / stderr
+// objects within 'sys' did not contain 'flush' methods
+bool flush_std_buffer(const char* name) {
 
-int flush_std_buffers() {
-  int status = 0;
-  PyObject* tmp = NULL;
-  PyObject *error_type, *error_value, *error_traceback;
-  PyErr_Fetch(&error_type, &error_value, &error_traceback);
+  // returns borrowed reference
+  PyObject* buffer(PySys_GetObject(name));
+  if (buffer == NULL || buffer == Py_None)
+    return true;
 
-  PyObject* sys_stdout(PySys_GetObject("stdout"));  // returns borrowed reference
-  if (sys_stdout == NULL)
-    status = -1;
-  else
-    tmp = PyObject_CallMethod(sys_stdout, "flush", NULL);
+  // try to invoke flush method
+  PyObject* result = PyObject_CallMethod(buffer, "flush", NULL);
+  if (result != NULL) {
+    Py_DecRef(result);
+    return true;
+  }
 
-  if (tmp == NULL)
-    status = -1;
-  else {
-    Py_DecRef(tmp);
-    tmp = NULL;
+  // if we got here, an error must have occurred; print it
+  PyObject *ptype, *pvalue, *ptraceback;
+  PyErr_Fetch(&ptype, &pvalue, &ptraceback);
+  PyErr_NormalizeException(&ptype, &pvalue, &ptraceback);
+  if (pvalue) {
+    PyObject* pvalue_str = PyObject_Str(pvalue);
+    if (pvalue_str) {
+      REprintf("Error flushing Python %s: %s\n", name, PyUnicode_AsUTF8(pvalue_str));
+      Py_DecRef(pvalue_str);
+    }
   }
 
-  PyObject* sys_stderr(PySys_GetObject("stderr"));  // returns borrowed reference
-  if (sys_stderr == NULL)
-    status = -1;
-  else
-    tmp = PyObject_CallMethod(sys_stderr, "flush", NULL);
+  // clean up
+  if (ptype)      Py_DecRef(ptype);
+  if (pvalue)     Py_DecRef(pvalue);
+  if (ptraceback) Py_DecRef(ptraceback);
 
-  if (tmp == NULL)
-    status = -1;
-  else
-    Py_DecRef(tmp);
+  return false;
+
+}
+
+int flush_std_buffers() {
 
+  PyObject *error_type, *error_value, *error_traceback;
+  PyErr_Fetch(&error_type, &error_value, &error_traceback);
+  bool stdout_ok = flush_std_buffer("stdout");
+  bool stderr_ok = flush_std_buffer("stderr");
+  bool ok = stdout_ok && stderr_ok;
   PyErr_Restore(error_type, error_value, error_traceback);
-  return status;
+
+  return ok ? 0 : -1;
+
 }
 
 

src/python.cpp

@@ -2797,10 +2797,34 @@ extern "C" PyObject* schedule_python_function_on_main_thread(
   return Py_None;
 }
 
+#ifdef _WIN32
+
+static void (*s_interrupt_handler)(int) = nullptr;
+
+static int win32_interrupt_handler(long unsigned int ignored) {
+  if (s_interrupt_handler != nullptr) {
+    s_interrupt_handler(SIGINT);
+  }
+  return TRUE;
+}
+
+#endif
+
+static PyOS_sighandler_t reticulate_setsig(int signum, PyOS_sighandler_t handler) {
+
+#ifdef _WIN32
+  s_interrupt_handler = handler;
+  SetConsoleCtrlHandler(NULL, FALSE);
+  SetConsoleCtrlHandler(win32_interrupt_handler, FALSE);
+  SetConsoleCtrlHandler(win32_interrupt_handler, TRUE);
+#endif
+
+  return PyOS_setsig(signum, handler);
+
+}
 
-static void
-interrupt_handler(int signum) {
-  // This handler is called by the OS when signaling a SIGINT
+
+static void interrupt_handler(int signum) {
 
   // Tell R that an interrupt is pending. This will cause R to signal an
   // "interrupt" R condition next time R_CheckUserInterrupt() is called
@@ -2819,7 +2843,8 @@ interrupt_handler(int signum) {
   // i.e., if R_CheckUserInterrupt() or PyCheckSignals(), is called first.
 
   // Reinstall this C handler, as it may have been cleared when invoked by the OS
-  PyOS_setsig(signum, interrupt_handler);
+  reticulate_setsig(signum, interrupt_handler);
+
 }
 
 
@@ -2853,7 +2878,7 @@ PyOS_sighandler_t install_interrupt_handlers_() {
   //
   // This *must* be after setting the Python handler, because signal.signal()
   // will also reset the OS C handler to one that is not aware of R.
-  return PyOS_setsig(SIGINT, interrupt_handler);
+  return reticulate_setsig(SIGINT, interrupt_handler);
 }
 
 // [[Rcpp::export]]
@@ -3173,7 +3198,7 @@ void py_initialize(const std::string& python,
     PySys_SetArgv(1, const_cast<char**>(argv));
 
     orig_interrupt_handler = install_interrupt_handlers_();
-    PyOS_setsig(SIGINT, interrupt_handler);
+    reticulate_setsig(SIGINT, interrupt_handler);
   }
 
   s_main_thread = tthread::this_thread::get_id();
@@ -3233,7 +3258,7 @@ void py_finalize() {
     PyGILState_Ensure();
     Py_MakePendingCalls();
     if (orig_interrupt_handler)
-      PyOS_setsig(SIGINT, orig_interrupt_handler);
+      reticulate_setsig(SIGINT, orig_interrupt_handler);
     is_py_finalized = true;
     Py_Finalize();
   }

tests/testthat/test-interrupts.R

@@ -83,6 +83,7 @@ test_that("interrupts can be caught by Python", {
   skip_on_cran()
 
   p <- callr::r_bg(args = list(python = py_exe()), function(python) {
+
     Sys.setenv(RETICULATE_PYTHON = python)
     library(reticulate)
     get_frames <- function() {
@@ -112,6 +113,7 @@ test_that("interrupts can be caught by Python", {
     stopifnot(identical(frames_before, frames_after))
 
     cat("R Finished!")
+
   })
 
   p$poll_io(5000)
@@ -127,9 +129,9 @@ test_that("interrupts can be caught by Python", {
   p$wait()
 
   expect_identical(p$get_exit_status(), 0L)
-  expect_identical(
-    p$read_all_output(),
-    "Caught interrupt; Running finally; Python finished; R Finished!")
+  output <- p$read_all_output()
+  expected <- "Caught interrupt; Running finally; Python finished; R Finished!"
+  expect_identical(output, expected)
 
 })