Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow users to set a custom resource destructor #625

Closed
wants to merge 2 commits into from

Conversation

cocoa-xu
Copy link
Contributor

Hi this PR adds an arm to the rustler::resource! macro which users to set a custom resource destructor, and also makes rustler::resource::align_alloced_mem_for_struct public so that user can use it in these customised resource destructors.

This should make things easier when a custom resource destructor is needed.

@filmor
Copy link
Member

filmor commented Jun 27, 2024

Why would you want this? If you need a particular destructor behaviour, you should implement Drop. The concrete stored data is an implementation detail, I'd really not want to expose it.

@cocoa-xu
Copy link
Contributor Author

cocoa-xu commented Jun 27, 2024

Hi @filmor, I was trying to send a message to an Erlang process when the resource is GC'ed in the Erlang world, and doing that in the resource dtor ensures the message will be sent at the exact time Erlang releases the resource instead of the time the value is being GC'ed/dropped in Rust -- as it was noted in impl<T> Drop for ResourceArc<T>

    /// However, note that in general, the Rust value in a resource is dropped
    /// at an unpredictable time: whenever the VM decides to do garbage
    /// collection.

@filmor
Copy link
Member

filmor commented Jun 28, 2024

You are misunderstanding the comment. Rust's drop is called from the resource destructor, but the time when this one is called is decided by the Erlang GC.

@filmor
Copy link
Member

filmor commented Jun 28, 2024

But one point here still stands: It's currently not possible to use the NIF env that is passed to the resource destructor from Rust, see https://github.com/rusterlium/rustler/blob/master/rustler/src/resource.rs#L60-L66

I can add that as an additional optional function in the refactoring (#617).

@cocoa-xu
Copy link
Contributor Author

cocoa-xu commented Jun 28, 2024

You are misunderstanding the comment. Rust's drop is called from the resource destructor, but the time when this one is called is decided by the Erlang GC.

Oh I see! Thanks for the clarification!

So I'm trying to implement a NIF in Rust but I have serval issues/questions when using rustler. I would greatly appreciate any suggestions anyone may have for me as I don't quite think that I'm the right track for this...

Related PR that attempts to do it in Rust elixir-explorer/explorer#931. Currently it's implemented in C here in elixir-explorer/explorer#930.

Some background on this

I'm writing a message_on_gc function which is a NIF that receives a process pid and a term message. And it will return a reference representing a NIF resource. Once the resource is GCed, it should send the term message it received earlier to the given process pid.

Core lines from the C implementation:

C Code
static void *message_resource;
struct message {
  ErlNifEnv *env;
  ErlNifPid pid;
  ERL_NIF_TERM value;
};

void destruct_local_message(ErlNifEnv *env, void *obj) {
  struct message *m = (struct message *)obj;
  enif_send(env, &m->pid, m->env, m->value);
  enif_free_env(m->env);
}

ERL_NIF_TERM local_message_on_gc(ErlNifEnv *env, const ERL_NIF_TERM pid_term,
                                 const ERL_NIF_TERM term) {
  if (!enif_is_pid(env, pid_term)) {
    return enif_make_badarg(env);
  }
  ErlNifPid pid;
  if (!enif_get_local_pid(env, pid_term, &pid)) {
    return kAtomError;
  }

  struct message *m =
      enif_alloc_resource(message_resource, sizeof(struct message));
  if (!m) {
    return kAtomError;
  }

  m->env = enif_alloc_env();
  if (!m->env) {
    return kAtomError;
  }
  m->pid = pid;
  m->value = enif_make_copy(m->env, term);

  ERL_NIF_TERM res_term = enif_make_resource(env, m);
  enif_release_resource(m);
  return res_term;
}

void local_message_open_resource(ErlNifEnv *env) {
  void *rt =
      enif_open_resource_type(env, "Elixir.Explorer.Remote.LocalGC", "message",
                              destruct_local_message, ERL_NIF_RT_CREATE, 0);
  if (!rt)
    return;
  message_resource = rt;

  kAtomError = enif_make_atom(env, "error");
}

Questions/Issues

When implementing it in Rust, the very first issue I have is,

if I want to hold a value of a NIF term (passed from Elixir, assuming it can be any valid Erlang terms) in LocalMessage, it will have a lifetime 'a, thus LocalMessage should be:

pub struct LocalMessage<'a> {
    pub pid: LocalPid,
    pub term: Term<'a>,
}

and all subsequent structs that contains a LocalMessage would also have lifetime that is at least 'a. Therefore, if I want to have an ExLocalMessageRef that holds a LocalMessage, it should be:

pub struct ExLocalMessageRef<'a>(pub LocalMessage<'a>);

impl<'a> ExLocalMessageRef<'a> {
    pub fn new(m: LocalMessage<'a>) -> Self {
        Self(m)
    }
}

As a result, this requires me to mark the on_load function with lifetime 'a so that I can use rustler::resource! for ExLocalMessageRef<'a>

fn on_load<'a>(env: Env, _info: Term) -> bool {
    rustler::resource!(ExLocalMessageRef<'a>, env);
    true
}

But the problem is, in rustler::resource!, the resource type is declared as static, which requires lifetime 'a to outlive 'static.

static mut STRUCT_TYPE: Option<$crate::resource::ResourceType<$struct_name>> = None;

I'm not sure if I should use lifetime 'static for LocalMessage and ExLocalMessageRef because it doesn't look quite right to me. Plus using 'static is invalid on structs as it is a reserved lifetime name.

@filmor
Copy link
Member

filmor commented Jun 28, 2024

This is not the right place for a discussion like this. I'll close the PR for now (but I'll incorporate the feature of allowing to use the caller env in the destructor). Please copy your questions to the Discussions or a forum like the Elixir forum and @-reference me.

@filmor filmor closed this Jun 28, 2024
@cocoa-xu
Copy link
Contributor Author

Okay, thanks for the help!

@cocoa-xu cocoa-xu deleted the cx-custom-dtor branch June 28, 2024 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants