hyperium / tonic

A native gRPC client & server implementation with async/await support.
https://docs.rs/tonic
MIT License
9.76k stars 997 forks source link

Support non-static UDS client connection paths #1612

Open kriswuollett opened 8 months ago

kriswuollett commented 8 months ago

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:

https://github.com/hyperium/tonic/blob/c30cb783b9e5eaa4ea05003d9cacac733d135c58/examples/src/uds/client.rs#L16-L28

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

philjb commented 6 months ago

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 commented 1 month ago

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: https://github.com/hyperium/tonic/pull/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 commented 1 month ago

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 commented 1 month ago

This works. Thanks for the suggestion.