Skip to content

Implement ResourceArc::make_binary #487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Sep 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 48 additions & 14 deletions rustler/src/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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>),
Expand Down Expand Up @@ -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) };
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
53 changes: 45 additions & 8 deletions rustler/src/types/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,8 @@ unsafe impl Send 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>,
}

Expand All @@ -256,7 +257,8 @@ impl<'a> Binary<'a> {
)
};
Binary {
inner: owned.0,
buf: owned.0.data,
size: owned.0.size,
term,
}
}
Expand Down Expand Up @@ -288,12 +290,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
Expand All @@ -311,8 +327,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,
})
}
Expand All @@ -325,7 +344,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.
Expand All @@ -338,10 +357,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(),
Expand All @@ -351,8 +384,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,
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions rustler_tests/lib/rustler_test.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions rustler_tests/native/rustler_test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
41 changes: 40 additions & 1 deletion rustler_tests/native/rustler_test/src/test_resource.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rustler::{Env, ResourceArc};
use rustler::{Binary, Env, ResourceArc};
use std::sync::RwLock;

pub struct TestResource {
Expand All @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
9 changes: 9 additions & 0 deletions rustler_tests/test/resource_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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