Skip to content

Conversation

@alexcrichton
Copy link
Member

This commit refactors the implementation of wasi:sockets for WASIp2 and WASIp3 to use the same underlying host data structure for the UdpSocket resource in WIT. Previously each version of WASI had its own socket which resulted in duplicated code. There's some minor differences between WASIp2 and WASIp3 but it's easy enough to paper over with the same socket type. This is intended to help with the maintainability of this going forward to only have one type to operate on rather than two (which also ensures that bugfixes for one should affect the other).

One other change made in this commit is that sprinkled checks for whether or not UDP is allowed are all removed and canonicalized during UDP socket creation. This means that UDP socket creation is the only location that checks for whether UDP is allowed. Once a UDP socket is created it can be used freely regardless of whether the UDP setting is enabled or disabled. This is not intended to have a large practical effect but it does mean the behavior of hosts that deny UDP but manually give access to a UDP socket resource to a component may behave subtly differently.

@alexcrichton alexcrichton requested a review from a team as a code owner August 5, 2025 17:48
socket: Arc::new(socket),
udp_state: UdpState::Default,
family: socket_address_family,
socket_addr_check: None,
Copy link
Member

@rvolosatovs rvolosatovs Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we clone the "check" from context here and avoid having to Arc::clone on each p3 usage of it? (#11291 (comment))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately no because the check will live in the store no matter what and we can't borrow the store across awaits, so it's got to get moved out. This is mostly just here to handle how in WASIp2 the socket check is inherited from the Network type rather than unconditionally from WasiCtx. While esoteric it's possible for embedders to push their own Network without using WasiCtx which has a distinct check than the one in WasiCtx itself, although we might want to remove such a feature eventually (which would remove the need for this field here)

This commit refactors the implementation of `wasi:sockets` for WASIp2
and WASIp3 to use the same underlying host data structure for the
`UdpSocket` resource in WIT. Previously each version of WASI had its own
socket which resulted in duplicated code. There's some minor differences
between WASIp2 and WASIp3 but it's easy enough to paper over with the
same socket type. This is intended to help with the maintainability of
this going forward to only have one type to operate on rather than two
(which also ensures that bugfixes for one should affect the other).

One other change made in this commit is that sprinkled checks for
whether or not UDP is allowed are all removed and canonicalized during
UDP socket creation. This means that UDP socket creation is the only
location that checks for whether UDP is allowed. Once a UDP socket is
created it can be used freely regardless of whether the UDP setting is
enabled or disabled. This is not intended to have a large practical
effect but it does mean the behavior of hosts that deny UDP but manually
give access to a UDP socket resource to a component may behave subtly
differently.
@alexcrichton alexcrichton force-pushed the refactor-wasi-udp-socket branch from 70ac013 to ff093a8 Compare August 5, 2025 19:19
@alexcrichton alexcrichton enabled auto-merge August 5, 2025 19:24
@alexcrichton alexcrichton requested a review from a team as a code owner August 5, 2025 20:06
@alexcrichton alexcrichton requested review from pchickey and removed request for a team August 5, 2025 20:06
@github-actions github-actions bot added the wasi Issues pertaining to WASI label Aug 5, 2025
@alexcrichton alexcrichton added this pull request to the merge queue Aug 6, 2025
@alexcrichton alexcrichton removed this pull request from the merge queue due to a manual request Aug 6, 2025
@alexcrichton alexcrichton added this pull request to the merge queue Aug 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 6, 2025
@rvolosatovs rvolosatovs added this pull request to the merge queue Aug 6, 2025
Merged via the queue into bytecodealliance:main with commit 00dd668 Aug 6, 2025
44 checks passed
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Aug 6, 2025
This is the same as bytecodealliance#11384 except for `TcpSocket` to share the
implementation of TCP across WASIp{2,3} and avoid duplicating code
between the two.
github-merge-queue bot pushed a commit that referenced this pull request Aug 7, 2025
* Use the same `TcpSocket` in WASIp{2,3}

This is the same as #11384 except for `TcpSocket` to share the
implementation of TCP across WASIp{2,3} and avoid duplicating code
between the two.

* Fix CI issues

* Review comments

* Trim "TCP allowed?" checks

Like UDP only perform the check on socket creation, not after it's
created.
bongjunj pushed a commit to prosyslab/wasmtime that referenced this pull request Oct 20, 2025
* Use the same `UdpSocket` in WASIp{2,3}

This commit refactors the implementation of `wasi:sockets` for WASIp2
and WASIp3 to use the same underlying host data structure for the
`UdpSocket` resource in WIT. Previously each version of WASI had its own
socket which resulted in duplicated code. There's some minor differences
between WASIp2 and WASIp3 but it's easy enough to paper over with the
same socket type. This is intended to help with the maintainability of
this going forward to only have one type to operate on rather than two
(which also ensures that bugfixes for one should affect the other).

One other change made in this commit is that sprinkled checks for
whether or not UDP is allowed are all removed and canonicalized during
UDP socket creation. This means that UDP socket creation is the only
location that checks for whether UDP is allowed. Once a UDP socket is
created it can be used freely regardless of whether the UDP setting is
enabled or disabled. This is not intended to have a large practical
effect but it does mean the behavior of hosts that deny UDP but manually
give access to a UDP socket resource to a component may behave subtly
differently.

* Review comments

* Fix p3-less warnings

* Update UDP denial test

* Fix some clippy issues

* Fix no-udp test warnings
bongjunj pushed a commit to prosyslab/wasmtime that referenced this pull request Oct 20, 2025
* Use the same `TcpSocket` in WASIp{2,3}

This is the same as bytecodealliance#11384 except for `TcpSocket` to share the
implementation of TCP across WASIp{2,3} and avoid duplicating code
between the two.

* Fix CI issues

* Review comments

* Trim "TCP allowed?" checks

Like UDP only perform the check on socket creation, not after it's
created.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasi Issues pertaining to WASI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants