Skip to content
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

Decouple WASIp2 sockets from WasiFd #131449

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

nickrum
Copy link
Contributor

@nickrum nickrum commented Oct 9, 2024

This is a follow up to #129638, decoupling WASIp2's socket implementation from WASIp1's WasiFd as discussed with @alexcrichton.

Quite a few trait implementations in std::os::fd rely on the fact that there is an additional layer of abstraction between Socket and OwnedFd. I thus had to add a thin WasiSocket wrapper struct that just "forwards" to OwnedFd. Alternatively, I could have added a lot of conditional compilation to std::os::fd, which feels even worse.

Since WasiFd::sock_accept is no longer accessible from TcpListener and since WASIp2 has proper support for accepting sockets through Socket::accept, the std::os::wasi::net module has been removed from WASIp2, which only contains a single TcpListenerExt trait with a sock_accept method as well as an implementation for TcpListener. Let me know if this is an acceptable solution.

@rustbot
Copy link
Collaborator

rustbot commented Oct 9, 2024

r? @Amanieu

rustbot has assigned @Amanieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added O-wasi Operating system: Wasi, Webassembly System Interface S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 9, 2024
@alexcrichton
Copy link
Member

I'm happy to take on review of this

r? @alexcrichton

@rustbot rustbot assigned alexcrichton and unassigned Amanieu Oct 9, 2024
@alexcrichton
Copy link
Member

@bors: r+

Looks great to me, thanks! It's ok to change std::os::wasi on the p2 target because that module is unstable on wasm32-wasip2 -- for precisely this reason!

@bors
Copy link
Contributor

bors commented Oct 9, 2024

📌 Commit 01e248f has been approved by alexcrichton

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 9, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 10, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#123951 (Reserve guarded string literals (RFC 3593))
 - rust-lang#130827 (Library: Rename "object safe" to "dyn compatible")
 - rust-lang#131383 (Add docs about slicing slices at the ends)
 - rust-lang#131403 (Fix needless_lifetimes in rustc_serialize)
 - rust-lang#131417 (Fix methods alignment on mobile)
 - rust-lang#131449 (Decouple WASIp2 sockets from WasiFd)
 - rust-lang#131462 (Mention allocation errors for `open_buffered`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 866869b into rust-lang:master Oct 10, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Oct 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 10, 2024
Rollup merge of rust-lang#131449 - nickrum:wasip2-net-decouple-fd, r=alexcrichton

Decouple WASIp2 sockets from WasiFd

This is a follow up to rust-lang#129638, decoupling WASIp2's socket implementation from WASIp1's `WasiFd` as discussed with `@alexcrichton.`

Quite a few trait implementations in `std::os::fd` rely on the fact that there is an additional layer of abstraction between `Socket` and `OwnedFd`. I thus had to add a thin `WasiSocket` wrapper struct that just "forwards" to `OwnedFd`. Alternatively, I could have added a lot of conditional compilation to `std::os::fd`, which feels even worse.

Since `WasiFd::sock_accept` is no longer accessible from `TcpListener` and since WASIp2 has proper support for accepting sockets through `Socket::accept`, the `std::os::wasi::net` module has been removed from WASIp2, which only contains a single `TcpListenerExt` trait with a `sock_accept` method as well as an implementation for `TcpListener`. Let me know if this is an acceptable solution.
@nickrum nickrum deleted the wasip2-net-decouple-fd branch October 10, 2024 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-wasi Operating system: Wasi, Webassembly System Interface S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants