Skip to content

Implement the remaining socket-related WASI functions.#4776

Merged
sunfishcode merged 4 commits into
bytecodealliance:mainfrom
sunfishcode:sunfishcode/send-recv
Aug 26, 2022
Merged

Implement the remaining socket-related WASI functions.#4776
sunfishcode merged 4 commits into
bytecodealliance:mainfrom
sunfishcode:sunfishcode/send-recv

Conversation

@sunfishcode
Copy link
Copy Markdown
Member

The original WASI specification included sock_read, sock_write, and
shutdown. Now that we have some sockets support, implement these
additional functions, to make it easier for people porting existing code
to WASI.

It's expected that this will all be subsumed by the wasi-sockets
proposal, but for now, this is a relatively small change which should
hopefully unblock people trying to use the current accept support.

The original WASI specification included `sock_read`, `sock_write`, and
`shutdown`. Now that we have some sockets support, implement these
additional functions, to make it easier for people porting existing code
to WASI.

It's expected that this will all be subsumed by the wasi-sockets
proposal, but for now, this is a relatively small change which should
hopefully unblock people trying to use the current `accept` support.
@sunfishcode sunfishcode force-pushed the sunfishcode/send-recv branch from 373b427 to 79a5481 Compare August 24, 2022 20:47
@github-actions github-actions Bot added the wasi Issues pertaining to WASI label Aug 24, 2022
@github-actions
Copy link
Copy Markdown

Subscribe to Label Action

cc @kubkon

Details This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

  • kubkon: wasi

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

Copy link
Copy Markdown
Contributor

@jameysharp jameysharp left a comment

Choose a reason for hiding this comment

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

Looks right to me! edit: although apparently CI disagrees 😅

@sunfishcode
Copy link
Copy Markdown
Member Author

Windows disagrees 🙃

@jameysharp
Copy link
Copy Markdown
Contributor

I assume cargo-vet is going to need a delta audit for system-interface.

@stevelr
Copy link
Copy Markdown

stevelr commented Aug 25, 2022

I am not sure I see how sock_shutdown is intended to be used if the socket is in use by a wasm module.

Scenario: the host creates a tcp listener socket bound to an address, and adds it to WasiCtx as a preopened_socket. The wasm module creates a listener via the wasi api and calls accept().

Sometime later, while wasm module is still waiting on accept, the host wants to stop the module. It can't directly call sock_shutdown because neither TcpListen nor Socket are cloneable, so it can't hold onto a cloned object to call shutdown on it later. If the host kept the u32 file descriptor, it could either recreate a Socket from the fd and call sock_shutdown, or pass the fd to a low-level unix function to close it, which seems extremely non-portable. There is a try_clone and if it succeeds it creates an alias file descriptor, but it's not clear from the documentation if it would work for this purpose.

What is the recommended method of closing a socket in this scenario?

@sunfishcode
Copy link
Copy Markdown
Member Author

@stevelr shutdown doesn't do a close; it just marks a stream as no longer readable, writeable, or both. The file descriptor remains open. And it isn't required to be called.

The main use case for it that I know is for one side of a socket to declare that it's done writing to a stream and will subsequently only be reading the response.

@sunfishcode sunfishcode merged commit 9b3477f into bytecodealliance:main Aug 26, 2022
@sunfishcode sunfishcode deleted the sunfishcode/send-recv branch August 26, 2022 18:39
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.

3 participants