wasip3: Implement nonblocking UDP I/O #775
wasip3: Implement nonblocking UDP I/O #775alexcrichton wants to merge 1 commit intoWebAssembly:mainfrom
Conversation
|
I'll note that procedurally this is based on #774 for now. This requires a Wasmtime due to the reliance of cancellation with nonblocking I/O and cancellation does not work well on 41.0.0. Updating Wasmtime, however, requires updating the wasip3 snapshot, hence the dependency. @badeend you're likely pretty interested in this as well. Out of curiosity would you be up for reviewing this? I understand you haven't done a lot of stuff in wasi-libc so there's no need to boot up on all the context if you aren't willing/able to, but I figured I'd ask nonetheless! |
|
@alexcrichton Now that #774 is merged, would you mind rebasing this PR off |
| // the subtask it's no longer candiate for cancellation so it's immediately | ||
| // dropped and zero'd out to avoid process in the `out` label below. | ||
| if (event.waitable == timeout_subtask) { |
There was a problem hiding this comment.
| // the subtask it's no longer candiate for cancellation so it's immediately | |
| // dropped and zero'd out to avoid process in the `out` label below. | |
| if (event.waitable == timeout_subtask) { | |
| // the subtask it's no longer candidate for cancellation so it's immediately | |
| // dropped and zero'd out to avoid processing in the `out` label below. | |
| if (event.waitable == timeout_subtask) { |
| } | ||
| } | ||
|
|
||
| static int poll_impl(struct pollfd *fds, size_t nfds, int timeout) { |
There was a problem hiding this comment.
I'm not able to immediately point a finger to any particular place, but something tells me there may be trouble if fds contains multiple entries for the same fd. Especially surrounding the register/ready callbacks per descriptor.
There was a problem hiding this comment.
Agreed yeah, I suspect that'll have a lot of problems. I'll work on that.
There was a problem hiding this comment.
It is an edge case, so as long as libc doesn't silently corrupt itself it's fine by me.
This commit implements nonblocking I/O for UDP sockets, specifically with `send` and `recv`. This additionally fills out enough of `poll` to get various tests doing nonblocking I/O to work. The main intention of this commit is to do the first foray into implementing nonblocking I/O in wasi-libc for wasip3. Broadly-speaking nonblocking I/O is in two categories: one being UDP and the other being `stream<u8>`-based reads/writes. This implements only the UDP half, and defers `stream<u8>` (and thus TCP for example) to a future commit. The implementation is relatively gnarly given the wasip3 APIs we currently have, but this was expected. I've done my best to be as careful as I can in terms of managing state machines but I'm pretty likely to inevitably get something wrong, so this should be expected to be something that's iterated on.
alexcrichton
force-pushed
the
udp-nonblocking
branch
from
ebe2e29 to
e4382cf
Compare
badeend
left a comment
badeend
left a comment
There was a problem hiding this comment.
Quite some intricate logic, but I think it all checks out.
| return ret; | ||
| } | ||
|
|
||
| /// TODO |
| } while (0) | ||
|
|
||
| #define _DEBUG 0 | ||
| #define _DEBUG 1 |
There was a problem hiding this comment.
Was this just for testing or do you want this enabled indefinitely?
There was a problem hiding this comment.
Its been a while since the last time I worked on this file. So I want to leave behind a general note, not related to the task at hand:
I see the p3 implementation builds on top of the p2 "streams" infrastructure. Strictly speaking, this should be unnecessary. All this bookkeeping exists because in 0.2 implicit binds were forbidden as that would bypass the concept of network handles. 0.3 doesn't have network handles and fully relies on virtualization. Everything regarding udp_socket_state_t, UDP_SOCKET_STATE_**, udp_socket_streams_t, etc. can be considered p2-only. The p3 async bookkeeping could in theory also be direct members on udp_socket_t.
Just a piece of background info.
| return true; | ||
| } | ||
|
|
||
| // Ends the process of receiving a packet on `sokcet`. |
There was a problem hiding this comment.
| // Ends the process of receiving a packet on `sokcet`. | |
| // Ends the process of receiving a packet on `socket`. |
| switch (socket->state.tag) { | ||
| case UDP_SOCKET_STATE_UNBOUND: | ||
| case UDP_SOCKET_STATE_BOUND_NOSTREAMS: | ||
| udp_socket_streams_t *streams = udp_streams(socket);; |
There was a problem hiding this comment.
| udp_socket_streams_t *streams = udp_streams(socket);; | |
| udp_socket_streams_t *streams = udp_streams(socket); |
| // Ensure there's a subtask started or a packet ready. If this is a nonblocking | ||
| // operation and a subtask was started then polling below isn't very useful, | ||
| // so go ahead and return `EWOULDBLOCK`. | ||
| bool started = wasip3_recv_start(socket, streams); |
There was a problem hiding this comment.
wasip3_recv_start also returns true when the subtask finished immediately without blocking. Ideally, this function shouldn't return EWOULDBLOCK for that case. To prevent a redundant roundtrip to poll.
| // * If this is a nonblocking context, poll the result and see what happens, | ||
| // bailing out if the subtask isn't ready yet. | ||
| // | ||
| // Upon completion of the subtask it's cleared out and then the reuslt is |
There was a problem hiding this comment.
| // Upon completion of the subtask it's cleared out and then the reuslt is | |
| // Upon completion of the subtask it's cleared out and then the result is |
| // This is technically deferring an error on a call to `send` to the next call | ||
| // to `send` for nonblocking I/O, but there's only so much that can be done | ||
| // about that... |
There was a problem hiding this comment.
In case it makes you feel any better; exposing errors from previous sends is normal for UDP.
There is no "connection", so the only way to find out about network issues is to just send out data, and then see if follow-up sends come back with e.g. ECONNREFUSED or EHOSTUNREACH. Those errors actually indicate that the prior sends failed.
2 participants
This commit implements nonblocking I/O for UDP sockets, specifically
with
sendandrecv. This additionally fills out enough ofpolltoget various tests doing nonblocking I/O to work. The main intention of
this commit is to do the first foray into implementing nonblocking I/O
in wasi-libc for wasip3. Broadly-speaking nonblocking I/O is in two
categories: one being UDP and the other being
stream<u8>-basedreads/writes. This implements only the UDP half, and defers
stream<u8>(and thus TCP for example) to a future commit.The implementation is relatively gnarly given the wasip3 APIs we
currently have, but this was expected. I've done my best to be as
careful as I can in terms of managing state machines but I'm pretty
likely to inevitably get something wrong, so this should be expected to
be something that's iterated on.