From e8881a300fbeff8ed0f2f8a9f6bb1dcea89b018a Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Mon, 3 Jun 2024 07:01:23 +0200 Subject: [PATCH] Refactor resources - Move storage of "Type->NIF Resource Type Handle" to a OnceLock map to allow implementing resource types without resorting to dynamic trait implementations - Implement resource types as a `derive` macro - Add direct access methods to get an immutable reference to the wrapped objects - Add corresponding converters --- rustler/src/codegen_runtime.rs | 13 +- rustler/src/lib.rs | 2 +- rustler/src/resource.rs | 224 ++++++++++-------- rustler/src/wrapper/resource.rs | 4 - rustler_codegen/Cargo.toml | 2 +- rustler_codegen/src/init.rs | 7 +- rustler_codegen/src/lib.rs | 7 + rustler_codegen/src/resource.rs | 19 ++ .../native/rustler_test/src/test_resource.rs | 6 +- 9 files changed, 162 insertions(+), 122 deletions(-) create mode 100644 rustler_codegen/src/resource.rs diff --git a/rustler/src/codegen_runtime.rs b/rustler/src/codegen_runtime.rs index 6e8754a9..027ba15b 100644 --- a/rustler/src/codegen_runtime.rs +++ b/rustler/src/codegen_runtime.rs @@ -100,15 +100,12 @@ impl fmt::Debug for NifReturned { /// # Unsafe /// /// This takes arguments, including raw pointers, that must be correct. -pub unsafe fn handle_nif_init_call( - function: Option fn(Env<'a>, Term<'a>) -> bool>, - r_env: NIF_ENV, - load_info: NIF_TERM, +pub unsafe fn handle_nif_init_call<'a>( + function: Option fn(Env<'b>, Term<'b>) -> bool>, + env: Env<'a>, + load_info: Term<'a>, ) -> c_int { - let env = Env::new(&(), r_env); - let term = Term::new(env, load_info); - - function.map_or(0, |inner| i32::from(!inner(env, term))) + function.map_or(0, |inner| i32::from(!inner(env, load_info))) } pub fn handle_nif_result( diff --git a/rustler/src/lib.rs b/rustler/src/lib.rs index 46a3ffc0..8166211f 100644 --- a/rustler/src/lib.rs +++ b/rustler/src/lib.rs @@ -74,7 +74,7 @@ pub type NifResult = Result; #[cfg(feature = "derive")] pub use rustler_codegen::{ init, nif, NifException, NifMap, NifRecord, NifStruct, NifTaggedEnum, NifTuple, NifUnitEnum, - NifUntaggedEnum, + NifUntaggedEnum, Resource, }; #[cfg(feature = "serde")] diff --git a/rustler/src/resource.rs b/rustler/src/resource.rs index fb442f5d..ee6ebff0 100644 --- a/rustler/src/resource.rs +++ b/rustler/src/resource.rs @@ -4,43 +4,102 @@ //! NIF calls. The struct will be automatically dropped when the BEAM GC decides that there are no //! more references to the resource. -use std::marker::PhantomData; -use std::mem; +use std::any::TypeId; +use std::collections::HashMap; use std::ops::Deref; use std::ptr; +use std::sync::OnceLock; +use std::{ffi::CString, mem}; use super::{Binary, Decoder, Encoder, Env, Error, NifResult, Term}; -use crate::wrapper::{ +use crate::resource::resource::open_resource_type; +pub use crate::wrapper::{ c_void, resource, NifResourceFlags, MUTABLE_NIF_RESOURCE_HANDLE, NIF_ENV, NIF_RESOURCE_TYPE, }; +#[derive(Debug)] +pub struct ResourceRegistration { + name: &'static str, + get_type_id: fn() -> TypeId, + destructor: unsafe extern "C" fn(_env: NIF_ENV, handle: MUTABLE_NIF_RESOURCE_HANDLE), +} +inventory::collect!(ResourceRegistration); + +static mut RESOURCE_TYPES: OnceLock> = OnceLock::new(); + +fn get_resource_type() -> Option { + let map = unsafe { RESOURCE_TYPES.get()? }; + map.get(&TypeId::of::()) + .map(|ptr| *ptr as NIF_RESOURCE_TYPE) +} + +impl ResourceRegistration { + pub const fn new(name: &'static str) -> Self { + Self { + name, + destructor: resource_destructor::, + get_type_id: TypeId::of::, + } + } + + pub fn initialize(env: Env) { + for reg in inventory::iter::() { + reg.register(env); + } + } + + pub fn register(&self, env: Env) { + let res: Option = unsafe { + open_resource_type( + env.as_c_arg(), + CString::new(self.name).unwrap().as_bytes_with_nul(), + Some(self.destructor), + NIF_RESOURCE_FLAGS::ERL_NIF_RT_CREATE, + ) + }; + + let type_id = (self.get_type_id)(); + + unsafe { + RESOURCE_TYPES.get_or_init(Default::default); + RESOURCE_TYPES + .get_mut() + .unwrap() + .insert(type_id, res.unwrap() as usize); + } + } +} + /// Re-export a type used by the `resource!` macro. #[doc(hidden)] pub use crate::wrapper::NIF_RESOURCE_FLAGS; -/// The ResourceType struct contains a NIF_RESOURCE_TYPE and a phantom reference to the type it -/// is for. It serves as a holder for the information needed to interact with the Erlang VM about -/// the resource type. -/// -/// This is usually stored in an implementation of ResourceTypeProvider. #[doc(hidden)] -pub struct ResourceType { - pub res: NIF_RESOURCE_TYPE, - pub struct_type: PhantomData, +pub trait ResourceType: Sized + Send + Sync + 'static { + fn get_resource_type() -> Option { + get_resource_type::() + } } -/// This trait gets implemented for the type we want to put into a resource when -/// resource! is called on it. It provides the ResourceType. -/// -/// In most cases the user should not have to worry about this. -#[doc(hidden)] -pub trait ResourceTypeProvider: Sized + Send + Sync + 'static { - fn get_type() -> &'static ResourceType; +impl<'a> Term<'a> { + unsafe fn get_resource_ptrs(&self) -> Option<(*const c_void, *mut T)> { + let typ = T::get_resource_type()?; + let res = resource::get_resource(self.get_env().as_c_arg(), self.as_c_arg(), typ)?; + Some((res, align_alloced_mem_for_struct::(res) as *mut T)) + } + + pub fn get_resource(&self) -> Option<&'a T> { + unsafe { self.get_resource_ptrs().map(|(_, ptr)| &*ptr) } + } + + pub unsafe fn get_mut_resource(&self) -> Option<&'a mut T> { + self.get_resource_ptrs().map(|(_, ptr)| &mut *ptr) + } } impl Encoder for ResourceArc where - T: ResourceTypeProvider, + T: ResourceType, { fn encode<'a>(&self, env: Env<'a>) -> Term<'a> { self.as_term(env) @@ -48,16 +107,28 @@ where } impl<'a, T> Decoder<'a> for ResourceArc where - T: ResourceTypeProvider + 'a, + T: ResourceType + 'a, { fn decode(term: Term<'a>) -> NifResult { ResourceArc::from_term(term) } } +impl<'a, T> Decoder<'a> for &'a T +where + T: ResourceType + 'a, +{ + fn decode(term: Term<'a>) -> NifResult { + term.get_resource().ok_or(Error::BadArg) + } +} + /// Drop a T that lives in an Erlang resource. (erlang_nif-sys requires us to declare this /// function safe, but it is of course thoroughly unsafe!) -extern "C" fn resource_destructor(_env: NIF_ENV, handle: MUTABLE_NIF_RESOURCE_HANDLE) { +pub unsafe extern "C" fn resource_destructor( + _env: NIF_ENV, + handle: MUTABLE_NIF_RESOURCE_HANDLE, +) { unsafe { let aligned = align_alloced_mem_for_struct::(handle); let res = aligned as *mut T; @@ -65,33 +136,6 @@ extern "C" fn resource_destructor(_env: NIF_ENV, handle: MUTABLE_NIF_RESOURCE } } -/// This is the function that gets called from resource! in on_load to create a new -/// resource type. -/// -/// # Panics -/// -/// Panics if `name` isn't null-terminated. -#[doc(hidden)] -pub fn open_struct_resource_type( - env: Env, - name: &str, - flags: NifResourceFlags, -) -> Option> { - let res: Option = unsafe { - resource::open_resource_type( - env.as_c_arg(), - name.as_bytes(), - Some(resource_destructor::), - flags, - ) - }; - - res.map(|r| ResourceType { - res: r, - struct_type: PhantomData, - }) -} - fn get_alloc_size_struct() -> usize { mem::size_of::() + mem::align_of::() } @@ -115,25 +159,26 @@ unsafe fn align_alloced_mem_for_struct(ptr: *const c_void) -> *const c_void { /// convert back and forth between the two using `Encoder` and `Decoder`. pub struct ResourceArc where - T: ResourceTypeProvider, + T: ResourceType, { raw: *const c_void, inner: *mut T, } // Safe because T is `Sync` and `Send`. -unsafe impl Send for ResourceArc where T: ResourceTypeProvider {} -unsafe impl Sync for ResourceArc where T: ResourceTypeProvider {} +unsafe impl Send for ResourceArc where T: ResourceType {} +unsafe impl Sync for ResourceArc where T: ResourceType {} impl ResourceArc where - T: ResourceTypeProvider, + T: ResourceType, { /// Makes a new ResourceArc from the given type. Note that the type must have - /// ResourceTypeProvider implemented for it. See module documentation for info on this. + /// ResourceType implemented for it. See module documentation for info on this. pub fn new(data: T) -> Self { let alloc_size = get_alloc_size_struct::(); - let mem_raw = unsafe { resource::alloc_resource(T::get_type().res, alloc_size) }; + let resource_type = T::get_resource_type().unwrap(); + let mem_raw = unsafe { rustler_sys::enif_alloc_resource(resource_type, alloc_size) }; let aligned_mem = unsafe { align_alloced_mem_for_struct::(mem_raw) as *mut T }; unsafe { ptr::write(aligned_mem, data) }; @@ -185,28 +230,18 @@ where } fn from_term(term: Term) -> Result { - let res_resource = match unsafe { - resource::get_resource( - term.get_env().as_c_arg(), - term.as_c_arg(), - T::get_type().res, - ) - } { - Some(res) => res, - None => return Err(Error::BadArg), - }; - unsafe { - resource::keep_resource(res_resource); - } - let casted_ptr = unsafe { align_alloced_mem_for_struct::(res_resource) as *mut T }; - Ok(ResourceArc { - raw: res_resource, - inner: casted_ptr, - }) + let (raw, inner) = unsafe { term.get_resource_ptrs::() }.ok_or(Error::BadArg)?; + unsafe { rustler_sys::enif_keep_resource(raw) }; + Ok(ResourceArc { raw, inner }) } fn as_term<'a>(&self, env: Env<'a>) -> Term<'a> { - unsafe { Term::new(env, resource::make_resource(env.as_c_arg(), self.raw)) } + unsafe { + Term::new( + env, + rustler_sys::enif_make_resource(env.as_c_arg(), self.raw), + ) + } } fn as_c_arg(&mut self) -> *const c_void { @@ -220,7 +255,7 @@ where impl Deref for ResourceArc where - T: ResourceTypeProvider, + T: ResourceType, { type Target = T; @@ -231,14 +266,12 @@ where impl Clone for ResourceArc where - T: ResourceTypeProvider, + T: ResourceType, { /// Cloning a `ResourceArc` simply increments the reference count for the /// resource. The `T` value is not cloned. fn clone(&self) -> Self { - unsafe { - resource::keep_resource(self.raw); - } + unsafe { rustler_sys::enif_keep_resource(self.raw) }; ResourceArc { raw: self.raw, inner: self.inner, @@ -248,7 +281,7 @@ where impl Drop for ResourceArc where - T: ResourceTypeProvider, + T: ResourceType, { /// When a `ResourceArc` is dropped, the reference count is decremented. If /// there are no other references to the resource, the `T` value is dropped. @@ -263,30 +296,11 @@ where #[macro_export] macro_rules! resource { - ($struct_name:ty, $env: ident) => { - { - static mut STRUCT_TYPE: Option<$crate::resource::ResourceType<$struct_name>> = None; - - let temp_struct_type = - match $crate::resource::open_struct_resource_type::<$struct_name>( - $env, - concat!(stringify!($struct_name), "\x00"), - $crate::resource::NIF_RESOURCE_FLAGS::ERL_NIF_RT_CREATE - ) { - Some(inner) => inner, - None => { - println!("Failure in creating resource type"); - return false; - } - }; - unsafe { STRUCT_TYPE = Some(temp_struct_type) }; - - impl $crate::resource::ResourceTypeProvider for $struct_name { - fn get_type() -> &'static $crate::resource::ResourceType { - unsafe { &STRUCT_TYPE }.as_ref() - .expect("The resource type hasn't been initialized. Did you remember to call the function where you used the `resource!` macro?") - } - } - } - } + ($struct_name:ty, $env: ident) => {{ + impl $crate::resource::ResourceType for $struct_name {} + + let tuple = rustler::resource::ResourceRegistration::new::<$struct_name>( + stringify!(#name) + ).register($env); + }}; } diff --git a/rustler/src/wrapper/resource.rs b/rustler/src/wrapper/resource.rs index 8b9cdad4..2975b966 100644 --- a/rustler/src/wrapper/resource.rs +++ b/rustler/src/wrapper/resource.rs @@ -3,10 +3,6 @@ use crate::wrapper::{ }; use rustler_sys::c_char; -pub use rustler_sys::{ - enif_alloc_resource as alloc_resource, enif_keep_resource as keep_resource, - enif_make_resource as make_resource, enif_release_resource as release_resource, -}; use std::mem::MaybeUninit; use std::ptr; diff --git a/rustler_codegen/Cargo.toml b/rustler_codegen/Cargo.toml index e7d38940..b440609b 100644 --- a/rustler_codegen/Cargo.toml +++ b/rustler_codegen/Cargo.toml @@ -10,7 +10,7 @@ edition = "2021" [lib] name = "rustler_codegen" -proc_macro = true +proc-macro = true [dependencies] syn = { version = "2.0", features = ["full", "extra-traits"] } diff --git a/rustler_codegen/src/init.rs b/rustler_codegen/src/init.rs index 5bd15c35..cfc803c2 100644 --- a/rustler_codegen/src/init.rs +++ b/rustler_codegen/src/init.rs @@ -103,8 +103,13 @@ impl From for proc_macro2::TokenStream { load_info: rustler::codegen_runtime::NIF_TERM ) -> rustler::codegen_runtime::c_int { unsafe { + let env = rustler::Env::new(&env, env); + rustler::resource::ResourceRegistration::initialize(env); // TODO: If an unwrap ever happens, we will unwind right into C! Fix this! - rustler::codegen_runtime::handle_nif_init_call(#load, env, load_info) + let load_info = rustler::Term::new(env, load_info); + rustler::codegen_runtime::handle_nif_init_call( + #load, env, load_info + ) } } Some(nif_load) diff --git a/rustler_codegen/src/lib.rs b/rustler_codegen/src/lib.rs index 8bace2f9..90af3dac 100644 --- a/rustler_codegen/src/lib.rs +++ b/rustler_codegen/src/lib.rs @@ -9,6 +9,7 @@ mod init; mod map; mod nif; mod record; +mod resource; mod tagged_enum; mod tuple; mod unit_enum; @@ -400,3 +401,9 @@ pub fn nif_untagged_enum(input: TokenStream) -> TokenStream { let ast = syn::parse(input).unwrap(); untagged_enum::transcoder_decorator(&ast).into() } + +#[proc_macro_derive(Resource)] +pub fn resource_derive(input: TokenStream) -> TokenStream { + let ast = syn::parse(input).unwrap(); + resource::transcoder_decorator(&ast).into() +} diff --git a/rustler_codegen/src/resource.rs b/rustler_codegen/src/resource.rs new file mode 100644 index 00000000..d87c1297 --- /dev/null +++ b/rustler_codegen/src/resource.rs @@ -0,0 +1,19 @@ +use proc_macro2::TokenStream; +use quote::quote; + +use super::context::Context; + +pub fn transcoder_decorator(ast: &syn::DeriveInput) -> TokenStream { + let ctx = Context::from_ast(ast); + let name = ctx.ident; + let name_s = name.to_string(); + + quote! { + impl rustler::resource::ResourceType for #name {} + rustler::codegen_runtime::inventory::submit!( + rustler::resource::ResourceRegistration::new::<#name>( + #name_s + ) + ); + } +} diff --git a/rustler_tests/native/rustler_test/src/test_resource.rs b/rustler_tests/native/rustler_test/src/test_resource.rs index a2e57d34..3bea25d3 100644 --- a/rustler_tests/native/rustler_test/src/test_resource.rs +++ b/rustler_tests/native/rustler_test/src/test_resource.rs @@ -1,12 +1,14 @@ -use rustler::{Binary, Env, ResourceArc}; +use rustler::{Binary, Env, Resource, ResourceArc}; use std::sync::{OnceLock, RwLock}; +// #[derive(Resource)] pub struct TestResource { test_field: RwLock, } /// This one is designed to look more like pointer data, to increase the /// chance of segfaults if the implementation is wrong. +#[derive(Debug, Resource)] pub struct ImmutableResource { a: u32, b: u32, @@ -19,7 +21,7 @@ pub struct WithBinaries { pub fn on_load(env: Env) -> bool { rustler::resource!(TestResource, env); - rustler::resource!(ImmutableResource, env); + // rustler::resource!(ImmutableResource, env); rustler::resource!(WithBinaries, env); true }