Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

potential fix for windows interrupts #1753

Merged
merged 7 commits into from
Feb 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions R/package.R
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
75 changes: 47 additions & 28 deletions src/libpython.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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;

}


Expand Down
39 changes: 32 additions & 7 deletions src/python.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);

}


Expand Down Expand Up @@ -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]]
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}
Expand Down
8 changes: 5 additions & 3 deletions tests/testthat/test-interrupts.R
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
Expand All @@ -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)

})

Expand Down
Loading