Skip to content

Commit

Permalink
Refurbished extendr#635 (extendr#658)
Browse files Browse the repository at this point in the history
fix: don't need to \0 terminate strings with `CString`
  • Loading branch information
CGMossa committed Feb 9, 2024
1 parent ab9e218 commit 94db446
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 102 deletions.
8 changes: 7 additions & 1 deletion .github/workflows/msrv.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,17 @@ permissions:

jobs:
test-min-rust-version:
# PowerShell core is available on all platforms and can be used to unify scripts
defaults:
run:
shell: pwsh
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: baptiste0928/cargo-install@v2
with:
crate: cargo-msrv
- name: Verify minimum rust version
run: cargo-msrv --path extendr-api/ verify
run: |
. ./ci-cargo.ps1
ci-cargo msrv --path extendr-api/ verify -ActionName "Verify Rust MSRV"
3 changes: 1 addition & 2 deletions extendr-api/src/lang_macros.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
//! Argument parsing and checking.
//!
use libR_sys::*;
//use crate::robj::*;
use crate::robj::GetSexp;
use crate::robj::Robj;
use crate::single_threaded;
use libR_sys::*;

/// Convert a list of tokens to an array of tuples.
#[doc(hidden)]
Expand Down
4 changes: 1 addition & 3 deletions extendr-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,9 +387,7 @@ pub use lang_macros::*;
pub use na::*;
pub use rmacros::*;
pub use robj::*;
pub use thread_safety::{
catch_r_error, handle_panic, single_threaded, this_thread_id, throw_r_error,
};
pub use thread_safety::{catch_r_error, handle_panic, single_threaded, throw_r_error};
pub use wrapper::*;

pub use extendr_macros::*;
Expand Down
4 changes: 2 additions & 2 deletions extendr-api/src/ownership.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
//! A single preserved vector holds ownership of all protected objects.
//!
//! Objects are reference counted, so multiple calls are possible,
//! unlike R_PreserveObject.
//! unlike `R_PreserveObject`.
//!
//! This module exports two functions, protect(sexp) and unprotect(sexp).
//! This module exports two functions, `protect(sexp)` and `unprotect(sexp)`.
use once_cell::sync::Lazy;
use std::collections::hash_map::{Entry, HashMap};
Expand Down
4 changes: 1 addition & 3 deletions extendr-api/src/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ pub use super::robj::{
RobjItertools, Slices, Types,
};

pub use super::thread_safety::{
catch_r_error, handle_panic, single_threaded, this_thread_id, throw_r_error,
};
pub use super::thread_safety::{catch_r_error, handle_panic, single_threaded, throw_r_error};

pub use super::wrapper::{
Complexes, Dataframe, Doubles, EnvIter, Environment, Expressions, ExternalPtr, FromList,
Expand Down
33 changes: 5 additions & 28 deletions extendr-api/src/thread_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,12 @@ use crate::*;
/// It is not tied to an actual instance of R.
static R_API_LOCK: parking_lot::ReentrantMutex<()> = parking_lot::ReentrantMutex::new(());

// Get an integer 1.. for each thread that calls this.
pub fn this_thread_id() -> u32 {
THREAD_ID.with(|&v| v)
}

/// Run a function single threaded.
/// Note: This will fail badly if the called function panics or calls RF_error.
/// Run `f` while ensuring that `f` runs in a single-threaded manner.
///
/// ```
/// use extendr_api::prelude::*;
/// use std::sync::atomic::{AtomicU32, Ordering};
/// static GLOBAL_THREAD_COUNT: AtomicU32 = AtomicU32::new(0);
/// This is intended for single-threaded access of the R's C-API.
/// It is possible to have nested calls of `single_threaded` without deadlocking.
///
/// let threads : Vec<_> = (0..100).map(|_| {
/// std::thread::spawn(move|| {
/// single_threaded(|| {
/// // check that we are single threaded.
/// let old_thread_count = GLOBAL_THREAD_COUNT.fetch_add(1, Ordering::AcqRel);
/// assert_eq!(old_thread_count, 0);
/// std::thread::sleep(std::time::Duration::from_millis(1));
/// GLOBAL_THREAD_COUNT.fetch_sub(1, Ordering::AcqRel);
/// // recursive calls are ok.
/// assert_eq!(single_threaded(|| {
/// 1
/// }), 1);
/// })
/// })
/// }).collect();
/// ```
/// Note: This will fail badly if the called function `f` panics or calls `Rf_error`.
pub fn single_threaded<F, R>(f: F) -> R
where
F: FnOnce() -> R,
Expand Down Expand Up @@ -73,7 +50,7 @@ pub fn throw_r_error<S: AsRef<str>>(s: S) -> ! {
};
}

/// Wrap an R function such as Rf_findFunction and convert errors and panics into results.
/// Wrap an R function such as `Rf_findFunction` and convert errors and panics into results.
/// ```ignore
/// use extendr_api::prelude::*;
/// test! {
Expand Down
2 changes: 1 addition & 1 deletion extendr-macros/src/wrappers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ pub fn make_function_wrappers(
// included in panic. The advantage would be the panic cause could be included
// in the R terminal error message and not only via std-err.
// but it should be handled in a separate function and not in-lined here.
let err_string = format!("user function panicked: {}\0",#r_name_str);
let err_string = format!("user function panicked: {}",#r_name_str);
// cannot use throw_r_error here for some reason.
// handle_panic() exports err string differently than throw_r_error.
extendr_api::handle_panic(err_string.as_str(), || panic!());
Expand Down
Loading

0 comments on commit 94db446

Please sign in to comment.