wasmtime / PR #11291 feat(wasip3): implement `wasi:sockets` · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #11291 feat(wasip3): implement `wasi:sockets`

Wasmtime GitHub notifications bot (Jul 21 2025 at 18:08):

rvolosatovs opened PR #11291 from rvolosatovs:feat/wasip3-sockets to bytecodealliance:main:

Implement wasip3 wasi:sockets.

I've refactored FutureWriter::write a little bit to allow for non-blocking send

Refs https://github.com/bytecodealliance/wasip3-prototyping/issues/228
Closes https://github.com/bytecodealliance/wasip3-prototyping/issues/226 (not the addition of yield from the guest)

Wasmtime GitHub notifications bot (Jul 21 2025 at 18:12):

rvolosatovs edited PR #11291:

Implement wasip3 wasi:sockets.

I've refactored FutureWriter::write a little bit to allow for non-blocking send

Refs https://github.com/bytecodealliance/wasip3-prototyping/issues/228
Closes https://github.com/bytecodealliance/wasip3-prototyping/issues/226 (not the addition of yield from the guest)
I was not able to reproduce https://github.com/bytecodealliance/wasip3-prototyping/issues/44 with:
while cargo test --features p3 p3_sockets_udp_sample_application; do :; done
on MacOS

Wasmtime GitHub notifications bot (Jul 21 2025 at 18:13):

rvolosatovs updated PR #11291.

Wasmtime GitHub notifications bot (Jul 21 2025 at 18:15):

rvolosatovs edited PR #11291:

Implement wasip3 wasi:sockets.

I've refactored FutureWriter::write a little bit to allow for non-blocking send

Refs https://github.com/bytecodealliance/wasip3-prototyping/issues/228
Closes https://github.com/bytecodealliance/wasip3-prototyping/issues/226 (note the addition of yield from the guest)
I was not able to reproduce https://github.com/bytecodealliance/wasip3-prototyping/issues/44 with:
while cargo test --features p3 p3_sockets_udp_sample_application; do :; done
on MacOS

Wasmtime GitHub notifications bot (Jul 21 2025 at 19:39):

rvolosatovs has marked PR #11291 as ready for review.

Wasmtime GitHub notifications bot (Jul 21 2025 at 19:39):

rvolosatovs requested wasmtime-wasi-reviewers for a review on PR #11291.

Wasmtime GitHub notifications bot (Jul 21 2025 at 19:39):

rvolosatovs requested dicej for a review on PR #11291.

Wasmtime GitHub notifications bot (Jul 21 2025 at 19:39):

rvolosatovs requested wasmtime-core-reviewers for a review on PR #11291.

Wasmtime GitHub notifications bot (Jul 21 2025 at 19:39):

rvolosatovs requested wasmtime-default-reviewers for a review on PR #11291.

Wasmtime GitHub notifications bot (Jul 22 2025 at 15:08):

rvolosatovs commented on PR #11291:

FYI: I'm currently working on reusing most of the socket utilities across p2 and p3 to avoid duplication

Wasmtime GitHub notifications bot (Jul 22 2025 at 20:57):

rvolosatovs updated PR #11291.

Wasmtime GitHub notifications bot (Jul 22 2025 at 20:58):

rvolosatovs updated PR #11291.

Wasmtime GitHub notifications bot (Jul 22 2025 at 21:17):

rvolosatovs updated PR #11291.

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

Could you detail a bit more the rationale for this change? The PR description mentions a non-blocking send but I don't fully grok that. In the context of https://github.com/bytecodealliance/wasip3-prototyping/pull/240 this method is going to change more so if precise control over sending messages is needed that may be a red flag of something we'll need to change at the call-site rather than here

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

I think this'll want some extra handling, namely:

If Some(...) is returned, that means the other side went away, so the loop can be exited immediately here.

If Some(...) is returned then the resource table needs to be updated here to take out what was previously pushed in

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

This may be a preexisting bug in the p2 implementation, but I'm under the impression that the equivalent of write_all for UDP is inappropriate. I'd expect this to return an error if the entire buffer wasn't sent personally, but I'm also not sure the context these are used within.

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

Could this be wrapped up in something like Arc<NonInheritedOptions> or something like that? That'll help cut down on the Arc-traffic and additionally enable scoping handling of this to a separate file and/or abstraction. Ideally it'd also be shared with the p2 implementation if possible which I suspect has very similar #[cfg].

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

I think this buf is intended to be outside of the loop?

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

In the future we may have to watch out for this as watch_reader is a pretty expensive operation right now and each iteration of the loop unconditionally sets this up. The changes at https://github.com/bytecodealliance/wasip3-prototyping/pull/240 may help though.

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

Should this be using Receiving instead of Closed? As-is it looks like you can send-then-receive but you can't receive-then-send.

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

All of the breaks in this loop are write-then-break, so could this be refactored as break-with-value followed by just one write?

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

This branch I think could be replaced with a ? on the with block.

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

One thing we may want to keep an eye out for in the future is some sort of fast path here to avoid the Arc::clone in the typical case.

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

It might be worth leaving a FIXME here along the lines of handling cancellation. Here we'll ideally want to hook into the drop of the outer future and put the socket back in the store or something like that.

Wasmtime GitHub notifications bot (Jul 22 2025 at 22:15):

alexcrichton created PR review comment:

Also, mind extracting this constant to a const at the top of the file (or something like that) to reference?

Wasmtime GitHub notifications bot (Jul 23 2025 at 05:51):

badeend submitted PR review.

Wasmtime GitHub notifications bot (Jul 23 2025 at 05:51):

badeend created PR review comment:

Nice catch. This is indeed not correct for UDP.

The P2 implementation simply ignores the return value.

I'd have to refresh my mind on this, but I recall that UDP either: returns successfully with input buffer size or fails with EMSGSIZE. Could be wrong.

Wasmtime GitHub notifications bot (Jul 23 2025 at 05:57):

badeend edited PR review comment.

Wasmtime GitHub notifications bot (Jul 23 2025 at 06:04):

badeend edited PR review comment.

Wasmtime GitHub notifications bot (Jul 23 2025 at 09:36):

rvolosatovs created PR review comment:

Currently, calling FutureWriter::write without awaiting the returned future does nothing. This patch makes it so that once Future::write returns it is guaranteed that a value is written akin to https://docs.rs/tokio/latest/tokio/sync/oneshot/struct.Sender.html#method.send, which is used internally. async is only used then to await the receipt of the value.

From the WASI implementation perspective this means that we now can write future values without an async context, meaning, for example, that we don't need to spawn a task just to send a static value like was done e.g. here: https://github.com/bytecodealliance/wasip3-prototyping/blob/9c837320498adb1f4bda9ad7bbb12769fcdd08b5/crates/wasi/src/p3/sockets/host/types/tcp.rs#L482-L487

Wasmtime GitHub notifications bot (Jul 23 2025 at 09:36):

rvolosatovs submitted PR review.

Wasmtime GitHub notifications bot (Jul 23 2025 at 09:42):

rvolosatovs created PR review comment:

Closed is normally used throughout the codebase as a marker to emulate a take operation, the resulting state is never Closed, it's either changed (to receiving) or rolled back, if it was not connected

Wasmtime GitHub notifications bot (Jul 23 2025 at 09:42):

rvolosatovs submitted PR review.

Wasmtime GitHub notifications bot (Jul 23 2025 at 17:28):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 23 2025 at 17:28):

alexcrichton created PR review comment:

I think it'd be reasonable to check the return value to double-check it matches the message size on success and synthesizing our trap (or error) on failure.

Wasmtime GitHub notifications bot (Jul 23 2025 at 17:32):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 23 2025 at 17:32):

alexcrichton created PR review comment:

If possible I'd prefer to avoid this change and perhaps add a new method for this. I'm worried about conflating the guarantees/semantics of Rust-level async with the semantics desired here. For example this seems like it would encourage calling write and dropping the returned future while still having a guarantee that the item was sent. That the item is sent in this situation is, in my opinion, a bug. Dropping a future should cancel whatever that future's computation is so in my mind the semantics of this function is that you're never guaranteed that the value is sent until the future is .await-ed.

I think it's reasonable though to shoot for an ideal of being able to send a value without having to full wait for its receipt. For this an extra host-level function I think would be appropriate. Even that though I think would be best to wait for the refactoring above.

In the interm would you be ok to back out this change and spawn tasks if necessary?

Wasmtime GitHub notifications bot (Jul 23 2025 at 17:34):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 23 2025 at 17:34):

alexcrichton created PR review comment:

Oh sorry I somehow missed the *tcp_state = .. below. The take/replace pattern seems reasonable to me but I just missed that (not sure how I did...)

Wasmtime GitHub notifications bot (Jul 29 2025 at 07:14):

rvolosatovs updated PR #11291.

Wasmtime GitHub notifications bot (Jul 29 2025 at 07:42):

rvolosatovs updated PR #11291.

Wasmtime GitHub notifications bot (Jul 29 2025 at 07:57):

rvolosatovs submitted PR review.

Wasmtime GitHub notifications bot (Jul 29 2025 at 07:57):

rvolosatovs created PR review comment:

I've updated the p3 implementation.

I do want to point out that on Windows at most i32::MAX bytes will be written https://github.com/rust-lang/rust/blob/cb6785f73df1aa3f558796a22a4ab9652cf38e26/library/std/src/sys/net/connection/socket.rs#L806 https://github.com/rust-lang/rust/blob/cb6785f73df1aa3f558796a22a4ab9652cf38e26/library/std/src/sys/net/connection/socket/windows.rs#L19

According to the spec, it appears that lists can have up to u32::MAX elements (core Wasm https://webassembly.github.io/spec/core/binary/conventions.html#binary-vec and CM https://github.com/WebAssembly/component-model/blob/main/design/mvp/Explainer.md#container-types). So it appears that at least on Windows a partial write is theoretically possible, but I'm not sure if it could cause an issue in practice for most use cases

Wasmtime GitHub notifications bot (Jul 29 2025 at 08:01):

rvolosatovs submitted PR review.

Wasmtime GitHub notifications bot (Jul 29 2025 at 08:01):

rvolosatovs created PR review comment:

Yes, although that means we need to do more bookkeeping to e.g. ensure that the socket is indeed dropped in the reuseaddr tests (https://github.com/bytecodealliance/wasip3-prototyping/issues/226)

Moving the PR to draft to figure this out.

Perhaps I will just add the additional function to the runtime to support this use case, which can be removed or reworked in #11325

Wasmtime GitHub notifications bot (Jul 29 2025 at 08:11):

badeend submitted PR review.

Wasmtime GitHub notifications bot (Jul 29 2025 at 08:11):

badeend created PR review comment:

That shouldn't be an issue for UDP. The spec says:

Implementations may trap if the data length exceeds 64 KiB.

Wasmtime GitHub notifications bot (Jul 29 2025 at 08:13):

rvolosatovs updated PR #11291.

Wasmtime GitHub notifications bot (Jul 29 2025 at 08:31):

rvolosatovs updated PR #11291.

Wasmtime GitHub notifications bot (Jul 29 2025 at 09:13):

rvolosatovs updated PR #11291.

Wasmtime GitHub notifications bot (Jul 29 2025 at 09:13):

rvolosatovs deleted PR review comment.

Wasmtime GitHub notifications bot (Jul 29 2025 at 10:17):

rvolosatovs updated PR #11291.

Wasmtime GitHub notifications bot (Jul 29 2025 at 21:58):

alexcrichton updated PR #11291.

Wasmtime GitHub notifications bot (Jul 29 2025 at 21:58):

alexcrichton has marked PR #11291 as ready for review.

Wasmtime GitHub notifications bot (Jul 29 2025 at 22:22):

alexcrichton updated PR #11291.

Wasmtime GitHub notifications bot (Jul 29 2025 at 23:35):

alexcrichton submitted PR review:

I had to update the REUSEADDR test a bit more and I'll file a follow-up issue for that, but otherwise this all looks good to me.

Wasmtime GitHub notifications bot (Jul 29 2025 at 23:39):

alexcrichton submitted PR review:

I had to update the REUSEADDR test a bit more and I'll file a follow-up issue for that (EDIT https://github.com/bytecodealliance/wasmtime/issues/11342), but otherwise this all looks good to me.

Wasmtime GitHub notifications bot (Jul 30 2025 at 00:00):

alexcrichton merged PR #11291.

Last updated: Dec 06 2025 at 06:05 UTC