-
Notifications
You must be signed in to change notification settings - Fork 735
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
Add TCP networking for target_os = "wasi" #1549
Conversation
d821baa
to
10a6222
Compare
Have you actually tested this? Because last I remember my code didn't really work. |
yes I did 😀 |
10a6222
to
121d6ff
Compare
Corrected the |
@haraldh I just tried the TCP server example, but it doesn't work. Have you tried running the non-wasm version? The non-wasm returns a response on each packet of bytes send, but the wasm only responds when the connection is closed. When using |
121d6ff
to
32158ae
Compare
You are totally right... how about now with pub(crate) fn accept(
listener: &net::TcpListener,
) -> io::Result<(net::TcpStream, net::SocketAddr)> {
let (stream, addr) = listener.accept()?;
stream.set_nonblocking(true)?;
Ok((stream, addr))
} @Thomasdezeeuw should now work as expected. |
@haraldh please don't force push during a review. |
Oops. sorry, didn't know one was happening.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Their are some more problems with this stil.
- Can't use read and write
Interest
in a single register call. This is a major issue. Waker
not implemented.Selector::try_clone
is broken.- Still needs UDP implementation.
- Multiple threads not supported. Not a super big issue I think.
I put some review notes and change in Thomasdezeeuw@e530779. |
WebAssembly has no threads. Therefore, at least some of these issues don't apply. For example, what is the point of implementing WASI has no UDP support. Only streams are supported at this time. |
Isn't that something in the pipeline? I don't remember the details of it, but I thought it was, but last I checked was a couple of years ago.
Ok. So then we have:
Could you |
The spec isn't finalized yet. Portions of the proposal have been implemented in some runtimes. But WASI has no official way to spawn yet. |
32158ae
to
85fcddf
Compare
@Thomasdezeeuw thanks for your review. I merged your PR and added additional Also added more
|
85fcddf
to
e883424
Compare
pushed more commits to be squashed in the end |
@Thomasdezeeuw Any chance we can merge this? Are we waiting for something? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One final nit, but otherwise LGTM.
If you don't get to the final change @haraldh I'll do it tonight and merge the pr.
examples/tcp_server.rs
Outdated
@@ -1,19 +1,17 @@ | |||
// You can run this example from the root of the mio repo: | |||
// cargo run --example tcp_server --features="os-poll net" | |||
use mio::event::Event; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you revert all changes here, the changes make the example much harder to read. Please use the same structure as for the UDP example, with the extra main
. I know it will generate a number of warnings, but that's OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, will do
Based on tokio-rs#1395 And with * bytecodealliance/wasmtime#3711 * rust-lang/rust#93158 merged, mio can have limited support for networking for the `wasm32-wasi` target. Co-authored-by: Thomas de Zeeuw <thomasdezeeuw@gmail.com> Signed-off-by: Harald Hoyer <harald@profian.com>
Signed-off-by: Harald Hoyer <harald@profian.com>
2fd6719
to
db6652f
Compare
@Thomasdezeeuw squashed and left the old examples alone |
oh... you wanted it like UDP.. will change the examples. |
Otherwise `cargo test` fails. Signed-off-by: Harald Hoyer <harald@profian.com>
Use the `LISTEN_FDS` mechanism to use pre-opened sockets. Especially for `wasm32-wasi` there is no other way to get access to sockets, than to use pre-opened sockets. Because `wasm32-wasi` does not yet return `TcpListener::local_addr()`, an unspecified IP address and port will be returned and displayed. ``` $ cargo +nightly build --target wasm32-wasi --release --example tcp_listenfd_server --features="os-poll net" Compiling cfg-if v1.0.0 Compiling wasi v0.10.2+wasi-snapshot-preview1 Compiling log v0.4.14 Compiling libc v0.2.112 Compiling ppv-lite86 v0.2.15 Compiling wasi v0.11.0+wasi-snapshot-preview1 Compiling getrandom v0.2.3 Compiling rand_core v0.6.3 Compiling env_logger v0.8.4 Compiling rand_chacha v0.3.1 Compiling mio v0.8.0 (/home/harald/git/mio) Compiling rand v0.8.4 Finished release [optimized] target(s) in 2.92s $ wasmtime run --tcplisten 127.0.0.1:9000 --env 'LISTEN_FDS=1' target/wasm32-wasi/release/examples/tcp_listenfd_server.wasm Using preopened socket FD 3 You can connect to the server using `nc`: $ nc <IP> <PORT> You'll see our welcome message and anything you type will be printed here. ``` Signed-off-by: Harald Hoyer <harald@profian.com>
db6652f
to
2b8b022
Compare
ok, ready to be merged, except if you want to squash all commits. |
@Thomasdezeeuw Thanks! When can we get this in a release so that we can start working on the higher level components? |
Thanks @haraldh and @npmccallum for the work (and the patience).
Yes, I would like to include #1551 as well, so I think we can make a release this week. |
I can't seem to run it in a non-wasm environment anymore. How should I run this example on macos? |
Did you follow the instruction in the example? mio/examples/tcp_listenfd_server.rs Lines 4 to 5 in e077a23
That run for me on macOS. If it doesn't work for you can you provide some more details such as the error you're seeing. |
Oh, yeah the non-wasi version needs some update. |
What is the purpose of our setting LISTEN_FDS here? We don't seem to be using this value. |
The |
With
sock_accept
and basic networking bytecodealliance/wasmtime#3711sock_accept
and enable networking rust-lang/rust#93158merged, mio can have limited support for TCP networking for the
wasm32-wasi
target.I also modified the
tcp_server
example.