Stream: git-wasmtime

Topic: wasmtime / PR #7895 wasi-sockets: (TCP) Use tokio's built...

Wasmtime GitHub notifications bot (Feb 08 2024 at 13:15):

badeend opened PR #7895 from badeend:tcp-tokio to bytecodealliance:main:

Full discussion: https://bytecodealliance.zulipchat.com/#narrow/stream/217126-wasmtime/topic/tokio.20always.20reports.20readiness

TLDR: Our implementation was flawed. It called tokio::net::TcpStream::try_from(...) directly after creating the socket. That triggers Tokio to call epoll_ctl to add it to the poll list. Linux sees that the socket is not connected (yet) and emits EPOLLHUP. Tokio interprets that to be a "final" state and never recovers from it. All future attempts to await I/O readiness immediately succeed.

This PR moves away from rustix and uses Tokio's own methods for constructing, binding, connecting, listening & accepting sockets. It still uses rustix for socket options

Wasmtime GitHub notifications bot (Feb 08 2024 at 13:15):

badeend requested wasmtime-core-reviewers for a review on PR #7895.

Wasmtime GitHub notifications bot (Feb 08 2024 at 13:15):

badeend requested alexcrichton for a review on PR #7895.

Wasmtime GitHub notifications bot (Feb 08 2024 at 14:52):

alexcrichton submitted PR review:

Nice! One very small comment but otherwise everything looks as I'd expect here :+1:

Wasmtime GitHub notifications bot (Feb 08 2024 at 14:52):

alexcrichton submitted PR review:

Nice! One very small comment but otherwise everything looks as I'd expect here :+1:

Wasmtime GitHub notifications bot (Feb 08 2024 at 14:52):

alexcrichton created PR review comment:

How come this does a spawn?

Wasmtime GitHub notifications bot (Feb 08 2024 at 15:27):

badeend submitted PR review.

Wasmtime GitHub notifications bot (Feb 08 2024 at 15:27):

badeend created PR review comment:

Without it, tokio would complain about the future not running within a tokio runtime context. Let me know if there are better ways to deal with this.

Wasmtime GitHub notifications bot (Feb 09 2024 at 11:01):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Feb 09 2024 at 11:03):

alexcrichton created PR review comment:

Ah ok in that case you can handle that with some judicious calls to with_ambient_tokio_runtime. This got a panicking test to pass for me for example:

diff --git a/crates/wasi/src/preview2/host/tcp.rs b/crates/wasi/src/preview2/host/tcp.rs
index 1fce052cd..614495874 100644
--- a/crates/wasi/src/preview2/host/tcp.rs
+++ b/crates/wasi/src/preview2/host/tcp.rs
@@ -158,7 +160,7 @@ impl<T: WasiView> crate::preview2::host::tcp::tcp::HostTcpSocket for T {
             TcpState::ConnectReady(result) => result,
             TcpState::Connecting(mut future) => {
                 let mut cx = std::task::Context::from_waker(futures::task::noop_waker_ref());
-                match future.as_mut().poll(&mut cx) {
+                match with_ambient_tokio_runtime(|| future.as_mut().poll(&mut cx)) {
                     Poll::Ready(result) => result,
                     Poll::Pending => {
                         socket.tcp_state = TcpState::Connecting(future);

Wasmtime GitHub notifications bot (Feb 09 2024 at 11:03):

alexcrichton commented on PR #7895:

Looking into the spawn bits now one orthogonal thing to highlight is that this doesn't compile currently on macOS and there's a few more bits and pieces that need an update (feel free to prtest:full here)

Wasmtime GitHub notifications bot (Feb 11 2024 at 13:48):

badeend updated PR #7895.

Wasmtime GitHub notifications bot (Feb 11 2024 at 13:51):

badeend commented on PR #7895:

Thanks! I've removed the task spawn and fixed the MacOS build.

Wasmtime GitHub notifications bot (Feb 12 2024 at 15:53):

alexcrichton merged PR #7895.

Last updated: Oct 23 2024 at 20:03 UTC