Skip to content

Initial attempt at implementing enif_select#738

Open
filmor wants to merge 6 commits into
masterfrom
select
Open

Initial attempt at implementing enif_select#738
filmor wants to merge 6 commits into
masterfrom
select

Conversation

@filmor
Copy link
Copy Markdown
Member

@filmor filmor commented May 22, 2026

Try to provide a proper Rust interface to enif_select, ref #735.

@filmor
Copy link
Copy Markdown
Member Author

filmor commented May 22, 2026

@lilioid Could you provide a test-case?

@lilioid
Copy link
Copy Markdown
Contributor

lilioid commented May 22, 2026

@lilioid Could you provide a test-case?

Nothing fully fleshed out at the moment (maybe over the weekend though).
For now I can point you towards the code I'm currently experimenting with that uses enif_select().

And then to test it I was doing this:

$ run0 --user=$(whoami) --chdir=$(pwd) --property "AmbientCapabilities=CAP_NET_ADMIN"
DMIN"
$ iex -S mix
iex(1)> {:ok, hdl} = P2pChat.Transport.PrimTun.make_tun_device true
iex(2)> receive do
...(2)>   msg -> msg
...(2)> end

@filmor
Copy link
Copy Markdown
Member Author

filmor commented May 22, 2026

The respective call would now be

let _ = handle.select(
    env,
    handle.device.as_raw_fd().into(),
    SelectMode::Read,
    None,
    None
);

Works on my machine :)

@filmor
Copy link
Copy Markdown
Member Author

filmor commented May 23, 2026

Seeing that there quite a few optional parameters, should we maybe use the builder pattern here? Like resource.select_read(evt).with_pid(pid).with_message(msg).with_reference(ref).commit(env);

@filmor filmor marked this pull request as ready for review May 23, 2026 19:45
@filmor filmor requested a review from a team May 23, 2026 19:45
@lilioid
Copy link
Copy Markdown
Contributor

lilioid commented May 23, 2026

The new call works well for me, thank you for this work!

Seeing that there quite a few optional parameters, should we maybe use the builder pattern here?

I don't really see the need tbh. There's already quite some wrapping and type conversion going on and I wouldn't overcomplicate it.

@lilioid
Copy link
Copy Markdown
Contributor

lilioid commented May 23, 2026

Ahh but I just saw is that maybe the ref arg is not typed correctly.
enif_select requires any term but the way it's implemented here only data of type Reference can be used.

What enif_select() calls a reference here is as far as I can see just a passthrough term that gets passed back to the process when an IO notification message arrives.

@lilioid
Copy link
Copy Markdown
Contributor

lilioid commented May 23, 2026

Another issue is that in order to correctly use the API a user needs to always call ResourceArc::stop() to ensure safe closing of a file-descriptor.

Use ERL_NIF_SELECT_STOP as mode in order to safely close an event object that has been passed to enif_select
This safe way of closing event objects must be used even if all notifications have been received (or cancelled) and no further calls to enif_select have been made.

I'm not sure if this is possible with the current API when a process dies and all handles to the Resource object are lost.
My idea would be to use either the down() or destructor() callback on the Resource implementation to then issue the stop call however in this callback, no ResourceRef argument is given (only self for the containing type). This also makes sense because no handles exist for the object anymore but just the object itself.
It is however not possible to just use that self argument for issuing the stop call because the way rustler uses allocated memory ensures alignment which can lead to self not being located at the memory location that the VM expects for the obj pointer.

@filmor
Copy link
Copy Markdown
Member Author

filmor commented May 23, 2026

Quoting the docs:

Argument ref must be either a reference obtained from erlang:make_ref/0 or the atom undefined. It will be passed as Ref in the notifications. If a selective receive statement is used to wait for the notification then a reference created just before the receive will exploit a runtime optimization that bypasses all earlier received messages in the queue.

The original C API simply does not distinguish different types of terms the same way as we do.

If you want actual custom messages, we'd need to implement the newer enif_select_read and enif_select_write (see https://www.erlang.org/doc/apps/erts/erl_nif.html#enif_select_write).

@filmor
Copy link
Copy Markdown
Member Author

filmor commented May 23, 2026

Regarding the necessity to call stop:

Use enif_monitor_process together with enif_select to detect failing Erlang processes and prevent them from causing permanent leakage of resources and their contained OS event objects.

But for this we would indeed need to pass the original ResourceArc into the down callback. A breaking change, but not a huge issue. It is explicitly not possible to do this in the dtor callback:

The obj argument is a pointer to the resource. The only allowed use for the resource in the destructor is to access its user data one final time. The destructor is guaranteed to be the last callback before the resource is deallocated.

@filmor
Copy link
Copy Markdown
Member Author

filmor commented May 25, 2026

A few notes (also for me :)):

  • enif_select_read/write are not real functions, instead they use enif_select_x, introduced in v2.15 and not yet mapped, with the CUSTOM_MSG bit set
  • The API should probably allow for AsRawFd/AsRawHandle implementors, only internally converting to ErlNifEvent (= int on Unix and = HANDLE on Windows)
  • Safe lifetime handling might be tricky. We could do something akin to the "trick" for resource binaries, to enforce that the handles/fds have a lifetime that is a superset of the resource's lifetime (resource.select(..., |r| &r.my_handle)). In the end, that's what one probably wants to do anyhow.

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.

2 participants