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

Generic solution to AsyncRead as std::io::Read #14

Open
najamelan opened this issue Mar 11, 2020 · 4 comments
Open

Generic solution to AsyncRead as std::io::Read #14

najamelan opened this issue Mar 11, 2020 · 4 comments

Comments

@najamelan
Copy link
Contributor

I just ran into this: https://crates.io/crates/async-stdio

It's only in an alpha release, I haven't scrutinized it, but if it's good, it's probably worth for tokio-tungstenite, async-tungstenite and potentially other (I thought tokio had the same issue somewhere) to re-use a generic solution to this problem rather than handrolling code for this in several places.

@sdroege
Copy link
Owner

sdroege commented Mar 12, 2020

Yeah I like the idea and already saw that crate a few weeks ago. Once it's stable, I think we can move over to it. Back then I didn't see anything obviously wrong there.

One thing that might be a problem from our side is that we need to be able to have up to two wakers per read/write operation (reading a ping will cause a pong to be sent, so you can have a waker from your read and write task waiting for writing to be possible again). That will have to be investigated.

@najamelan
Copy link
Contributor Author

One thing that might be a problem from our side is that we need to be able to have up to two wakers per read/write operation

True, I forgot about that issue. That makes it quite specific and maybe not so easy to use a general solution. That really is a particular tungstenite quirk...

@sdroege
Copy link
Owner

sdroege commented Apr 12, 2020

Needs someone to investigate in any case :)

@najamelan
Copy link
Contributor Author

Just wanted to mention that stjepang also released async-compat, but unfortunately it doesnt work for tokio 0.3 and it seems to always load tokio with the rt feature... also might not deal with the double waker issue.

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

2 participants