rust-lang / libs-team

The home of the library team
Apache License 2.0
116 stars 18 forks source link

Add new API for named fifo #416

Closed NobodyXu closed 1 week ago

NobodyXu commented 1 month ago

Proposal

Problem statement

Annoymous and named pipe are widely used, there is crate os_pipe which just provides an abstraction for pipe on unix and has 15 million download.

While having a third-party crate for it is enough for many crates, I think it'd be better if it's in stdlib so that it can be used without having to another dependency for this.

It would also enable better integration with the std::process API, since users might want to pipe output of multiple processes to one pipe and read them.

Motivating examples or use cases

jobserver-rs, for example, is used by cc-rs and pulled in as build-dependencies quite often.

It internally implements all kinds of API for named fifo, contains a bunch of unsafe code for this and quite some code for just managing the fifo.

It'd be great if we could move them to stdlib and make jobserver-rs easier to maintain. It might also speedup jobserver-rs compilation since it could've drop the libc dependency.

tokio, the widely used async executor, already provide pipe support in mod tokio::net::unix::pipe.

Solution sketch

mod std::fifo {
    #[derive(Debug, Default, Clone)]
    pub struct FifoOpenOptions { ... }
    impl FifoOpenOptions {
        pub fn new() -> Self;

        pub fn create(&mut self, create: bool) -> &mut Self;
        pub fn create_new(&mut self, create_new: bool) -> &mut Self;

        pub fn open<P: AsRef<Path>>(&self, path: P) -> io::Result<Fifo>;
    }

    #[derive(Debug)]
    pub struct Fifo(/* private fields */);

    impl Fifo {
        pub fn try_clone(&self) -> io::Result<Self>;
    }

    #[cfg(unix)]
    impl AsFd for Fifo { ... }
    #[cfg(unix)]
    impl AsRawFd for Fifo { ... }

    #[cfg(windows)]
    impl AsHandle for Fifo { ... }
    #[cfg(windows)]
    impl AsRawHandle for Fifo { ... }

    // Use TryFrom here, because not every owned fd is a valid pipe
    #[cfg(unix)]
    impl From<OwnedFd> for Fifo { ... }
    #[cfg(windows)]
    impl From<OwnedHandle> for Fifo { ... }

    #[cfg(unix)]
    impl From<Fifo> for OwnedFd { ... }
    #[cfg(windows)]
    impl From<Fifo> for OwnedHandle { ... }

    impl From<Fifo> for Stdio { ... }

    #[cfg(unix)]
    impl FromRawFd for Fifo { ... }
    #[cfg(unix)]
    impl IntoRawFd for Fifo { ... }

    #[cfg(windows)]
    impl FromRawHandle for Fifo { ... }
    #[cfg(windows)]
    impl IntoRawHandle for Fifo { ... }

    impl<'a> Read for &'a Fifo { ...}
    impl Read for Fifo { ... }

    impl<'a> Write for &'a Fifo { ...}
    impl Write for Fifo { ... }
}

Alternatives

Alternatively, we could let the user to use File to open a fifo, however File has different characteristics from a fifo and the user would still need a third-party to create a fifo.

Or we could just leave them up to third-party crates, which is the current status-quo, which is OK-ish but not good enough for users who needfifo, they would have to grab a third-party crate for this.

Links and related work

kennytm commented 1 month ago
    // Use TryFrom here, because not every owned fd is a valid pipe
    #[cfg(unix)]
    impl TryFrom<OwnedFd> for Fifo { ... }
    #[cfg(windows)]
    impl TryFrom<OwnedHandle> for Fifo { ... }

We allow implementing From<OwnedFd> for UdpSocket and TcpStream etc which clearly "not every owned fd is a valid socket". I don't see why the Fifo requires special treatment.

If you turn a non-pipe fd to a Fifo, when using the pipe-specific functions the underlying OS API should be able to recognize the invalid input and return an error.

NobodyXu commented 1 month ago

Thanks, I've updated the proposed API

Amanieu commented 1 month ago

We discussed this in the libs-api meeting. A question was raised of whether this is sufficiently different from a File to justify introducing a new type. We could instead just expose mkfifo as a unix-specific filesystem method and then let people open it like a normal file.

NobodyXu commented 1 month ago

A question was raised of whether this is sufficiently different from a File to justify introducing a new type.

@Amanieu There are quite some differences:

It'd be better to create a dedicated abstraction for fifo.

joshtriplett commented 1 week ago

fifo on unix sometimes needs to be opened with readable and writable, otherwise it would block until the other end is opened, which is quite surprising behavior

Surprising but easily documented: people can open it with a File as read/write.

fifo does not implement Seek

Neither do various other things that you can turn into a File or open as a File; if you try to seek you'll get an error.

fifo support additional methods: linux-specific tee, unix-specific set pipe size

If we expose those at all, we could expose them as methods taking AsFd. And having a distinct type doesn't particularly prevent calling these methods on an unexpected kind of file descriptor.

fifo supports non-blocking read (which I might open a separate issue for)

So do some other kinds of file descriptor.

joshtriplett commented 1 week ago

We discussed this in today's @rust-lang/libs-api meeting.

We don't want to accept this ACP as written, and in particular don't want a separate Fifo type.

We would, however, accept a PR adding std::os::unix::mkfifo (accepting an AsRef<Path> and returning std::io::Result<()>).