Skip to content
This repository has been archived by the owner on Dec 29, 2023. It is now read-only.

resource mutability #11

Open
goertzenator opened this issue Jun 23, 2016 · 3 comments
Open

resource mutability #11

goertzenator opened this issue Jun 23, 2016 · 3 comments

Comments

@goertzenator
Copy link
Collaborator

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?

@hansihe
Copy link
Member

hansihe commented Jun 23, 2016

Seems like a sensible change.

It might cause breakage, but it would be trivial to fix.

@jorendorff
Copy link
Contributor

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.

@goertzenator
Copy link
Collaborator Author

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants