Repository: extendr/extendr
Pull request #175 (2021-02-10 23:44:53)

Avoid r preserve object
This PR introduces an alternative to `R_PreserveObject`.

This implementation tracks `SEXP` pointers and reference counts them so we can implement
safer owned `Robj` instances.

This should allow us to implement `Clone` on Robj by incrementing the reference.

It will not be cost free, but probably better than the linked-list version of `R_PreserveObject`
which is the default.

We will have to benchmark it in due course and may be able to improve the performance with
a custom hasher.


andy-thomason: I suspect that the large number of allocations in the test are exposing weaknesses elsewhere hence
the occasional CI failures.

andy-thomason: I did originally use protect, but overflowed the stack limit.

I'll check it out in the morning.

Andy.

On Thu, 11 Feb 2021, 00:30 Claus Wilke, <notifications@github.com> wrote:

> *@clauswilke* commented on this pull request.
> ------------------------------
>
> In extendr-api/src/ownership.rs
> <https://github.com/extendr/extendr/pull/175#discussion_r574182167>:
>
> > +    #[test]
> +    fn collection_test() {
> +        test! {
> +            single_threaded(|| unsafe {
> +                {
> +                    let mut own = OWNERSHIP.lock().expect("protect failed");
> +                    own.check_objects();
> +                }
> +
> +                // Force a garbage collect.
> +                let test_size = INITIAL_PRESERVATION_SIZE + EXTRA_PRESERVATION_SIZE * 5;
> +
> +                // Make some test objects.
> +                let sexps = (0..test_size).map(|_| {
> +                    let sexp = Rf_ScalarInteger(1);
> +                    R_PreserveObject(sexp);
>
> I bet here you're triggering the issue described here: RcppCore/Rcpp#1081
> <https://github.com/RcppCore/Rcpp/issues/1081>
> The crash was fixed in recent versions of R, that's why only the oldrel CI
> runs fail.
>
> Any reason you're using R_PreserveObject() instead of Rf_protect()? The
> latter shouldn't cause the problem.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/extendr/extendr/pull/175#pullrequestreview-588176691>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAL36XCH3WZVYBDAIMOXRQ3S6MQLDANCNFSM4XN4DFKQ>
> .
>

clauswilke: Yeah, I guess `Rf_protect()` is meant for rapid protecting and unprotecting. Maybe just allocate an R vector and add these pointers to that vector, just like you do in your actual protection code also.
andy-thomason: Good idea.

On Thu, Feb 11, 2021 at 12:54 AM Claus Wilke <notifications@github.com>
wrote:

> Yeah, I guess Rf_protect() is meant for rapid protecting and
> unprotecting. Maybe just allocate an R vector and add these pointers to
> that vector, just like you do in your actual protection code also.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/extendr/extendr/pull/175#issuecomment-777141028>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAL36XBQWV2Z6IKR3OPAD6LS6MTDNANCNFSM4XN4DFKQ>
> .
>

andy-thomason: :tada: Build is green.
andy-thomason: I'll update it, then do a merge.

This PR is quite a big change as it affects the object model in fundamental
ways.

We may be able to deal with mutability in a more sensible way after this.




On Fri, Feb 12, 2021 at 12:25 AM Claus Wilke <notifications@github.com>
wrote:

> *@clauswilke* commented on this pull request.
> ------------------------------
>
> In extendr-api/src/ownership.rs
> <https://github.com/extendr/extendr/pull/175#discussion_r574919510>:
>
> > @@ -303,9 +318,12 @@ mod test {
>
>                  let test_size = INITIAL_PRESERVATION_SIZE + EXTRA_PRESERVATION_SIZE * 5;
>
>
>
>                  // Make some test objects.
>
> -                let sexps = (0..test_size).map(|_| {
>
> +                let sexp_pres = Rf_allocVector(VECSXP, test_size as R_xlen_t);
>
> +                R_PreserveObject(sexp_pres);
>
>
> While this will work, it would be more idiomatic at this point to use
> Rf_protect(). You’re just protecting the SEXP, doing a quick calculation
> with it, and then release again.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/extendr/extendr/pull/175#pullrequestreview-589099507>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAL36XGHZNTFHXQAAMQ5O4LS6RYN3ANCNFSM4XN4DFKQ>
> .
>


Patch:
diff --git a/extendr-api/Cargo.toml b/extendr-api/Cargo.toml
index 453eccdf5..0feb19808 100644
--- a/extendr-api/Cargo.toml
+++ b/extendr-api/Cargo.toml
@@ -18,6 +18,7 @@ libR-sys = "0.2.0"
 extendr-macros = { path = "../extendr-macros", version="0.1.11" }
 extendr-engine = { path = "../extendr-engine", version="0.1.11" }
 ndarray = { version = "0.13.1", optional = true }
+lazy_static = "1.4"
 
 [features]
 default = []
diff --git a/extendr-api/src/lib.rs b/extendr-api/src/lib.rs
index 0d07fe4cc..6ef44c40e 100644
--- a/extendr-api/src/lib.rs
+++ b/extendr-api/src/lib.rs
@@ -208,6 +208,7 @@ pub mod lang_macros;
 pub mod logical;
 pub mod matrix;
 pub mod metadata;
+pub mod ownership;
 pub mod prelude;
 pub mod rmacros;
 
diff --git a/extendr-api/src/ownership.rs b/extendr-api/src/ownership.rs
new file mode 100644
index 000000000..6459867bf
--- /dev/null
+++ b/extendr-api/src/ownership.rs
@@ -0,0 +1,348 @@
+//! Maintain ownership of R objects.
+//!
+//! This provides the functions protect() and unprotect()
+//! A single preserved vector holds ownership of all protected objects.
+//!
+//! Objects are refernce counted, so multiple calls are possible,
+//! unlike R_PreserveObject.
+//!
+//! This module exports two functions, protect(sexp) and unprotect(sexp).
+
+use lazy_static::lazy_static;
+use std::collections::hash_map::{Entry, HashMap};
+use std::sync::Mutex;
+
+use libR_sys::{
+    R_NilValue, R_PreserveObject, R_ReleaseObject, R_xlen_t, Rf_allocVector, LENGTH,
+    SET_VECTOR_ELT, SEXP, VECSXP, VECTOR_ELT,
+};
+
+lazy_static! {
+    static ref OWNERSHIP: Mutex<Ownership> = Mutex::new(Ownership::new());
+}
+
+pub unsafe fn protect(sexp: SEXP) {
+    let mut own = OWNERSHIP.lock().expect("protect failed");
+    own.protect(sexp);
+}
+
+pub unsafe fn unprotect(sexp: SEXP) {
+    let mut own = OWNERSHIP.lock().expect("unprotect failed");
+    own.unprotect(sexp);
+}
+
+pub const INITIAL_PRESERVATION_SIZE: usize = 25000;
+pub const EXTRA_PRESERVATION_SIZE: usize = 25000;
+
+struct Object {
+    refcount: usize,
+    index: usize,
+}
+
+// A reference counted object with an index in the preservation vector.
+struct Ownership {
+    // A growable vector containing all owned objects.
+    preservation: usize,
+
+    // An incrementing count of objects through the vector.
+    cur_index: usize,
+
+    // The size of the vector.
+    max_index: usize,
+
+    // A hash map from SEXP address to object.
+    objects: HashMap<usize, Object>,
+}
+
+impl Ownership {
+    fn new() -> Self {
+        unsafe {
+            let preservation = Rf_allocVector(VECSXP, INITIAL_PRESERVATION_SIZE as R_xlen_t);
+            R_PreserveObject(preservation);
+            Ownership {
+                preservation: preservation as usize,
+                cur_index: 0,
+                max_index: INITIAL_PRESERVATION_SIZE,
+                objects: HashMap::with_capacity(INITIAL_PRESERVATION_SIZE),
+            }
+        }
+    }
+
+    unsafe fn protect(&mut self, sexp: SEXP) {
+        if self.cur_index == self.max_index {
+            self.garbage_collect();
+        }
+
+        let sexp_usize = sexp as usize;
+        match *self {
+            Ownership {
+                ref mut preservation,
+                ref mut cur_index,
+                ref mut max_index,
+                ref mut objects,
+            } => {
+                let mut entry = objects.entry(sexp_usize);
+                let preservation_sexp = *preservation as SEXP;
+                match entry {
+                    Entry::Occupied(ref mut occupied) => {
+                        if occupied.get().refcount == 0 {
+                            // Address re-used - re-set the sexp.
+                            SET_VECTOR_ELT(
+                                preservation_sexp,
+                                occupied.get().index as R_xlen_t,
+                                sexp,
+                            );
+                        }
+                        occupied.get_mut().refcount += 1;
+                    }
+                    Entry::Vacant(vacant) => {
+                        let index = *cur_index;
+                        SET_VECTOR_ELT(preservation_sexp, index as R_xlen_t, sexp);
+                        *cur_index += 1;
+                        assert!(index != *max_index);
+                        let refcount = 1;
+                        vacant.insert(Object { refcount, index });
+                    }
+                }
+            }
+        }
+    }
+
+    pub unsafe fn unprotect(&mut self, sexp: SEXP) {
+        let sexp_usize = sexp as usize;
+        match *self {
+            Ownership {
+                preservation,
+                cur_index: _,
+                max_index: _,
+                ref mut objects,
+            } => {
+                let mut entry = objects.entry(sexp_usize);
+                match entry {
+                    Entry::Occupied(ref mut occupied) => {
+                        let object = occupied.get_mut();
+                        if object.refcount == 0 {
+                            panic!("Attempt to unprotect an already unprotected object.")
+                        } else {
+                            object.refcount -= 1;
+                            if object.refcount == 0 {
+                                // Clear the preservation vector, but keep the hash table entry.
+                                // It is hard to clear the hash table entry here because we don't
+                                // have a ref to objects anymore and it is faster to clear them up en-masse.
+                                let preservation_sexp = preservation as SEXP;
+                                SET_VECTOR_ELT(
+                                    preservation_sexp,
+                                    object.index as R_xlen_t,
+                                    R_NilValue,
+                                );
+                            }
+                        }
+                    }
+                    Entry::Vacant(_) => {
+                        panic!("Attempt to unprotect a never protected object.")
+                    }
+                }
+            }
+        }
+    }
+
+    #[allow(dead_code)]
+    unsafe fn ref_count(&mut self, sexp: SEXP) -> usize {
+        match *self {
+            Ownership {
+                preservation: _,
+                cur_index: _,
+                max_index: _,
+                ref mut objects,
+            } => {
+                let sexp_usize = sexp as usize;
+                let mut entry = objects.entry(sexp_usize);
+                match entry {
+                    Entry::Occupied(ref mut occupied) => occupied.get().refcount,
+                    Entry::Vacant(_) => 0,
+                }
+            }
+        }
+    }
+
+    // Garbage collect the tracking structures.
+    unsafe fn garbage_collect(&mut self) {
+        // println!("garbage_collect {} {}", self.cur_index, self.max_index);
+        let new_size = self.cur_index + EXTRA_PRESERVATION_SIZE;
+        let new_sexp = Rf_allocVector(VECSXP, new_size as R_xlen_t);
+        R_PreserveObject(new_sexp);
+        let old_sexp = self.preservation as SEXP;
+
+        let mut new_objects = HashMap::with_capacity(new_size);
+
+        // copy non-null elements to new vector and hashmap.
+        let mut j = 0;
+        for (addr, object) in self.objects.iter() {
+            if object.refcount != 0 {
+                SET_VECTOR_ELT(new_sexp, j as R_xlen_t, *addr as SEXP);
+                new_objects.insert(
+                    *addr,
+                    Object {
+                        refcount: object.refcount,
+                        index: j,
+                    },
+                );
+                j += 1;
+            }
+        }
+        // println!("j={}", j);
+
+        R_ReleaseObject(old_sexp);
+        self.preservation = new_sexp as usize;
+        self.cur_index = j;
+        self.max_index = new_size;
+        self.objects = new_objects;
+    }
+
+    // Check the consistency of the model.
+    #[allow(dead_code)]
+    unsafe fn check_objects(&mut self) {
+        let preservation_sexp = self.preservation as SEXP;
+        assert_eq!(self.max_index, LENGTH(preservation_sexp) as usize);
+
+        // println!("\ncheck");
+
+        for (addr, object) in self.objects.iter() {
+            assert!(object.index < self.max_index);
+            let elt = VECTOR_ELT(preservation_sexp, object.index as R_xlen_t);
+            // println!(
+            //     "refcount={:?} index={:?} elt={:?}",
+            //     object.refcount, object.index, elt
+            // );
+            if object.refcount != 0 {
+                // A non-zero refcount implies the object is in the vector.
+                assert_eq!(elt, *addr as SEXP);
+            } else {
+                // A zero refcount implies the object is NULL in the vector.
+                assert_eq!(elt, R_NilValue);
+            }
+        }
+
+        // println!("check 2");
+        for i in 0..self.max_index {
+            let elt = VECTOR_ELT(preservation_sexp, i as R_xlen_t);
+            if elt == R_NilValue {
+                assert_eq!(self.ref_count(elt), 0);
+            } else {
+                assert!(self.ref_count(elt) != 0);
+            }
+        }
+        // println!("/check");
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::*;
+    use libR_sys::{Rf_ScalarInteger, Rf_protect, Rf_unprotect};
+
+    #[test]
+    fn basic_test() {
+        test! {
+            single_threaded(|| unsafe {
+                {
+                    let mut own = OWNERSHIP.lock().expect("lock failed");
+                    own.check_objects();
+                }
+
+                let sexp1 = Rf_protect(Rf_ScalarInteger(1));
+                let sexp2 = Rf_protect(Rf_ScalarInteger(2));
+                protect(sexp1);
+                {
+                    let mut own = OWNERSHIP.lock().expect("lock failed");
+                    own.check_objects();
+                    assert_eq!(own.ref_count(sexp1), 1);
+                    assert_eq!(own.ref_count(sexp2), 0);
+                }
+
+                protect(sexp1);
+                {
+                    let mut own = OWNERSHIP.lock().expect("lock failed");
+                    own.check_objects();
+                    assert_eq!(own.ref_count(sexp1), 2);
+                    assert_eq!(own.ref_count(sexp2), 0);
+                }
+
+                unprotect(sexp1);
+                {
+                    let mut own = OWNERSHIP.lock().expect("lock failed");
+                    own.check_objects();
+                    assert_eq!(own.ref_count(sexp1), 1);
+                    assert_eq!(own.ref_count(sexp2), 0);
+                }
+
+                unprotect(sexp1);
+                {
+                    let mut own = OWNERSHIP.lock().expect("lock failed");
+                    own.check_objects();
+                    assert_eq!(own.ref_count(sexp1), 0);
+                    assert_eq!(own.ref_count(sexp2), 0);
+                }
+
+                protect(sexp2);
+                {
+                    let mut own = OWNERSHIP.lock().expect("lock failed");
+                    own.check_objects();
+                    assert_eq!(own.ref_count(sexp1), 0);
+                    assert_eq!(own.ref_count(sexp2), 1);
+                }
+
+                protect(sexp1);
+                {
+                    let mut own = OWNERSHIP.lock().expect("lock failed");
+                    own.check_objects();
+                    assert_eq!(own.ref_count(sexp1), 1);
+                    assert_eq!(own.ref_count(sexp2), 1);
+                }
+                Rf_unprotect(2);
+            });
+        }
+    }
+
+    #[test]
+    fn collection_test() {
+        test! {
+            single_threaded(|| unsafe {
+                {
+                    let mut own = OWNERSHIP.lock().expect("protect failed");
+                    own.check_objects();
+                }
+
+                // Force a garbage collect.
+                let test_size = INITIAL_PRESERVATION_SIZE + EXTRA_PRESERVATION_SIZE * 5;
+
+                // Make some test objects.
+                let sexp_pres = Rf_allocVector(VECSXP, test_size as R_xlen_t);
+                Rf_protect(sexp_pres);
+
+                let sexps = (0..test_size).map(|i| {
+                    let sexp = Rf_ScalarInteger(1);
+                    SET_VECTOR_ELT(sexp_pres, i as R_xlen_t, sexp);
+                    sexp
+                }).collect::<Vec<_>>();
+
+                for (i, sexp) in sexps.iter().enumerate() {
+                    protect(*sexp);
+                    if i % 2 == 0 {
+                        unprotect(*sexp);
+                    }
+                }
+
+                {
+                    let mut own = OWNERSHIP.lock().expect("protect failed");
+                    own.check_objects();
+                    own.garbage_collect();
+                    own.check_objects();
+                }
+
+                Rf_unprotect(1);
+            });
+        }
+    }
+}
diff --git a/extendr-api/src/robj/into_robj.rs b/extendr-api/src/robj/into_robj.rs
index 24221772e..c2fdc341d 100644
--- a/extendr-api/src/robj/into_robj.rs
+++ b/extendr-api/src/robj/into_robj.rs
@@ -321,7 +321,7 @@ where
         let sexptype = I::Item::sexptype();
         if sexptype != 0 {
             let sexp = Rf_allocVector(sexptype, len as R_xlen_t);
-            R_PreserveObject(sexp);
+            ownership::protect(sexp);
             match sexptype {
                 REALSXP => {
                     let ptr = REAL(sexp);
diff --git a/extendr-api/src/robj/mod.rs b/extendr-api/src/robj/mod.rs
index 5fcdc066b..2378ddba9 100644
--- a/extendr-api/src/robj/mod.rs
+++ b/extendr-api/src/robj/mod.rs
@@ -129,7 +129,13 @@ pub enum Robj {
 
 impl Clone for Robj {
     fn clone(&self) -> Self {
-        self.duplicate()
+        unsafe {
+            match *self {
+                Robj::Owned(sexp) => new_owned(sexp),
+                Robj::Borrowed(sexp) => new_borrowed(sexp),
+                Robj::Sys(sexp) => new_sys(sexp),
+            }
+        }
     }
 }
 
@@ -728,7 +734,7 @@ impl Robj {
     pub unsafe fn unprotected(self) -> Robj {
         match self {
             Robj::Owned(sexp) => {
-                single_threaded(|| R_ReleaseObject(sexp));
+                single_threaded(|| ownership::unprotect(sexp));
                 Robj::Borrowed(sexp)
             }
             _ => self,
@@ -1011,7 +1017,7 @@ impl Robj {
 
 #[doc(hidden)]
 pub unsafe fn new_owned(sexp: SEXP) -> Robj {
-    single_threaded(|| R_PreserveObject(sexp));
+    single_threaded(|| ownership::protect(sexp));
     Robj::Owned(sexp)
 }
 
@@ -1216,7 +1222,7 @@ impl Drop for Robj {
     fn drop(&mut self) {
         unsafe {
             match self {
-                Robj::Owned(sexp) => single_threaded(|| R_ReleaseObject(*sexp)),
+                Robj::Owned(sexp) => single_threaded(|| ownership::unprotect(*sexp)),
                 Robj::Borrowed(_) => (),
                 Robj::Sys(_) => (),
             }
diff --git a/extendr-api/src/wrapper.rs b/extendr-api/src/wrapper.rs
index 3dce4cfdc..c0b1955ab 100644
--- a/extendr-api/src/wrapper.rs
+++ b/extendr-api/src/wrapper.rs
@@ -233,7 +233,7 @@ impl<'a> From<Raw<'a>> for Robj {
         single_threaded(|| unsafe {
             let val = val.0;
             let sexp = Rf_allocVector(RAWSXP, val.len() as R_xlen_t);
-            R_PreserveObject(sexp);
+            ownership::protect(sexp);
             let ptr = RAW(sexp);
             for (i, &v) in val.iter().enumerate() {
                 *ptr.offset(i as isize) = v;
@@ -377,7 +377,7 @@ where
     single_threaded(|| unsafe {
         let values = values.into_iter();
         let sexp = Rf_allocVector(sexptype, values.len() as R_xlen_t);
-        R_PreserveObject(sexp);
+        ownership::protect(sexp);
         for (i, val) in values.enumerate() {
             SET_VECTOR_ELT(sexp, i as R_xlen_t, val.into().get());
         }


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 2c2be3cc1..68242a98b 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -140,10 +140,23 @@ jobs:
           foreach($target in ($env:BUILD_TARGETS).Split(',')) {
             ci-cargo test $(if($target -ne 'default') {"--target=$target"} ) '--' --nocapture -ActionName "Testing for $target target"
           }
+
+      - name: Obtain 'rextendr'
+        run : |
+          git clone https://github.com/extendr/rextendr ./tests/rextendr
+
+      # Dependencies from both test packages are combined
       - name: Query dependencies for integration testing
         run: |
           install.packages('remotes')
-          saveRDS(remotes::dev_package_deps(pkgdir = "tests/extendrtests", dependencies = TRUE), ".github/depends.Rds", version = 2)
+          saveRDS(
+            unique(
+              rbind(
+                remotes::dev_package_deps(pkgdir = "tests/extendrtests", dependencies = TRUE), 
+                remotes::dev_package_deps(pkgdir = "tests/rextendr", dependencies = TRUE))),
+            ".github/depends.Rds", 
+            version = 2
+          )
           writeLines(sprintf("R-%i.%i", getRversion()$major, getRversion()$minor), ".github/R-version")
         shell: Rscript {0}
 
@@ -154,29 +167,97 @@ jobs:
           key: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-${{ hashFiles('.github/depends.Rds') }}
           restore-keys: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-
 
-      - name: Install R dependencies for integration testing
+      # Currently rextendr requires R > 4, so if R version is older (e.g. 3.6.3), rextendr tests are omitted
+      # Regex is used to inject absolute path into Cargo.toml
+      - name: Install dependencies & configure R for integration testing
         run: |
           remotes::install_deps(pkgdir = "tests/extendrtests", dependencies = TRUE)
+          if (as.integer(R.version$major) >= 4) { 
+            remotes::install_deps(pkgdir = "tests/rextendr", dependencies = TRUE)
+          }
           remotes::install_cran("rcmdcheck")
+
+          api_path <- normalizePath(file.path(getwd(), "extendr-api"), winslash = "/")
+          toml_path <- file.path(getwd(), "tests", "extendrtests", "src", "rust", "Cargo.toml")
+          lines <- readLines(toml_path)
+          lines <- gsub(
+            "(^\\s*extendr-api\\s*=\\s*\\{\\s*path\\s*=\\s*\")(.*?)(\"\\s*\\})",
+            paste0("\\1", api_path, "\\3"),
+            lines
+          )
+          writeLines(lines, toml_path)
         shell: Rscript {0}
         
       # Windows: modified PATH will launch 32-bit Rscript if the build targets only i686/i386, so no conflict will arise
       # Rust pseudo multi-targeting: R CMD CHECK receives an extra argument '--no-multiarch' if BUILD_TARGETS is not 'default' and does not contain any commas
-      - name: Run R integration tests
-        id: r_integration_tests
+      # To enable RStudio support, extendrtests should be installable from 'extendr/tests/extendrtests',
+      # for this work directory should be modified and then reverted back
+      - name: Run R integration tests using {extendrtests}
+        id: extendrtests_check
         env:
           _R_CHECK_CRAN_INCOMING_REMOTE_: false
         run: |
-          setwd("tests/extendrtests")
-          rcmdcheck::rcmdcheck(path = ".", args = c("--no-manual", "--as-cran"), error_on = "warning", check_dir = "../../../")
+          cat("::group::Checking 'extendrtests'\n")
+          rcmdcheck::rcmdcheck(
+            path = "tests/extendrtests", 
+            args = c("--no-manual", "--as-cran", "--force-multiarch"),
+            error_on = "warning", 
+            check_dir = "extendrtests_check"
+          )
+          cat("::endgroup::\n")
         shell: Rscript {0}
         
-      - name: Upload check results from R integration tests
-        if: failure()
+      # If R is older than 4.0, rextendr is not tested and a warning is emitted. 
+      # With https://github.com/extendr/rextendr/pull/31
+      # rextendr can be configured using environment variables.
+      # 'patch.crates_io' is used to point libraries to local copies of
+      # extendr-api and extendr-macros, so rextendr tests current version of extendr-*
+      - name: Run R integration tests using {rextendr}
+        id: rextendr_check
+        env:
+          _R_CHECK_CRAN_INCOMING_REMOTE_: false
+        run: |
+          if (as.integer(R.version$major) < 4) {
+            cat("::warning:: R version is not compatible with 'rextendr'\n")
+          } else {
+            cat("::group::Checking 'rextendr'\n")
+            
+            patch.crates_io <-
+              paste(
+                paste0(
+                  "extendr-api = { path = \"",
+                  normalizePath(file.path(getwd(), "extendr-api"), winslash = "/"),
+                  "\" }"),
+                paste0(
+                  "extendr-macros = { path = \"",
+                  normalizePath(file.path(getwd(), "extendr-macros"), winslash = "/"),
+                  "\" }"),
+                sep = ";")
+
+            Sys.setenv(REXTENDR_PATCH_CRATES_IO = patch.crates_io)
+            rcmdcheck::rcmdcheck(
+              path = "tests/rextendr", 
+              args = c("--no-manual", "--as-cran", "--force-multiarch"), 
+              error_on = "warning", 
+              check_dir = "rextendr_check")
+            cat("::endgroup::\n")
+          }
+        shell: Rscript {0}
+
+
+      - name: Upload extendrtests check results from R integration tests
+        if: steps.extendrtests_check.outcome != 'success'
+        uses: actions/upload-artifact@main
+        with:
+          name:  extendrtests-${{ matrix.config.os }}-R-${{ matrix.config.r }}-rust-${{ matrix.config.rust-version }}
+          path: extendrtests_check
+
+      - name: Upload rextendr check results from R integration tests
+        if: steps.rextendr_check.outcome != 'success'
         uses: actions/upload-artifact@main
         with:
-          name:  ${{ matrix.config.os }}-R-${{ matrix.config.r }}-rust-${{ matrix.config.rust-version }}
-          path: check
+          name:  rextendr-${{ matrix.config.os }}-R-${{ matrix.config.r }}-rust-${{ matrix.config.rust-version }}
+          path: rextendr_check
 
 
   # All tests under this job are run with R devel and freshly generated bindings.
diff --git a/tests/extendrtests/src/rust/Cargo.toml b/tests/extendrtests/src/rust/Cargo.toml
index 035e26cb3..562b44c59 100644
--- a/tests/extendrtests/src/rust/Cargo.toml
+++ b/tests/extendrtests/src/rust/Cargo.toml
@@ -16,10 +16,10 @@ crate-type = ["staticlib"]
 extendr-api = "*"
 
 [patch.crates-io]
-## Build against local extendr sources. Check locally by 
-## running rcmdcheck::rcmdcheck(check_dir = "../../../") from
-## the root of extendrtests.
-extendr-api = { path = "../../../../../extendr/extendr-api/"}
+## This is configured to work with RStudio features.
+## Replace by absolute path to simplify testing.
+## CI overrides this path.
+extendr-api = { path = "../../../../../extendr/extendr-api"}
 
 ## Build against current extendr version on github. Not recommended
 ## for development work.