diff --git a/CHANGELOG.md b/CHANGELOG.md index c3e7acae..c459f3bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,12 +12,19 @@ versions. ### Added -- Resource types can now be registered implicitly via a `#[derive(Resource)]` - instead of explicit registration in a `load` function using - `rustler::resource!` (#617) +- Resource type registration has been refactored to eventually remove the + `rustler::resource!` macro (#617, necessary due to a pending deprecation of a + Rust feature, #606) +- Resources can now explicitly implement the new `Resource` trait and provide a + custom `destructor` function that is run before `drop` and receives an `Env` + parameter (#617) +- Process monitoring via resources can now be used on resource types that + implement the `MonitorResource` trait (#617) ### Fixed +- Unwinding in the `on_load` callback is now caught and leads to a panic (#617) + ### Changed - NIF implementations are now discovered automatically and the respective diff --git a/rustler/src/codegen_runtime.rs b/rustler/src/codegen_runtime.rs index 750d5d78..725f42c3 100644 --- a/rustler/src/codegen_runtime.rs +++ b/rustler/src/codegen_runtime.rs @@ -6,7 +6,6 @@ use std::fmt; use crate::{Encoder, Env, OwnedBinary, Term}; // Re-export of inventory -pub use crate::resource::Registration; pub use inventory; // Names used by the `rustler::init!` macro or other generated code. diff --git a/rustler/src/env.rs b/rustler/src/env.rs index 1d2490d1..acec2c25 100644 --- a/rustler/src/env.rs +++ b/rustler/src/env.rs @@ -55,6 +55,13 @@ impl<'a> Env<'a> { } } + #[doc(hidden)] + pub unsafe fn new_init_env(_lifetime_marker: &'a T, env: NIF_ENV) -> Env<'a> { + let mut res = Self::new(_lifetime_marker, env); + res.init = true; + res + } + pub fn as_c_arg(self) -> NIF_ENV { self.env } diff --git a/rustler/src/lib.rs b/rustler/src/lib.rs index 63a8e8b8..a7aea28b 100644 --- a/rustler/src/lib.rs +++ b/rustler/src/lib.rs @@ -46,7 +46,7 @@ pub use crate::types::{ pub use crate::types::BigInt; mod resource; -pub use crate::resource::{Monitor, MonitorResource, Resource, ResourceArc, ResourceInitError}; +pub use crate::resource::{Monitor, Resource, ResourceArc, ResourceInitError}; #[doc(hidden)] pub mod dynamic; diff --git a/rustler/src/resource/handle.rs b/rustler/src/resource/handle.rs index dccdeaa1..e9d04344 100644 --- a/rustler/src/resource/handle.rs +++ b/rustler/src/resource/handle.rs @@ -5,10 +5,7 @@ use std::ptr; use rustler_sys::{c_void, ErlNifEnv}; use crate::thread::is_scheduler_thread; -use crate::{ - Binary, Decoder, Encoder, Env, Error, LocalPid, Monitor, MonitorResource, NifResult, OwnedEnv, - Term, -}; +use crate::{Binary, Decoder, Encoder, Env, Error, LocalPid, Monitor, NifResult, OwnedEnv, Term}; use super::traits::{Resource, ResourceExt}; use super::util::{align_alloced_mem_for_struct, get_alloc_size_struct}; @@ -120,9 +117,13 @@ where impl ResourceArc where - T: MonitorResource, + T: Resource, { pub fn monitor(&self, caller_env: Option<&Env>, pid: &LocalPid) -> Option { + if !T::IMPLEMENTS_DOWN { + return None; + } + let env = maybe_env(caller_env); let mut mon = MaybeUninit::uninit(); let res = unsafe { @@ -136,13 +137,17 @@ where } pub fn demonitor(&self, caller_env: Option<&Env>, mon: &Monitor) -> bool { + if !T::IMPLEMENTS_DOWN { + return false; + } + let env = maybe_env(caller_env); unsafe { rustler_sys::enif_demonitor_process(env, self.raw, mon.as_c_arg()) == 0 } } } impl OwnedEnv { - pub fn monitor( + pub fn monitor( &self, resource: &ResourceArc, pid: &LocalPid, @@ -150,13 +155,13 @@ impl OwnedEnv { resource.monitor(None, pid) } - pub fn demonitor(&self, resource: &ResourceArc, mon: &Monitor) -> bool { + pub fn demonitor(&self, resource: &ResourceArc, mon: &Monitor) -> bool { resource.demonitor(None, mon) } } impl<'a> Env<'a> { - pub fn monitor( + pub fn monitor( &self, resource: &ResourceArc, pid: &LocalPid, @@ -164,7 +169,7 @@ impl<'a> Env<'a> { resource.monitor(Some(self), pid) } - pub fn demonitor(&self, resource: &ResourceArc, mon: &Monitor) -> bool { + pub fn demonitor(&self, resource: &ResourceArc, mon: &Monitor) -> bool { resource.demonitor(Some(self), mon) } } diff --git a/rustler/src/resource/init_env.rs b/rustler/src/resource/init_env.rs deleted file mode 100644 index f0d56d2a..00000000 --- a/rustler/src/resource/init_env.rs +++ /dev/null @@ -1,27 +0,0 @@ -use crate::{Env, MonitorResource, Resource}; - -use super::{Registration, ResourceInitError}; - -impl<'a> Env<'a> { - pub unsafe fn to_init_env(&mut self) { - self.init = true; - } - - pub fn add_resource_type(self) -> Result<(), ResourceInitError> { - if !self.init { - return Err(ResourceInitError); - } - - Registration::new::().register(self) - } - - pub fn add_monitor_resource_type(self) -> Result<(), ResourceInitError> { - if !self.init { - return Err(ResourceInitError); - } - - Registration::new::() - .add_down_callback::() - .register(self) - } -} diff --git a/rustler/src/resource/mod.rs b/rustler/src/resource/mod.rs index 582dd77f..5ace000f 100644 --- a/rustler/src/resource/mod.rs +++ b/rustler/src/resource/mod.rs @@ -5,7 +5,6 @@ //! more references to the resource. mod handle; -mod init_env; mod monitor; mod registration; mod traits; @@ -17,10 +16,9 @@ use super::{Decoder, Error, NifResult, Term}; pub use handle::ResourceArc; pub use monitor::Monitor; -pub use registration::Registration; use rustler_sys::c_void; +pub use traits::Resource; use traits::ResourceExt; -pub use traits::{MonitorResource, Resource}; use util::align_alloced_mem_for_struct; impl<'a> Term<'a> { @@ -68,6 +66,6 @@ pub struct ResourceInitError; macro_rules! resource { ($struct_name:ty, $env: ident) => {{ impl $crate::Resource for $struct_name {} - $env.add_resource_type::<$struct_name>().is_ok() + $env.register::<$struct_name>().is_ok() }}; } diff --git a/rustler/src/resource/registration.rs b/rustler/src/resource/registration.rs index bdae510e..0e107af2 100644 --- a/rustler/src/resource/registration.rs +++ b/rustler/src/resource/registration.rs @@ -1,9 +1,11 @@ +use super::traits; use super::util::align_alloced_mem_for_struct; -use super::{traits, ResourceInitError}; -use crate::{Env, LocalPid, Monitor, MonitorResource, Resource}; +use super::ResourceInitError; +use crate::{Env, LocalPid, Monitor, Resource}; +use rustler_sys::ErlNifResourceDtor; use rustler_sys::{ - c_char, c_void, ErlNifEnv, ErlNifMonitor, ErlNifPid, ErlNifResourceDown, ErlNifResourceDtor, - ErlNifResourceFlags, ErlNifResourceType, ErlNifResourceTypeInit, + c_char, c_void, ErlNifEnv, ErlNifMonitor, ErlNifPid, ErlNifResourceDown, ErlNifResourceFlags, + ErlNifResourceType, ErlNifResourceTypeInit, }; use std::any::TypeId; use std::ffi::CString; @@ -11,39 +13,70 @@ use std::mem::MaybeUninit; use std::ptr; #[derive(Debug)] -pub struct Registration { +struct Registration { get_type_id: fn() -> TypeId, get_type_name: fn() -> &'static str, init: ErlNifResourceTypeInit, } +impl<'a> Env<'a> { + pub fn register(&self) -> Result<(), ResourceInitError> { + Registration::new::().register(*self) + } +} + impl Registration { pub const fn new() -> Self { - let init = ErlNifResourceTypeInit { - dtor: resource_destructor:: as *const ErlNifResourceDtor, - stop: ptr::null(), - down: ptr::null(), - members: 1, - dyncall: ptr::null(), - }; Self { - init, + init: ErlNifResourceTypeInit { + dtor: ptr::null(), + stop: ptr::null(), + down: ptr::null(), + members: 0, + dyncall: ptr::null(), + }, get_type_name: std::any::type_name::, get_type_id: TypeId::of::, } + .maybe_add_destructor_callback::() + .maybe_add_down_callback::() } - pub const fn add_down_callback(self) -> Self { - Self { - init: ErlNifResourceTypeInit { - down: resource_down:: as *const ErlNifResourceDown, - ..self.init - }, - ..self + const fn maybe_add_destructor_callback(self) -> Self { + if T::IMPLEMENTS_DESTRUCTOR || std::mem::needs_drop::() { + Self { + init: ErlNifResourceTypeInit { + dtor: resource_destructor:: as *const ErlNifResourceDtor, + members: max(self.init.members, 1), + ..self.init + }, + ..self + } + } else { + self + } + } + + const fn maybe_add_down_callback(self) -> Self { + if T::IMPLEMENTS_DOWN { + Self { + init: ErlNifResourceTypeInit { + down: resource_down:: as *const ErlNifResourceDown, + members: max(self.init.members, 3), + ..self.init + }, + ..self + } + } else { + self } } pub fn register(&self, env: Env) -> Result<(), ResourceInitError> { + if !env.init { + return Err(ResourceInitError); + } + let type_id = (self.get_type_id)(); let type_name = (self.get_type_name)(); @@ -76,7 +109,7 @@ where ptr::read::(aligned as *mut T).destructor(env); } -unsafe extern "C" fn resource_down( +unsafe extern "C" fn resource_down( env: *mut ErlNifEnv, obj: *mut c_void, pid: *const ErlNifPid, @@ -113,3 +146,11 @@ pub unsafe fn open_resource_type( Some(res) } } + +const fn max(i: i32, j: i32) -> i32 { + if i > j { + i + } else { + j + } +} diff --git a/rustler/src/resource/traits.rs b/rustler/src/resource/traits.rs index db81fb7d..aa087a5e 100644 --- a/rustler/src/resource/traits.rs +++ b/rustler/src/resource/traits.rs @@ -17,12 +17,15 @@ pub(crate) unsafe fn register_resource_type(type_id: TypeId, resource_type: NifR } pub trait Resource: Sized + Send + Sync + 'static { - #[allow(unused_mut)] - fn destructor(mut self, _env: Env<'_>) {} -} + const IMPLEMENTS_DESTRUCTOR: bool = false; + const IMPLEMENTS_DOWN: bool = false; + + /// Callback function that is executed right before dropping a resource object. + #[allow(unused_mut, unused)] + fn destructor(mut self, env: Env<'_>) {} -pub trait MonitorResource: Resource { - fn down<'a>(&'a self, env: Env<'a>, pid: LocalPid, monitor: Monitor); + #[allow(unused)] + fn down<'a>(&'a self, env: Env<'a>, pid: LocalPid, monitor: Monitor) {} } #[doc(hidden)] diff --git a/rustler_codegen/src/init.rs b/rustler_codegen/src/init.rs index 53f64187..6d11ca5e 100644 --- a/rustler_codegen/src/init.rs +++ b/rustler_codegen/src/init.rs @@ -103,10 +103,9 @@ impl From for proc_macro2::TokenStream { load_info: rustler::codegen_runtime::NIF_TERM ) -> rustler::codegen_runtime::c_int { unsafe { - let mut env = rustler::Env::new(&env, env); + let mut env = rustler::Env::new_init_env(&env, env); // TODO: If an unwrap ever happens, we will unwind right into C! Fix this! let load_info = rustler::Term::new(env, load_info); - env.to_init_env(); #load.map_or(0, |inner| { rustler::codegen_runtime::handle_nif_init_call( inner, env, load_info diff --git a/rustler_tests/native/rustler_test/src/test_resource.rs b/rustler_tests/native/rustler_test/src/test_resource.rs index 8af8f4e2..2ede9954 100644 --- a/rustler_tests/native/rustler_test/src/test_resource.rs +++ b/rustler_tests/native/rustler_test/src/test_resource.rs @@ -1,4 +1,4 @@ -use rustler::{Binary, Env, LocalPid, Monitor, MonitorResource, Resource, ResourceArc}; +use rustler::{Binary, Env, LocalPid, Monitor, Resource, ResourceArc}; use std::sync::{Mutex, OnceLock, RwLock}; pub struct TestResource { @@ -14,9 +14,9 @@ pub struct TestMonitorResource { inner: Mutex, } -impl Resource for TestMonitorResource {} +impl Resource for TestMonitorResource { + const IMPLEMENTS_DOWN: bool = true; -impl MonitorResource for TestMonitorResource { fn down<'a>(&'a self, _env: Env<'a>, _pid: LocalPid, mon: Monitor) { let mut inner = self.inner.lock().unwrap(); assert!(Some(mon) == inner.mon); @@ -41,10 +41,8 @@ pub struct WithBinaries { pub fn on_load(env: Env) -> bool { rustler::resource!(TestResource, env) - && env.add_resource_type::().is_ok() - && env - .add_monitor_resource_type::() - .is_ok() + && env.register::().is_ok() + && env.register::().is_ok() && rustler::resource!(WithBinaries, env) }