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

Exposing current event loop handle to a service #1207

Closed
kw217 opened this issue Jun 5, 2017 · 9 comments
Closed

Exposing current event loop handle to a service #1207

kw217 opened this issue Jun 5, 2017 · 9 comments

Comments

@kw217
Copy link
Contributor

kw217 commented Jun 5, 2017

Hi – I’m trying to get async file I/O working within a Hyper service, and I’m having trouble getting hold of the event loop handle. I’m not sure if I’m missing something obvious, or if there is something missing from hyper 0.11a0.

(This may be related to #1075, and maybe I’ve just confused myself by using the “easy mode” APIs. If so, perhaps some improved docs / examples of how and when to do things the non-easy way would be helpful.)

I recently wrote a little static file webserver in Rust, which serves the files using async I/O. The crate tokio-file-unix (0.4.0) supports async I/O; all I needed to do was:

        let f = std::fs::File::open(path).expect("Couldn't open file");
        let file = tokio_file_unix::File::new_nb(f).expect("Couldn't set nonblocking");
        let reader = file.into_reader(&handle).expect("Fail to generate reader");
        // … and then you can use tokio_io::io::read(reader, buf) to read data from the file.

However, tokio_file_unix::File::into_reader needs to be provided with a tokio::reactor::handle (a handle to the current event loop), which it ultimately passes to tokio::reactor::PollEvented::new.

My problem is that I can’t see how to get hold of that handle!

As far as I can see, hyper::server::Http::bind() just passes an http::request::Request to the enclosed tokio_service::Service. There is no mention of handles.

To work around this, I forked hyper and made the following change. As you can see, it does the following:

  • hyper::server::Http::bind_connection now stores a clone of the current Handle in the HttpService struct.
  • The Service’s Request is now a pair (http::request::Request, tokio::reactor::Handle) rather than just an http::request::Request.
  • HttpService::call now passes a clone of the Handle (from the HttpService struct) to the service.

That’s obviously not the right solution. Am I missing something obvious, or is there something missing from Hyper at the moment?

Thanks.

@polachok
Copy link

polachok commented Jun 6, 2017

You can do something like this w/o modifying hyper:

    let mut core = Core::new().unwrap();
    let handle2 = core.handle(); 
    let listener = TcpListener::bind(&http_listen_addr, &handle2).unwrap();

    let server = listener.incoming().for_each(|(sock, addr)| {
        let s = MySuperService;
        Http::new().bind_connection(&handle2, sock, addr, s);

        Ok(())
    });
    core.run(server).unwrap();

Btw, I don't think tokio-file-unix does what you think it does.

@kw217
Copy link
Contributor Author

kw217 commented Jun 7, 2017

Ah, fair enough. I was worried that I would lose the goodness of Http::Server (which bind() creates), in particular, the shutdown handling, but I guess in reality that's not too important to me.

What's your concern about tokio-file-unix? The docs say "Asynchronous support for file-like objects via Tokio", and a superficial look through its lib.rs seems to suggest it does the right things.

@polachok
Copy link

polachok commented Jun 7, 2017

What's your concern about tokio-file-unix?

It doesn't work for regular files.

@kw217
Copy link
Contributor Author

kw217 commented Jun 7, 2017

I'm not sure what you mean by that. We have used it succesfully to read files from disk in our tests (using the code I pasted above), and it appeared to be behaving in a correctly asynchronous manner (judging by the response behaviour etc). This is on CentOS. By "doesn't work" do you mean it panics, or it appears to work but isn't fully asynchronous?

@kw217
Copy link
Contributor Author

kw217 commented Jun 13, 2017

OK, the answer to my original question seems to be that Hyper is fine, but the docs don't make it clear that if you want access to the handle, you must use Http::bind_connection rather than Http::bind.

Please could the docs be improved? As a newbie, I clicked through to hyper::server which lists four structs, one of which is "Server: an instance of a server created through "Http::bind". So I went to Http::bindand it seemed to do what I wanted. Immediately below isHttp::bind_connectionbut it says "This is the low-level method ... This method is typically not invoked directly but is rather transitively used throughbind`."

I think those statements are a bit too strong.

  • Firstly, despite the name, Server is just another server - it's not special in any way. Http is the key struct for HTTP servers. Can the first sentence of the Server comment say "Convenience type used by Http:bind", so it's more obvious this isn't the primary type?
  • Secondly, can Http::bind say "if you need more flexibility, use bind_connection" and Http::bind_connection remove the comments about "low-level" and "not typically invoked directly"? Instead it could say "For simple cases consider using bind instead".

@b4stien
Copy link

b4stien commented Jul 13, 2017

@kw217 would you mind providing a small gist of what your final solution (with #1207 (comment)) looks like?

I'm having troubles finding how to get back a reference to the handle in the call function of my service (spoiler alert: I'm a complete Rust beginner).

@kw217
Copy link
Contributor Author

kw217 commented Jul 14, 2017

Your service is a struct which implements the Service trait, and hence the call method. In your new method, just pass in the handle as an argument and store it in the struct. Then it's there for you to use in the call method.

The key point is that you should construct the server the way #1207 (comment) does with Http::bind_connection, rather than using Http::bind as the docs imply.

@seanmonstar
Copy link
Member

There is #1322 opened which proposes a solution to the base problem encountered here. As such, I'm going to close this particular issue.

@kw217
Copy link
Contributor Author

kw217 commented Sep 25, 2017

OK, I'm happy to follow #1322 instead, as long as the docs are improved to make the new usage clear.

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