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

accept_async does immediate async i/o unexpectedly #55

Open
zargony opened this issue Feb 27, 2019 · 2 comments
Open

accept_async does immediate async i/o unexpectedly #55

zargony opened this issue Feb 27, 2019 · 2 comments
Assignees

Comments

@zargony
Copy link
Contributor

zargony commented Feb 27, 2019

I found out the hard way that accept_async is not only returning a future that handles the handshake, but it immediately accesses the async i/o object to start the handshake and returns a future to complete it.

This means that accept_async must be called from a task context, which I think isn't obvious and maybe should be stated more clearly.

In a pure futures 0.1 application, this most probably doesn't matter since accept_async is called when the accepted TcpStream is available, which pretty much always happens inside a task. However if you're using futures 0.3, you need to wrap a future 0.1 in a compatibility wrapper to use it with 0.3 and a current task is only provided to the future while polling it. I found out the hard way that this currently doesn't work well with tokio-tungstenite (see rust-lang/futures-rs#1463).

I'd like to propose to change the accept functions to only return a future and do the actual handshake when the future gets polled. That also seems to be the way everybody else handles it (async fns, futures-rs functions and methods, any other crate I can remember) and is probably the most expected way and would make it work smoothly with futures 0.3 compatibility wrappers as far as I can see.

@daniel-abramov
Copy link
Member

Indeed, your suggestion makes sense, thank you.

As a quick workaround I may suggest you to use futures::lazy().

@najamelan
Copy link
Contributor

najamelan commented Jul 10, 2019

I can confirm that something like: ok( stream ).and_then( accept_async ) also works, but lazy is probably a better solution here.

I gave lazy a try, but I can't figure out how to properly name the type of the FnOnce. I havn't tried boxing it. However with and_then, you can name the type as:

use
{
   tungstenite       :: handshake::server::NoCallback ,
   tokio_tungstenite :: AcceptAsync                   ,
   futures01         :: future::FutureResult          ,
};

AndThen< FutureResult<T, tungstenite::error::Error>, AcceptAsync<T, NoCallback>, fn(T) -> AcceptAsync<T, NoCallback> >

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants