TCP overrides #2
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ency on cap_net_ext
…Socket. Also updated some overly strict test cases.
…okio::net::TcpListener::poll_accept
…-allocating variant wherever possible.
- BoxSyncFuture: for public facing signatures. This is just a regular `Pin<Box<dyn Future ...>>` - Preview2Future: utility type for use within the WASI wrapper code
rylev
approved these changes
Jan 15, 2024
| fn resolve_addresses(&mut self, name: String) -> ResolveAddressStream; | ||
| /// | ||
| /// Unicode domain names are automatically converted to ASCII using IDNA encoding. | ||
| /// If the input is an IP address string, the address is parsed and returned |
There was a problem hiding this comment.
Nit: these docs are written as if resolve_addresses was a concrete function. We should phrase them so that it's clear that these are expectations the implementor must uphold.
There was a problem hiding this comment.
For a large part I (mindlessly) copied these docs from the WIT files :)
That being said, don't think it's that weird to write the documentation for the consumer instead of the implementor. There is precedent
Comment on lines
+86
to
+97
| /// Non-boxing variant of [Network::resolve_addresses] | ||
| pub fn resolve_addresses( | ||
| &mut self, | ||
| name: String, | ||
| ) -> impl Future<Output = io::Result<Vec<IpAddr>>> + Send + Sync + 'static { | ||
| async move { resolve_addresses(&name).await } | ||
| } | ||
|
|
||
| /// Non-boxing variant of [Network::new_tcp_socket] | ||
| pub fn new_tcp_socket(&mut self, family: SocketAddrFamily) -> io::Result<SystemTcpSocket> { | ||
| SystemTcpSocket::new(family) | ||
| } |
There was a problem hiding this comment.
It's unclear to me why we need this functions instead of inlining their bodies everywhere they're called.
There was a problem hiding this comment.
Because the functions they call are internal to the crate. We could open those up, but I think it makes sense (at least conceptually) that consumers can't fabricate a SystemTcpSocket without going through SystemNetwork.
Co-authored-by: Ryan Levick <me@ryanlevick.com>
Co-authored-by: Ryan Levick <me@ryanlevick.com>
…to tcp-override
|
Amazing stuff! I'll get going on UDP! |
Merged
rylev
pushed a commit
that referenced
this pull request
Apr 2, 2024
…dealliance#7029) * Rename `Host*` things to avoid name conflicts with bindings. * Update to the latest resource-enabled wit files. * Adapting the code to the new bindings. * Update wasi-http to the resource-enabled wit deps. * Start adapting the wasi-http code to the new bindings. * Make `get_directories` always return new owned handles. * Simplify the `poll_one` implementation. * Update the wasi-preview1-component-adapter. FIXME: temporarily disable wasi-http tests. Add logging to the cli world, since stderr is now a reseource that can only be claimed once. * Work around a bug hit by poll-list, fix a bug in poll-one. * Comment out `test_fd_readwrite_invalid_fd`, which panics now. * Fix a few FIXMEs. * Use `.as_ref().trapping_unwrap()` instead of `TrappingUnwrapRef`. * Use `drop_in_place`. * Remove `State::with_mut`. * Remove the `RefCell` around the `State`. * Update to wit-bindgen 0.12. * Update wasi-http to use resources for poll and I/O. This required making incoming-body and outgoing-body resourrces too, to work with `push_input_stream_child` and `push_output_stream_child`. * Re-enable disabled tests, remove logging from the worlds. * Remove the `poll_list` workarounds that are no longer needed. * Remove logging from the adapter. That said, there is no replacement yet, so add a FIXME comment. * Reenable a test that now passes. * Remove `.descriptors_mut` and use `with_descriptors_mut` instead. Replace `.descriptors()` and `.descriptors_mut()` with functions that take closures, which limits their scope, to prevent them from invalid aliasing. * Implement dynamic borrow checking for descriptors. * Add a cargo-vet audit for wasmtime-wmemcheck. * Update cargo vet for wit-bindgen 0.12. * Cut down on duplicate sync/async resource types (#1) * Allow calling `get-directories` more than once (#2) For now `Clone` the directories into new descriptor slots as needed. * Start to lift restriction of stdio only once (#3) * Start to lift restriction of stdio only once This commit adds new `{Stdin,Stdout}Stream` traits which take over the job of the stdio streams in `WasiCtxBuilder` and `WasiCtx`. These traits bake in the ability to create a stream at any time to satisfy the API of `wasi:cli`. The TTY functionality is folded into them as while I was at it. The implementation for stdin is relatively trivial since the stdin implementation already handles multiple streams reading it. Built-in impls of the `StdinStream` trait are also provided for helper types in `preview2::pipe` which resulted in the implementation of `MemoryInputPipe` being updated to support `Clone` where all clones read the same original data. * Get tests building * Un-ignore now-passing test * Remove unneeded argument from `WasiCtxBuilder::build` * Fix tests * Remove some workarounds Stdio functions can now be called multiple times. * If `poll_oneoff` fails part-way through, clean up properly. Fix the `Drop` implementation for pollables to only drop the pollables that have been successfully added to the list. This fixes the poll_oneoff_files failure and removes a FIXME. --------- Co-authored-by: Alex Crichton <alex@alexcrichton.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For TCP sockets and IP name lookup, I've moved all the WASI interop code into the
crates/wasi/src/preview2/host/folder. The files directly incrates/wasi/src/preview2/should now be as "idiomatic" Rust as I could get it. Part of this change is that I updatedNetwork::resolve_addressesto return a plainBox<dyn Future>and effectively demotedResolveAddressStreamto be an implementation detail of the WASI interop.In the end I did get the regular AsyncRead and AsyncWrite to work, thanks to the existing
AsyncReadStreamandAsyncWriteStreamimplementations inpipe.rsandwrite_stream.rsI've combined the two distinct permission mechanisms (AllowedNetworkUses & SocketAddrCheck) into one.
In UdpSocket, I've removed the Option<> wrapper around the SocketAddrCheck.
Because the System*** types are expected to be wrapped inside other implementations, I've added non-boxing variants of methods that would otherwise require a box because of the dyn Trait restrictions. This eliminates unnecessary intermediate boxes in places where the inner type is statically known. For example, the
innerfield ofDefaultTcpSocketcan be of typeSystemTcpSocketinstead ofBox<dyn TcpSocket>. Places where this is relevant: