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 non-static UDS client connection paths #1612

Open
kriswuollett opened this issue Jan 30, 2024 · 4 comments
Open

Support non-static UDS client connection paths #1612

kriswuollett opened this issue Jan 30, 2024 · 4 comments

Comments

@kriswuollett
Copy link

Feature Request

Support non-static UDS client connection paths

Motivation

Currently the UDS client example is using a static path, I believe because of the ownership requirements of the service function closure:

// We will ignore this uri because uds do not use it
// if your connector does use the uri it will be provided
// as the request to the `MakeConnection`.
let channel = Endpoint::try_from("http://[::]:50051")?
.connect_with_connector(service_fn(|_: Uri| {
let path = "/tmp/tonic/helloworld";
// Connect to a Uds socket
UnixStream::connect(path)
}))
.await?;
let mut client = GreeterClient::new(channel);

It would be great if tonic and its hyperium dependencies, as needed, took ownership of the specified path to avoid the user needing to vend a &'static str.

Related

kriswuollett added a commit to kriswuollett/tonic that referenced this issue Jan 30, 2024
Update unix domain socket examples to show configurable paths. It
demonstrates a way of configuring the UDS client using the ustr crate to
meet the closure capture requirements of the connector service_fn.

Fixes: hyperium#1611
Refs: hyperium#1612
@philjb
Copy link

philjb commented Mar 28, 2024

I second this and am currently using Box::leak() to work around it. Some of my rust teammates helped me out: An Arc or a cloned PathBuf should also meet the 'static requirements.

like

    let path = PathBuf::from("/tmp/tonic/helloworld");

    let channel = Endpoint::try_from("http://[::]:50051")?
        .connect_with_connector(service_fn(move |_: Uri| {
            // Connect to a Uds socket
            UnixStream::connect(path.clone())
        }))
        .await?;

@youyuanwu
Copy link

I second this and am currently using Box::leak() to work around it. Some of my rust teammates helped me out: An Arc or a cloned PathBuf should also meet the 'static requirements.

like

    let path = PathBuf::from("/tmp/tonic/helloworld");

    let channel = Endpoint::try_from("http://[::]:50051")?
        .connect_with_connector(service_fn(move |_: Uri| {
            // Connect to a Uds socket
            UnixStream::connect(path.clone())
        }))
        .await?;

This no longer works on 0.12 version after this change: #1670

    let path = PathBuf::from("/tmp/my.sock");

    let channel = Endpoint::try_from("http://[::]:50051")?
        .connect_with_connector(service_fn(move |_| async {
            // Connect to a Uds socket
            let io = TokioIo::new(UnixStream::connect(path.clone()).await?);
            Ok::<_, std::io::Error>(io)
        }))
        .await?;

gives error:

   |
25 |           .connect_with_connector(service_fn(move |_| async {
   |  ____________________________________________--------_^
   | |                                            |      |
   | |                                            |      return type of closure `{async block@src/walfs/src/bin/walfsctl.rs:25:53: 29:10}` contains a lifetime `'2`
   | |                                            lifetime `'1` represents this closure's body
26 | |             // Connect to a Uds socket
27 | |             let io = TokioIo::new(UnixStream::connect(path.clone()).await?);
28 | |             Ok::<_, std::io::Error>(io)
29 | |         }))
   | |_________^ returning this value requires that `'1` must outlive `'2`
   |
   = note: closure implements `Fn`, so references to captured variables can't escape the closure

I wrote a custom ServiceFn (UdsConnector) that owns the path to work around, and I can upstream it here if tonic owners are open for it.
Is there a simpler workaround to reuse service_fn?
CC: @alexrudy @LucioFranco for comments.

@alexrudy
Copy link
Contributor

alexrudy commented Aug 1, 2024

ServiceFn must be re-usable, so the future returned can't borrow from the surrounding closure.

This works:

    let path = PathBuf::from("/tmp/my.sock");

    let channel = Endpoint::try_from("http://[::]:50051")?
        .connect_with_connector(service_fn(move |_| {
            let path = path.clone();
            async move {
                // Connect to a Uds socket
                let io = TokioIo::new(UnixStream::connect(path).await?);
                Ok::<_, std::io::Error>(io)
            }
        }))
        .await?;

@youyuanwu
Copy link

This works. Thanks for the suggestion.

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

No branches or pull requests

4 participants