Merge pull request #487 from filmor/make-resource-binary

Implement ResourceArc::make_binary

Author: filmor

rustler/src/resource.rs

@@ -9,9 +9,9 @@ use std::mem;
 use std::ops::Deref;
 use std::ptr;
 
-use super::{Decoder, Encoder, Env, Error, NifResult, Term};
+use super::{Binary, Decoder, Encoder, Env, Error, NifResult, Term};
 use crate::wrapper::{
-    c_void, NifResourceFlags, MUTABLE_NIF_RESOURCE_HANDLE, NIF_ENV, NIF_RESOURCE_TYPE,
+    c_void, resource, NifResourceFlags, MUTABLE_NIF_RESOURCE_HANDLE, NIF_ENV, NIF_RESOURCE_TYPE,
 };
 
 /// Re-export a type used by the `resource!` macro.
@@ -78,7 +78,7 @@ pub fn open_struct_resource_type<T: ResourceTypeProvider>(
     flags: NifResourceFlags,
 ) -> Option<ResourceType<T>> {
     let res: Option<NIF_RESOURCE_TYPE> = unsafe {
-        crate::wrapper::resource::open_resource_type(
+        resource::open_resource_type(
             env.as_c_arg(),
             name.as_bytes(),
             Some(resource_destructor::<T>),
@@ -133,8 +133,7 @@ where
     /// ResourceTypeProvider implemented for it. See module documentation for info on this.
     pub fn new(data: T) -> Self {
         let alloc_size = get_alloc_size_struct::<T>();
-        let mem_raw =
-            unsafe { crate::wrapper::resource::alloc_resource(T::get_type().res, alloc_size) };
+        let mem_raw = unsafe { resource::alloc_resource(T::get_type().res, alloc_size) };
         let aligned_mem = unsafe { align_alloced_mem_for_struct::<T>(mem_raw) as *mut T };
 
         unsafe { ptr::write(aligned_mem, data) };
@@ -145,9 +144,49 @@ where
         }
     }
 
+    /// Make a resource binary associated with the given resource
+    ///
+    /// The closure `f` is called with the referenced object and must return a slice with the same
+    /// lifetime as the object. This means that the slice either has to be derived directly from
+    /// the instance or that it has to have static lifetime.
+    pub fn make_binary<'env, 'a, F>(&self, env: Env<'env>, f: F) -> Binary<'env>
+    where
+        F: FnOnce(&'a T) -> &'a [u8],
+    {
+        // This call is safe because `f` can only return a slice that lives at least as long as
+        // the given instance of `T`.
+        unsafe { self.make_binary_unsafe(env, f) }
+    }
+
+    /// Make a resource binary without strict lifetime checking
+    ///
+    /// The user *must* ensure that the lifetime of the returned slice is at least as long as the
+    /// lifetime of the referenced instance.
+    ///
+    /// # Safety
+    ///
+    /// This function is only safe if the slice that is returned from the closure is guaranteed to
+    /// live at least as long as the `ResourceArc` instance. If in doubt, use the safe version
+    /// `ResourceArc::make_binary` which enforces this bound through its signature.
+    pub unsafe fn make_binary_unsafe<'env, 'a, 'b, F>(&self, env: Env<'env>, f: F) -> Binary<'env>
+    where
+        F: FnOnce(&'a T) -> &'b [u8],
+    {
+        let bin = f(&*self.inner);
+        let binary = rustler_sys::enif_make_resource_binary(
+            env.as_c_arg(),
+            self.raw,
+            bin.as_ptr() as *const c_void,
+            bin.len(),
+        );
+
+        let term = Term::new(env, binary);
+        Binary::from_term_and_slice(term, bin)
+    }
+
     fn from_term(term: Term) -> Result<Self, Error> {
         let res_resource = match unsafe {
-            crate::wrapper::resource::get_resource(
+            resource::get_resource(
                 term.get_env().as_c_arg(),
                 term.as_c_arg(),
                 T::get_type().res,
@@ -157,7 +196,7 @@ where
             None => return Err(Error::BadArg),
         };
         unsafe {
-            crate::wrapper::resource::keep_resource(res_resource);
+            resource::keep_resource(res_resource);
         }
         let casted_ptr = unsafe { align_alloced_mem_for_struct::<T>(res_resource) as *mut T };
         Ok(ResourceArc {
@@ -167,12 +206,7 @@ where
     }
 
     fn as_term<'a>(&self, env: Env<'a>) -> Term<'a> {
-        unsafe {
-            Term::new(
-                env,
-                crate::wrapper::resource::make_resource(env.as_c_arg(), self.raw),
-            )
-        }
+        unsafe { Term::new(env, resource::make_resource(env.as_c_arg(), self.raw)) }
     }
 
     fn as_c_arg(&mut self) -> *const c_void {
@@ -203,7 +237,7 @@ where
     /// resource. The `T` value is not cloned.
     fn clone(&self) -> Self {
         unsafe {
-            crate::wrapper::resource::keep_resource(self.raw);
+            resource::keep_resource(self.raw);
         }
         ResourceArc {
             raw: self.raw,

rustler/src/types/binary.rs

@@ -237,7 +237,8 @@ unsafe impl Sync for OwnedBinary {}
 /// See [module-level doc](index.html) for more information.
 #[derive(Copy, Clone)]
 pub struct Binary<'a> {
-    inner: ErlNifBinary,
+    buf: *const u8,
+    size: usize,
     term: Term<'a>,
 }
 
@@ -257,7 +258,8 @@ impl<'a> Binary<'a> {
             )
         };
         Binary {
-            inner: owned.0,
+            buf: owned.0.data,
+            size: owned.0.size,
             term,
         }
     }
@@ -289,12 +291,26 @@ impl<'a> Binary<'a> {
         {
             return Err(Error::BadArg);
         }
+
+        let binary = unsafe { binary.assume_init() };
         Ok(Binary {
-            inner: unsafe { binary.assume_init() },
+            buf: binary.data,
+            size: binary.size,
             term,
         })
     }
 
+    /// Creates a Binary from a `term` and the associated slice
+    ///
+    /// The `term` *must* be constructed from the given slice, it is not checked.
+    pub(crate) unsafe fn from_term_and_slice(term: Term<'a>, binary: &[u8]) -> Self {
+        Binary {
+            term,
+            buf: binary.as_ptr(),
+            size: binary.len(),
+        }
+    }
+
     /// Creates a `Binary` from `term`.
     ///
     /// # Errors
@@ -312,8 +328,11 @@ impl<'a> Binary<'a> {
         {
             return Err(Error::BadArg);
         }
+
+        let binary = unsafe { binary.assume_init() };
         Ok(Binary {
-            inner: unsafe { binary.assume_init() },
+            buf: binary.data,
+            size: binary.size,
             term,
         })
     }
@@ -326,7 +345,7 @@ impl<'a> Binary<'a> {
 
     /// Extracts a slice containing the entire binary.
     pub fn as_slice(&self) -> &'a [u8] {
-        unsafe { ::std::slice::from_raw_parts(self.inner.data, self.inner.size) }
+        unsafe { ::std::slice::from_raw_parts(self.buf, self.size) }
     }
 
     /// Returns a new view into the same binary.
@@ -339,10 +358,24 @@ impl<'a> Binary<'a> {
     /// If `offset + length` is out of bounds, an error will be returned.
     pub fn make_subbinary(&self, offset: usize, length: usize) -> NifResult<Binary<'a>> {
         let min_len = length.checked_add(offset);
-        if min_len.ok_or(Error::BadArg)? > self.inner.size {
+        if min_len.ok_or(Error::BadArg)? > self.size {
             return Err(Error::BadArg);
         }
 
+        Ok(unsafe { self.make_subbinary_unchecked(offset, length) })
+    }
+
+    /// Returns a new view into the same binary.
+    ///
+    /// This method is an unsafe variant of `Binary::make_subbinary` in that it does not check for
+    /// `offset + length < self.len()` and always returns a `Binary`.
+    ///
+    /// # Safety
+    ///
+    /// If `offset + length` is out of bounds, this call results in *undefined behavior*. The
+    /// caller has to ensure that `offset + length < self.len()`.
+    #[allow(unused_unsafe)]
+    pub unsafe fn make_subbinary_unchecked(&self, offset: usize, length: usize) -> Binary<'a> {
         let raw_term = unsafe {
             rustler_sys::enif_make_sub_binary(
                 self.term.get_env().as_c_arg(),
@@ -352,8 +385,12 @@ impl<'a> Binary<'a> {
             )
         };
         let term = unsafe { Term::new(self.term.get_env(), raw_term) };
-        // This should never fail, as we are always passing in a binary term.
-        Ok(Binary::from_term(term).ok().unwrap())
+
+        Binary {
+            buf: unsafe { self.buf.add(offset) },
+            size: length,
+            term,
+        }
     }
 }
 

rustler_tests/lib/rustler_test.ex

@@ -38,6 +38,9 @@ defmodule RustlerTest do
   def resource_make_immutable(_), do: err()
   def resource_immutable_count(), do: err()
 
+  def resource_make_with_binaries(), do: err()
+  def resource_make_binaries(_), do: err()
+
   def make_shorter_subbinary(_), do: err()
   def parse_integer(_), do: err()
   def binary_new(), do: err()

rustler_tests/native/rustler_test/src/lib.rs

@@ -38,6 +38,8 @@ rustler::init!(
         test_resource::resource_get_integer_field,
         test_resource::resource_make_immutable,
         test_resource::resource_immutable_count,
+        test_resource::resource_make_with_binaries,
+        test_resource::resource_make_binaries,
         test_atom::atom_to_string,
         test_atom::atom_equals_ok,
         test_atom::binary_to_atom,

rustler_tests/native/rustler_test/src/test_resource.rs

@@ -1,4 +1,4 @@
-use rustler::{Env, ResourceArc};
+use rustler::{Binary, Env, ResourceArc};
 use std::sync::RwLock;
 
 pub struct TestResource {
@@ -12,9 +12,15 @@ pub struct ImmutableResource {
     b: u32,
 }
 
+pub struct WithBinaries {
+    a: [u8; 10],
+    b: Vec<u8>,
+}
+
 pub fn on_load(env: Env) -> bool {
     rustler::resource!(TestResource, env);
     rustler::resource!(ImmutableResource, env);
+    rustler::resource!(WithBinaries, env);
     true
 }
 
@@ -42,6 +48,7 @@ use std::sync::atomic::{AtomicUsize, Ordering};
 
 lazy_static::lazy_static! {
     static ref COUNT: AtomicUsize = AtomicUsize::new(0);
+    static ref STATIC_BIN: [u8; 10] = [0,1,2,3,4,5,6,7,8,9];
 }
 
 impl ImmutableResource {
@@ -69,3 +76,35 @@ pub fn resource_make_immutable(u: u32) -> ResourceArc<ImmutableResource> {
 pub fn resource_immutable_count() -> u32 {
     COUNT.load(Ordering::SeqCst) as u32
 }
+
+#[rustler::nif]
+pub fn resource_make_with_binaries() -> ResourceArc<WithBinaries> {
+    ResourceArc::new(WithBinaries {
+        a: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9],
+        b: vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9],
+    })
+}
+
+#[rustler::nif]
+pub fn resource_make_binaries(
+    env: Env,
+    resource: ResourceArc<WithBinaries>,
+) -> (Binary, Binary, Binary) {
+    // This won't compile:
+    // let temp = [1,2,3];
+    // resource.make_binary(env, |_w| &temp)
+
+    (
+        // From slice (embedded in T)
+        resource.make_binary(env, |w| &w.a),
+        // From vec (on the heap)
+        resource.make_binary(env, |w| &w.b),
+        // From static
+        resource.make_binary(env, |_| &*STATIC_BIN),
+    )
+}
+
+#[rustler::nif]
+pub fn resource_make_binary_from_vec(env: Env, resource: ResourceArc<WithBinaries>) -> Binary {
+    resource.make_binary(env, |w| &w.b)
+}

rustler_tests/test/resource_test.exs

@@ -35,4 +35,13 @@ defmodule RustlerTest.ResourceTest do
     # Erlang's exact GC should have cleaned all that up.
     assert RustlerTest.resource_immutable_count() == 0
   end
+
+  test "resource binaries" do
+    res = RustlerTest.resource_make_with_binaries()
+
+    {slice, vec, static} = RustlerTest.resource_make_binaries(res)
+
+    assert slice == vec
+    assert vec == static
+  end
 end