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

Support for temporary UNIX domain sockets #175

Closed
jasonwhite opened this issue Apr 7, 2022 · 2 comments · Fixed by #177
Closed

Support for temporary UNIX domain sockets #175

jasonwhite opened this issue Apr 7, 2022 · 2 comments · Fixed by #177

Comments

@jasonwhite
Copy link
Contributor

jasonwhite commented Apr 7, 2022

I have a case where I want to create a temporary socket file which should be deleted when the server shuts down. tempfile::NamedTempFile doesn't work for this because you can't call UnixListener::bind(path) on a file that already exists. It'd be great to have tempfile handle the file path construction, but delegate the bind to a callback function.

For example, the API could look something like this:

impl Builder {
    // The contract is that `F` must return `io::ErrorKind::AlreadyExists` or
    // `io::ErrorKind::AddrInUse` (in the case of sockets) to continue retrying.
    pub fn make<F, R>(&self, f: F) -> io::Result<R>
    where
        F: Fn(PathBuf) -> io::Result<R>,
    {
        self.make_in(&env::temp_dir(), f)
    }

    pub fn make_in<F, R, P>(&self, dir: P, f: F) -> io::Result<R>
    where
        F: Fn(PathBuf) -> io::Result<R>,
        P: AsRef<Path>,
    {
        util::create_helper(/* ... */)
    }
}

And used like this:

    let (listener, path) = tempfile::Builder::new()
        .make(|path| Ok((UnixListener::bind(&path)?, TempPath::from_path(path))))?;

If NamedTempFile gained a generic parameter (e.g., NamedTempFile<F = File>), then we could put the socket inside of that. I think this generic parameter could be a backward-compatible change, but I'm not sure. Such a change could come later, since the tuple above should work fine.

    let listener = tempfile::Builder::new().make(|path| {
        Ok(NamedTempFile::from_parts(
            UnixListener::bind(&path)?,
            TempPath::from_path(path),
        ))
    })?;

Let me know what you think of this idea! This is generic enough to support more use cases beyond UNIX domain sockets, but I'm not sure what they are yet. There also shouldn't be any breaking changes introduced by this.

I'm happy to implement this if it seems reasonable.

@jasonwhite
Copy link
Contributor Author

jasonwhite commented Apr 7, 2022

Thinking about this more, a generic NamedTempFile might be required for correctness reasons. For example, if one does

let listener = tempfile::Builder::new()
    .make(|path| UnixListener::bind(&path))?;

Then, we just get back the type UnixListener, which won't delete the file upon drop. Thus, the API should actually return NamedTempFile<UnixListener> so it gets deleted correctly. Alternatively, it could return (TempPath, UnixListener), but switching to NamedTempFile<UnixListener> later would be a breaking change.

@Stebalien
Copy link
Owner

I think the generic NamedTempFile makes sense. We can default to File to avoid breaking anything.

In terms of builder functions, we'll need to support both make and make_in (or maybe other or custom?).

We should define the function to take an &Path. That way the user can call:

tempfile::Builder::new().make(UnixListener::bind)

Finally, I'd also:

  1. Provide a convenience function for binding unix listeners.
  2. Define a convenience type alias UnixTempFIle.

(because it doesn't really hurt and may make it easier for users)

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 a pull request may close this issue.

2 participants