Skip to content

Commit

Permalink
implement a meaningful py_finalize()
Browse files Browse the repository at this point in the history
  • Loading branch information
t-kalinowski committed Aug 22, 2024
1 parent 289212c commit d61a54d
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 22 deletions.
8 changes: 8 additions & 0 deletions R/package.R
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,19 @@ is_python_initialized <- function() {
!is.null(.globals$py_config)
}

is_python_finalized <- function() {
identical(.globals$finalized, TRUE)
}

ensure_python_initialized <- function(required_module = NULL) {

# nothing to do if python is initialized
if (is_python_initialized())
return()

if (is_python_finalized())
stop("py_initialize() cannot be called more than once per R session or after py_finalize(). Please start a new R session.")

# notify front-end (if any) that Python is about to be initialized
callback <- getOption("reticulate.python.beforeInitialized")
if (is.function(callback))
Expand Down Expand Up @@ -218,6 +224,8 @@ initialize_python <- function(required_module = NULL, use_environment = NULL) {

)

reg.finalizer(.globals, \(e) py_finalize(), onexit = TRUE)

# set available flag indicating we have py bindings
config$available <- TRUE

Expand Down
4 changes: 1 addition & 3 deletions R/zzz.R
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,5 @@


# .onUnload <- function(libpath) {
# ## Don't acquire GIL if we're embedded; https://github.com/rpy2/rpy2/issues/872
# # py_finalize()
# # py_clear_last_error()
# py_finalize() # called from reg.finalizer(.globals) instead.
# }
30 changes: 24 additions & 6 deletions src/event_loop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ namespace {
volatile sig_atomic_t s_pollingRequested;
bool s_flush_std_buffers = true;

bool running = true;
tthread::thread* t = nullptr;

// Forward declarations
int pollForEvents(void*);

Expand All @@ -49,10 +52,7 @@ int pollForEvents(void*);
// the polling signal will not be set).
void eventPollingWorker(void *) {

while (true) {

// Throttle via sleep
tthread::this_thread::sleep_for(tthread::chrono::milliseconds(500));
while (running) {

// Schedule polling on the main thread if the interpeter is still running.
// Note that Py_AddPendingCall is documented to be callable from a background
Expand All @@ -65,6 +65,9 @@ void eventPollingWorker(void *) {
Py_AddPendingCall(pollForEvents, NULL);
}

// Throttle via sleep
tthread::this_thread::sleep_for(tthread::chrono::milliseconds(500));

}

}
Expand Down Expand Up @@ -138,9 +141,24 @@ int pollForEvents(void*) {

// Initialize event loop polling background thread
void initialize() {
tthread::thread t(eventPollingWorker, NULL);
t.detach();
running = true;
t = new tthread::thread(eventPollingWorker, NULL);
}

void deinitialize(bool wait) {
// signal the thread to stop
running = false;

// clear the ref
if (t) {
if (wait) { // default: false
t->join();
delete t; // Clean up the thread object
t = nullptr;
}
}
}


} // namespace event_loop
} // namespace reticulate
2 changes: 2 additions & 0 deletions src/event_loop.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ namespace event_loop {

void initialize();

void deinitialize(bool wait = false);

} // namespace event_loop
} // namespace reticulate

Expand Down
1 change: 1 addition & 0 deletions src/libpython.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ bool LibPython::loadSymbols(bool python3, std::string* pError)
bool is64bit = sizeof(size_t) >= 8;

LOAD_PYTHON_SYMBOL(Py_InitializeEx)
LOAD_PYTHON_SYMBOL(Py_Finalize)
LOAD_PYTHON_SYMBOL(Py_IsInitialized)
LOAD_PYTHON_SYMBOL(Py_GetVersion)
LOAD_PYTHON_SYMBOL(Py_AddPendingCall)
Expand Down
1 change: 1 addition & 0 deletions src/libpython.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ void initialize_type_objects(bool python3);
PyType_FastSubclass((PyTypeObject*)(x), Py_TPFLAGS_BASE_EXC_SUBCLASS))

LIBPYTHON_EXTERN void (*Py_InitializeEx)(int);
LIBPYTHON_EXTERN void (*Py_Finalize)();
LIBPYTHON_EXTERN int (*Py_IsInitialized)();
LIBPYTHON_EXTERN const char* (*Py_GetVersion)();
LIBPYTHON_EXTERN char* (*Py_GetProgramFullPath_v2)();
Expand Down
69 changes: 56 additions & 13 deletions src/python.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2630,10 +2630,11 @@ interrupt_handler(int signum) {
PyOS_setsig(signum, interrupt_handler);
}

// [[Rcpp::export]]
void install_interrupt_handlers() {

PyOS_sighandler_t orig_interrupt_handler = NULL;

PyOS_sighandler_t install_interrupt_handlers_() {
// Installs a C handler and a Python handler
//
// Exported as an R symbol in case the user did some action that cleared the
// handlers, e.g., calling signal.signal() in Python, and wants to restore
// the correct handler.
Expand All @@ -2652,14 +2653,19 @@ void install_interrupt_handlers() {
if (result.is_null()) {
PyErr_Print();
Rcpp::warning("Failed to set interrupt signal handlers");
return;
return NULL;
}

// install the C handler.
//
// 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.
PyOS_setsig(SIGINT, interrupt_handler);
return PyOS_setsig(SIGINT, interrupt_handler);
}

// [[Rcpp::export]]
void install_interrupt_handlers() {
install_interrupt_handlers_();
}

extern "C"
Expand Down Expand Up @@ -2942,7 +2948,7 @@ void py_initialize(const std::string& python,
const wchar_t *argv[1] = {s_python_v3.c_str()};
PySys_SetArgv_v3(1, const_cast<wchar_t**>(argv));

install_interrupt_handlers();
orig_interrupt_handler = install_interrupt_handlers_();
}

} else { // python2
Expand All @@ -2968,8 +2974,8 @@ void py_initialize(const std::string& python,
const char *argv[1] = {s_python.c_str()};
PySys_SetArgv(1, const_cast<char**>(argv));

orig_interrupt_handler = install_interrupt_handlers_();
PyOS_setsig(SIGINT, interrupt_handler);
install_interrupt_handlers();
}

s_main_thread = tthread::this_thread::get_id();
Expand Down Expand Up @@ -3012,13 +3018,50 @@ void py_initialize(const std::string& python,

// [[Rcpp::export]]
void py_finalize() {

if (R_ParseEvalString(".globals$finalized", ns_reticulate) != R_NilValue)
stop("py_finalize() can only be called once per R session");

reticulate::event_loop::deinitialize(/*wait =*/ false);
pending_py_calls_notifier::deinitialize();

// We shouldn't call PyFinalize() if R is embedded in Python. https://github.com/rpy2/rpy2/issues/872
// if(!s_is_python_initialized && !s_was_python_initialized_by_reticulate)
// return;
//
// ::Py_Finalize();
// s_is_python_initialized = false;
// s_was_python_initialized_by_reticulate = false;
if(!s_is_python_initialized || !s_was_python_initialized_by_reticulate)
return;

{
PyGILState_Ensure();
Py_MakePendingCalls();
if (orig_interrupt_handler)
PyOS_setsig(SIGINT, orig_interrupt_handler);
Py_Finalize();
}

{
// To allow to call py_initialize() again would take:
// - tracking and invalidating all objects declared in functions with
// `static PyObject*` (including static modules like numpy).
// - unloading the library: libpython::SharedLibrary::unload(&error);
// - setting all loaded symbols to NULL;
// - check if internal symbols Python loads persist in the process, and
// if they need to be somehow discovered and unloaded.
// - ... problably other things too.
}

s_is_python_initialized = false;
s_was_python_initialized_by_reticulate = false;

// Make sure that attempting to get the gil again will call
// `ensure_python_initialized()`, which will now throw an error.
R_ParseEvalString("local({ "
"rm(list = names(.globals), envir = .globals); " // clear R-level references to previous config or python objects
".globals$finalized <- TRUE; "
".globals$py_repl_active <- FALSE; " // used by IDE?
"})",
ns_reticulate);
PyGILState_Ensure = &_initialize_python_and_PyGILState_Ensure;

// reticulate::event_loop::deinitialize(/*wait =*/ true);
}

// [[Rcpp::export]]
Expand Down

0 comments on commit d61a54d

Please sign in to comment.