erlang_nif-sys
erlang_nif-sys copied to clipboard
resource mutability
While working on Ruster
, I've discovered that the resource functions in erlang_nif-sys
may have the wrong mutability:
fn enif_alloc_resource(_type: *mut ErlNifResourceType, size: size_t) -> *mut c_void
fn enif_release_resource(obj: *mut c_void)
fn enif_get_resource(arg1: *mut ErlNifEnv, term: ERL_NIF_TERM, _type: *mut ErlNifResourceType, objp: *mut *mut c_void) -> c_int
fn enif_keep_resource(obj: *mut c_void)
I think the object pointers in all except enif_alloc_resource
should be changed to *const c_void
. Consider the scenario where two Erlang processes have the same resource term and simultaneously call Nifs that manipulate that resource. Having a mutable pointer from enif_get_resource
will enable concurrent mutation (bad!). The user can be steered in a safer direction by providing a const pointer. Interior mutability can still be achieved with Cell
, RefCell
, and locks.
The underlying C NIF API is compelled to use mutable pointers here because resource refcounts need to be mutated. C can't hide these things under interior mutability while staying exteriorly immutable. With this Rust wrapper we have an opportunity to hide such implementation details.
@hansihe, would this change cause breakage for Rustler
?
Seems like a sensible change.
It might cause breakage, but it would be trivial to fix.
This change was made in 0.5 (rev f801f2e), so this can be closed.
I was surprised when I saw this discrepancy between the C and Rust function signatures — it's neat to see the reasoning behind it. ...Unfortunately, I don't see a good way to communicate this to users. Putting it in a doc comment wouldn't help, because users quickly learn not to look at the rustdoc, but to look at the erl_nif
docs instead.
Oops, thanks. But maybe I'll leave this open as a reminder to eventually write a paragraph about it in docs. It could be part of a larger discussion on mutability, such as why environments are always *mut (because they are not thread safe) and other things.