Implement r_safely! in harp and use it in session API

Author: lionel-

crates/ark/src/lib.rs

@@ -29,6 +29,16 @@ pub use r_task::r_task;
 use serde::Deserialize;
 use serde::Serialize;
 
+#[macro_export]
+macro_rules! r_safely {
+    ($($expr:tt)*) => {{
+        #[allow(unused_unsafe)]
+        ark::r_task::safely(|| {
+            unsafe { $($expr)* } }
+        )
+    }}
+}
+
 #[derive(Debug, Eq, PartialEq, Copy, Clone, Default, Deserialize, Serialize)]
 pub struct Position {
     row: usize,

crates/ark/src/r_task.rs

@@ -10,7 +10,7 @@ use std::{
     time::Duration,
 };
 
-use libR_sys::R_interrupts_suspended;
+use harp::exec::safely;
 
 extern "C" {
     pub static mut R_PolledEvents: Option<unsafe extern "C" fn()>;
@@ -119,38 +119,5 @@ fn acquire_r_main() -> &'static mut RMain {
     }
 }
 
-// TODO: Should probably run in a toplevel-exec. Tasks also need a timeout.
-// This could be implemented with R interrupts but would require to
-// unsafely jump over the Rust stack, unless we wrapped all R API functions
-// to return an Option.
-fn safely<'env, F, T>(f: F) -> T
-where
-    F: FnOnce() -> T,
-    F: 'env + Send,
-    T: 'env + Send,
-{
-    let polled_events = unsafe { R_PolledEvents };
-    let interrupts_suspended = unsafe { R_interrupts_suspended };
-    unsafe {
-        // Disable polled events in this scope.
-        R_PolledEvents = Some(r_polled_events_disabled);
-
-        // Disable interrupts in this scope.
-        R_interrupts_suspended = 1;
-    }
-
-    // Execute the callback.
-    let result = f();
-
-    // Restore state
-    // TODO: Needs unwind protection
-    unsafe {
-        R_interrupts_suspended = interrupts_suspended;
-        R_PolledEvents = polled_events;
-    }
-
-    result
-}
-
 // Tests are tricky because `harp::test::start_r()` is very bare bones and
 // doesn't have an `R_MAIN`.

crates/harp/src/exec.rs

@@ -15,6 +15,8 @@ use libR_sys::*;
 
 use crate::error::Error;
 use crate::error::Result;
+use crate::lock::r_polled_events_disabled;
+use crate::lock::R_PolledEvents;
 use crate::object::RObject;
 use crate::protect::RProtect;
 use crate::r_string;
@@ -418,6 +420,39 @@ pub fn r_parse(code: &str) -> Result<RObject> {
     }
 }
 
+// TODO: Should probably run in a toplevel-exec. Tasks also need a timeout.
+// This could be implemented with R interrupts but would require to
+// unsafely jump over the Rust stack, unless we wrapped all R API functions
+// to return an Option.
+pub fn safely<'env, F, T>(f: F) -> T
+where
+    F: FnOnce() -> T,
+    F: 'env,
+    T: 'env,
+{
+    let polled_events = unsafe { R_PolledEvents };
+    let interrupts_suspended = unsafe { R_interrupts_suspended };
+    unsafe {
+        // Disable polled events in this scope.
+        R_PolledEvents = Some(r_polled_events_disabled);
+
+        // Disable interrupts in this scope.
+        R_interrupts_suspended = 1;
+    }
+
+    // Execute the callback.
+    let result = f();
+
+    // Restore state
+    // TODO: Needs unwind protection
+    unsafe {
+        R_interrupts_suspended = interrupts_suspended;
+        R_PolledEvents = polled_events;
+    }
+
+    result
+}
+
 #[cfg(test)]
 mod tests {
 

crates/harp/src/lib.rs

@@ -42,6 +42,16 @@ macro_rules! r_lock {
 
 }
 
+#[macro_export]
+macro_rules! r_safely {
+    ($($expr:tt)*) => {{
+        #[allow(unused_unsafe)]
+        $crate::exec::safely(|| {
+            unsafe { $($expr)* } }
+        )
+    }}
+}
+
 #[macro_export]
 macro_rules! with_vector_impl {
     ($x:expr, $class:ident, $variable:ident, $($code:tt)*) => {{

crates/harp/src/session.rs

@@ -15,7 +15,7 @@ use crate::exec::r_parse;
 use crate::object::RObject;
 use crate::protect::RProtect;
 use crate::r_lang;
-use crate::r_lock;
+use crate::r_safely;
 use crate::r_symbol;
 use crate::utils::r_normalize_path;
 use crate::utils::r_try_eval_silent;
@@ -32,16 +32,16 @@ static mut OPTIONS_FN: Option<SEXP> = None;
 pub fn r_n_frame() -> crate::error::Result<i32> {
     SESSION_INIT.call_once(init_interface);
 
-    r_lock! {
+    r_safely! {
         let ffi = r_try_eval_silent(NFRAME_CALL.unwrap_unchecked(), R_BaseEnv)?;
         let n_frame = IntegerVector::new(ffi)?;
         Ok(n_frame.get_unchecked_elt(0))
     }
 }
 
 pub fn r_sys_frame(n: c_int) -> crate::error::Result<SEXP> {
-    r_lock! {
-        let mut protect = RProtect::new();
+    let mut protect = unsafe { RProtect::new() };
+    r_safely! {
         let n = protect.add(Rf_ScalarInteger(n));
         let call = protect.add(r_lang!(r_symbol!("sys.frame"), n));
         Ok(r_try_eval_silent(call, R_BaseEnv)?)
@@ -101,7 +101,7 @@ pub fn r_stack_info() -> anyhow::Result<Vec<FrameInfo>> {
 
     // FIXME: It's better not to use `r_try_catch()` here because it adds
     // frames to the stack. Should wrap in a top-level-exec instead.
-    let _ = r_lock!({
+    let _ = r_safely!({
         (|| -> anyhow::Result<()> {
             let info = r_try_eval_silent(STACK_INFO_CALL.unwrap(), R_GlobalEnv)?;
             Rf_protect(info);