Skip to content

Commit

Permalink
Last refactoring step
Browse files Browse the repository at this point in the history
- Drop `MonitorResource` trait again, since Rust's type system is
  currently not capable of expressing the sort of required
  specialization to make using a sub-trait really ergonomic
- Additional resource callbacks have to be implemented and an associated
  `const` has to be set
  • Loading branch information
filmor committed Jul 4, 2024
1 parent c4b94b2 commit 2a987fa
Show file tree
Hide file tree
Showing 11 changed files with 110 additions and 80 deletions.
13 changes: 10 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion rustler/src/codegen_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions rustler/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ impl<'a> Env<'a> {
}
}

#[doc(hidden)]
pub unsafe fn new_init_env<T>(_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
}
Expand Down
2 changes: 1 addition & 1 deletion rustler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
23 changes: 14 additions & 9 deletions rustler/src/resource/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -120,9 +117,13 @@ where

impl<T> ResourceArc<T>
where
T: MonitorResource,
T: Resource,
{
pub fn monitor(&self, caller_env: Option<&Env>, pid: &LocalPid) -> Option<Monitor> {
if !T::IMPLEMENTS_DOWN {
return None;
}

let env = maybe_env(caller_env);
let mut mon = MaybeUninit::uninit();
let res = unsafe {
Expand All @@ -136,35 +137,39 @@ 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<T: MonitorResource>(
pub fn monitor<T: Resource>(
&self,
resource: &ResourceArc<T>,
pid: &LocalPid,
) -> Option<Monitor> {
resource.monitor(None, pid)
}

pub fn demonitor<T: MonitorResource>(&self, resource: &ResourceArc<T>, mon: &Monitor) -> bool {
pub fn demonitor<T: Resource>(&self, resource: &ResourceArc<T>, mon: &Monitor) -> bool {
resource.demonitor(None, mon)
}
}

impl<'a> Env<'a> {
pub fn monitor<T: MonitorResource>(
pub fn monitor<T: Resource>(
&self,
resource: &ResourceArc<T>,
pid: &LocalPid,
) -> Option<Monitor> {
resource.monitor(Some(self), pid)
}

pub fn demonitor<T: MonitorResource>(&self, resource: &ResourceArc<T>, mon: &Monitor) -> bool {
pub fn demonitor<T: Resource>(&self, resource: &ResourceArc<T>, mon: &Monitor) -> bool {
resource.demonitor(Some(self), mon)
}
}
Expand Down
27 changes: 0 additions & 27 deletions rustler/src/resource/init_env.rs

This file was deleted.

6 changes: 2 additions & 4 deletions rustler/src/resource/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
//! more references to the resource.

mod handle;
mod init_env;
mod monitor;
mod registration;
mod traits;
Expand All @@ -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> {
Expand Down Expand Up @@ -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()
}};
}
83 changes: 62 additions & 21 deletions rustler/src/resource/registration.rs
Original file line number Diff line number Diff line change
@@ -1,49 +1,82 @@
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;
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<T: Resource>(&self) -> Result<(), ResourceInitError> {
Registration::new::<T>().register(*self)
}
}

impl Registration {
pub const fn new<T: Resource>() -> Self {
let init = ErlNifResourceTypeInit {
dtor: resource_destructor::<T> 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::<T>,
get_type_id: TypeId::of::<T>,
}
.maybe_add_destructor_callback::<T>()
.maybe_add_down_callback::<T>()
}

pub const fn add_down_callback<T: MonitorResource>(self) -> Self {
Self {
init: ErlNifResourceTypeInit {
down: resource_down::<T> as *const ErlNifResourceDown,
..self.init
},
..self
const fn maybe_add_destructor_callback<T: Resource>(self) -> Self {
if T::IMPLEMENTS_DESTRUCTOR || std::mem::needs_drop::<T>() {
Self {
init: ErlNifResourceTypeInit {
dtor: resource_destructor::<T> as *const ErlNifResourceDtor,
members: max(self.init.members, 1),
..self.init
},
..self
}
} else {
self
}
}

const fn maybe_add_down_callback<T: Resource>(self) -> Self {
if T::IMPLEMENTS_DOWN {
Self {
init: ErlNifResourceTypeInit {
down: resource_down::<T> 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)();

Expand Down Expand Up @@ -76,7 +109,7 @@ where
ptr::read::<T>(aligned as *mut T).destructor(env);
}

unsafe extern "C" fn resource_down<T: MonitorResource>(
unsafe extern "C" fn resource_down<T: Resource>(
env: *mut ErlNifEnv,
obj: *mut c_void,
pid: *const ErlNifPid,
Expand Down Expand Up @@ -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
}
}
13 changes: 8 additions & 5 deletions rustler/src/resource/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
3 changes: 1 addition & 2 deletions rustler_codegen/src/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,9 @@ impl From<InitMacroInput> 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
Expand Down
12 changes: 5 additions & 7 deletions 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::{Binary, Env, LocalPid, Monitor, MonitorResource, Resource, ResourceArc};
use rustler::{Binary, Env, LocalPid, Monitor, Resource, ResourceArc};
use std::sync::{Mutex, OnceLock, RwLock};

pub struct TestResource {
Expand All @@ -14,9 +14,9 @@ pub struct TestMonitorResource {
inner: Mutex<TestMonitorResourceInner>,
}

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);
Expand All @@ -41,10 +41,8 @@ pub struct WithBinaries {

pub fn on_load(env: Env) -> bool {
rustler::resource!(TestResource, env)
&& env.add_resource_type::<ImmutableResource>().is_ok()
&& env
.add_monitor_resource_type::<TestMonitorResource>()
.is_ok()
&& env.register::<ImmutableResource>().is_ok()
&& env.register::<TestMonitorResource>().is_ok()
&& rustler::resource!(WithBinaries, env)
}

Expand Down

0 comments on commit 2a987fa

Please sign in to comment.