agronholm / anyio

High level asynchronous concurrency and networking framework that works on top of either trio or asyncio
MIT License
1.81k stars 138 forks source link

Allow `anyio.wait_socket_readable` to accept a file descriptor #821

Open davidbrochart opened 4 days ago

davidbrochart commented 4 days ago

Things to check first

Feature description

It seems that anyio.wait_socket_readable(sock) can not only accept a socket.socket but also a file descriptor (int).

So I think there is no need to call socket.fromfd() as mentioned in https://github.com/agronholm/anyio/issues/386, but we just need to change the typing of the argument to be socket.socket | int?

Use case

I think socket.fromfd doubles the number of open file descriptors. Also, it might not be easy to auto-detect the file descriptor's socket family, type and protocol, which are required for socket.fromfd.

graingert commented 2 days ago

You don't actually need to use fromfd you can use socket.socket(fileno=fd) and it will automatically detect the family, type and protocol, it won't duplicate the socket and it still prevent someone passing something odd like a file.

Does this work with your use-case?

You do need to defend against the socket closing the FD though when it gets garbage collected, you have to use a try/finally socket.detach() if you need to let the owner of the FD be responsible for closing it

davidbrochart commented 2 days ago

Does this work with your use-case?

No, I get an OSError(88, 'Socket operation on non-socket').

graingert commented 2 days ago

Does this work with your use-case?

No, I get an OSError(88, 'Socket operation on non-socket').

Hmm then it probably won't work with wait_readable and wait_writable...

It's exactly the sort of fd we want to prevent being handed to these functions

davidbrochart commented 1 day ago

I think most ZMQ sockets are real sockets, except for in-process sockets that use shared memory (@minrk would know more).

minrk commented 1 day ago

@davidbrochart when you hit that error, were you on Linux?

The signaling FD used for the event loop is not the underlying socket, so it's always the same for a given libzmq context regardless of the underlying socket transport. But it's not the same across systems. Where available, it uses eventfd (i.e. linux), then AF_UNIX, then AF_INET. I think eventfd is not valid for socket.fromfd, which means wrapping zmq FDs in sockets usually doesn't work on Linux, but does elsewhere, and the socket family is not actually knowable on Windows because it might be AF_UNIX or AF_INET depending on Windows' really weird partial AF_UNIX support.

So it's nontrivial and not always possible to wrap a zmq FD in a Python socket. But what we do know is that it is always a valid FD to pass to select / add_reader / wait_readable, since that's what it is there for, and how it's used every day in applications like Jupyter. I'm not sure why you'd want to wrap integer handles in a Python socket object when the underlying implementations don't need or want that.

FWIW, here's a very basic test showing that both add_reader(socket.fileno()) and trio.lowlevel.wait_readable(socket.fileno()) work as expected with integer FDs from zmq sockets on all platforms, including Windows. FWIW, the socket.fromfd and socket.socket(fileno=) methods work on Windows and mac, but don't on Linux, and are more complicated and less efficient and less compatible.

davidbrochart commented 1 day ago

@davidbrochart when you hit that error, were you on Linux?

Yes.