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

Create server instance by passing in TcpListener #1602

Closed
yoshuawuyts opened this issue Jul 13, 2018 · 7 comments
Closed

Create server instance by passing in TcpListener #1602

yoshuawuyts opened this issue Jul 13, 2018 · 7 comments
Labels
A-server Area: server. B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature.

Comments

@yoshuawuyts
Copy link
Contributor

Hi there!

I was looking for a way to create a Hyper server instance from an existing TcpListener instance, and I couldn't find one. I was wondering it might be possible to add a method to allow for this.

Note: I might totally have missed it if there's already a method that allows for this. Apologies if any of this is redundant!

Motivation

Systemd has the capability of creating sockets and handing them off to applications using the $LISTEN_FD env var. Crates such as listenfd and systemfd make clever use of this to provide an uninterrupted reload experience during development.

As part of the CLI WG we've written clap-port-flag, which can create a socket from --port, $PORT, or $LISTEN_FD. It does this by exposing a .bind() method that contains all logic necessary to do this.

fn main() {
  let args = Cli::from_args();
  let _tcp_listener = args.port.bind().unwrap();
}

In order for Systemd's $LISTEN_FD to work with Hyper, a method would be needed to pass in a TcpListener instance.

Prior Art

actix-web has a server.listen() method that takes a TcpListener. It also exposes a server.bind() method which acts similar to Hyper's .bind() method.

Implementation

I propose adding a hyper::server::Server::listen() method with the following signature:

pub fn listen(listener: &TcpListener) -> Builder<AddrIncoming>

Note: To be honest I'm not completely sure if Builder<AddrIncoming> would be the right return type here. I hope it is!

Other Considerations

It looks like actix-web also exposes a .listen_tls() method which also accepts a TLS struct. I'm not sure what the interactions between TLS and external TcpListener initialization would be in Hyper.

References


Thanks so much for your time; I hope this is useful!

@bluetech
Copy link
Contributor

You can do this:

extern crate futures;
extern crate tokio;
extern crate hyper;

use futures::prelude::*;
use hyper::service::service_fn_ok;
use hyper::{Body, Response, Server};

fn main() {
    let addr = std::net::SocketAddr::from(([127, 0, 0, 1], 3000));
    let listener = std::net::TcpListener::bind(&addr).unwrap();
    let handle = tokio::reactor::Handle::current();
    let listener = tokio::net::TcpListener::from_std(listener, &handle).unwrap();

    let new_service = || service_fn_ok(|_req| Response::new(Body::from("Hello World")));

    let server = Server::builder(listener.incoming()).serve(new_service);

    tokio::run(server.map_err(|e| {
        eprintln!("server error: {}", e);
    }));
}

@seanmonstar
Copy link
Member

Yea, as the example pasted shows, this is currently possible. It's this method here: https://docs.rs/hyper/0.12.*/hyper/server/struct.Server.html#method.builder

Perhaps it could be clearer explained that this is how you do that? Maybe the server module documentation should mention building a server with a custom listener?

@yoshuawuyts
Copy link
Contributor Author

@bluetech awesome, thanks so much for your reply! That looks exactly like what I needed.

@seanmonstar I think an example in the docs would definitely be helpful!

@bluetech
Copy link
Contributor

bluetech commented Jul 13, 2018

I forgot to mention: hyper optionally sets some TCP options on the accepted sockets when it creates the listener. If you create the listener and you want to set them, you can do it as follows:

diff --git a/src/main.rs b/src/main.rs
index a74a15c..5d2f29c 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -11,10 +11,15 @@ fn main() {
     let listener = std::net::TcpListener::bind(&addr).unwrap();
     let handle = tokio::reactor::Handle::current();
     let listener = tokio::net::TcpListener::from_std(listener, &handle).unwrap();
+    let incoming = listener.incoming().map(|socket| {
+        socket.set_nodelay(true).unwrap();
+        socket.set_keepalive(Some(std::time::Duration::from_secs(7200))).unwrap();
+        socket
+    });
 
     let new_service = || service_fn_ok(|_req| Response::new(Body::from("Hello World")));
 
-    let server = Server::builder(listener.incoming()).serve(new_service);
+    let server = Server::builder(incoming).serve(new_service);
 
     tokio::run(server.map_err(|e| {
         eprintln!("server error: {}", e);

@yoshuawuyts
Copy link
Contributor Author

yoshuawuyts commented Jul 17, 2018

I took this back to the CLI WG a few days ago, and I don't think we've quite nailed the solution yet. Getting boundaries right for crates is tricky!

I think the core of the issue for us right now is that in clap-port-flags we have to choose between exposing a new, specific method just for Hyper support or telling people to copy-paste some boilerplate.

Examples

I think showing the problem might work best:

Current Usage

This is the current recommended usage. It's 3 lines using APIs from 3 different sources. For people that are getting started with HTTP, it requires that they understand the difference between sync / async sockets, and that they touch the hyper::Builder constructor.

use clap_port_flag::Port;
use futures::prelude::*;
use hyper::service::service_fn_ok;
use hyper::{Body, Response, Server};
use structopt::StructOpt;

#[derive(Debug, StructOpt)]
struct Cli {
  #[structopt(flatten)]
  port: Port,
}

fn main() -> Result<(), Box<std::error::Error>> {
  // Parse CLI args
  let args = Cli::from_args();

  // Create TCP socket
  let listener = args.port.bind()?;
  let handle = tokio::reactor::Handle::current();
  let listener = tokio::net::TcpListener::from_std(listener, &handle)?;

  // Run service
  let server = Server::builder(listener.incoming())
    .serve(|| service_fn_ok(|_| Response::new(Body::from("Hello World"))))
    .map_err(|e| eprintln!("server error: {}", e));
  tokio::run(server);

  Ok(())
}

Tokio in clap-port-flag

In this version we would integrate all tokio code into clap-port-flag. It's now only 1 line, and touches 1 crate. It still requires people to know the difference between sync and async sockets, and also relies on the hyper::Builder constructor.

use clap_port_flag::Port;
use futures::prelude::*;
use hyper::service::service_fn_ok;
use hyper::{Body, Response, Server};
use structopt::StructOpt;

#[derive(Debug, StructOpt)]
struct Cli {
  #[structopt(flatten)]
  port: Port,
}

fn main() -> Result<(), Box<std::error::Error>> {
  // Parse CLI args
  let args = Cli::from_args();

  // Create TCP socket
  let listener = args.port.bind_tokio()?;

  // Run service
  let server = Server::builder(listener.incoming())
    .serve(|| service_fn_ok(|_| Response::new(Body::from("Hello World"))))
    .map_err(|e| eprintln!("server error: {}", e));
  tokio::run(server);

  Ok(())
}

std::net::TcpListener support in hyper

This version would expose a new method on Hyper that accepts a std::net::TcpListener. This would be the same as the last example, except the Server::builder code is no longer required, and users no longer need to be aware of the differences between sync and async sockets.

use clap_port_flag::Port;
use futures::prelude::*;
use hyper::service::service_fn_ok;
use hyper::{Body, Response, Server};
use structopt::StructOpt;

#[derive(Debug, StructOpt)]
struct Cli {
  #[structopt(flatten)]
  port: Port,
}

fn main() -> Result<(), Box<std::error::Error>> {
  // Parse CLI args
  let args = Cli::from_args();

  // Create TCP socket, and run service
  let server = Server::listen(args.port.bind()?)
    .serve(|| service_fn_ok(|_| Response::new(Body::from("Hello World"))))
    .map_err(|e| eprintln!("server error: {}", e));
  tokio::run(server);

  Ok(())
}

Wrapping Up

I hope this somewhat explains what we're running into. I can't claim to know what the right solution is, given I don't know all the constraints Hyper itself is facing. But I do think it'd be great if we could figure out a way to make all this setup easier! I def hope it makes sense where I'm coming from.

Thanks again for your time!

@seanmonstar
Copy link
Member

I think the last option seems the nicest, since it is sort of boilerplate to just convert a std::net::TcpListener into a tokio one. I don't know if it should such a prized method name as listen... Some possible name options:

  • Server::tcp
  • Server::from_std
  • Server::from_std_tcp

Maybe something else, I don't know!

@seanmonstar seanmonstar added A-server Area: server. C-feature Category: feature. This is adding a new feature. B-rfc Blocked: More comments would be useful in determine next steps. labels Jul 17, 2018
@yoshuawuyts
Copy link
Contributor Author

Perhaps another option might be:

  • Server::from_tcp

I feel like Server::from_std_tcp is a bit long, but otherwise I think any name would work!

yoshuawuyts added a commit to yoshuawuyts/hyper that referenced this issue Aug 6, 2018
Adds the `from_tcp()` method as per
hyperium#1602.
yoshuawuyts added a commit to yoshuawuyts/hyper that referenced this issue Aug 7, 2018
Adds the `from_tcp()` method as per
hyperium#1602.
yoshuawuyts added a commit to yoshuawuyts/hyper that referenced this issue Aug 7, 2018
Adds the `from_tcp()` method as per
hyperium#1602.

fix clippy changes
yoshuawuyts added a commit to yoshuawuyts/hyper that referenced this issue Aug 7, 2018
Adds the `from_tcp()` method as per
hyperium#1602.

fix clippy changes
yoshuawuyts added a commit to yoshuawuyts/hyper that referenced this issue Aug 8, 2018
Adds the `from_tcp()` method as per
hyperium#1602.

fix clippy changes
yoshuawuyts added a commit to yoshuawuyts/hyper that referenced this issue Aug 8, 2018
Adds the `from_tcp()` method as per
hyperium#1602.

fix clippy changes
yoshuawuyts added a commit to yoshuawuyts/hyper that referenced this issue Aug 8, 2018
Adds the `from_tcp()` method as per
hyperium#1602.

fix clippy changes
yoshuawuyts added a commit to yoshuawuyts/hyper that referenced this issue Aug 8, 2018
Adds the `from_tcp()` method as per
hyperium#1602.

fix clippy changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-server Area: server. B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants