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

Add optional close/shutdown message to AsyncWriteStream #9680

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

itowlson
Copy link
Contributor

Summary

This proposal adds an optional Receiver<()> field to AsyncWriteStream. When signalled, this gracefully closes the underlying AsyncWrite.

Context

This emerged from trying to implement wasi-blobstore. I was passing the read end of a stream to a back-end provider, and the write end to the guest. I was using wasmtime_wasi to store the write end in a resource table as a HostOutputStream because I didn't want to write a stream host interface myself!

The API specifies that the guest calls a "finish" method (on a separate outgoing-value resource) to indicate that it has finished writing to the stream. However, because I was using AsyncWriteStream and wasmtime_wasi::HostOutputStream, my host didn't have access to the underlying AsyncWrite's shutdown function to EOF the stream. So the reader at the back end would continue waiting for input.

My first plan was to add a shutdown function to AsyncWriteStream, but I couldn't downcast HostOutputStream to AsyncWriteStream, so that didn't work. So what I ended up with was:

  • Have AsyncWriteStream accept the receiver end of a sync channel.
  • Have AsyncWriteStream run a background task which waited on the receiver. When it received a message, it would enqueue a shutdown of the underlying AsyncWrite.
  • Have the manager of the outbound connection (in wasi-blobstore, an outgoing-value resource) hold the sender end of the channel.
  • When the guest called outgoing-value::finish, send a message via the sender.

This seemed to work, although it's not been extensively tested.

As an interface, it's awkward, because for most operations you interact with AsyncWriteStream/HostOutputStream via methods, but for this one special case you interact with it via a sync channel. However, I don't see a way round that without adding a shutdown or close method to the WASI output-stream resource. It's possible that WASI intends that shutdown be done by dropping the output-stream resource, but in my testing this seemed to result in an abort rather than an EOF.

So after talking to @alexcrichton I'm putting this up for discussion and in the hope that WASI folks can come up with a better solution. Please let me know if folks need clarifications around the problem I was trying to solve or the constraints that led me in this direction!

@github-actions github-actions bot added the wasi Issues pertaining to WASI label Nov 25, 2024
@pchickey
Copy link
Contributor

pchickey commented Nov 27, 2024

The prior art for shutdown is in the wasi-sockets implementation here: https://github.com/bytecodealliance/wasmtime/blob/main/crates/wasi/src/tcp.rs#L641

Rather than shutdown be a method on a stream, its a method on the parent of the stream, in this case the tcp-socket https://github.com/WebAssembly/wasi-sockets/blob/main/wit/tcp.wit#L361-L385.

It looks like this finish method consuming parent resource outgoing-value in wasi-blobstore follows an approximately similar architecture. It has the added advantage that the output-stream is already guaranteed to be dropped at that point. Cn you follow the architecture used by wasmtime-wasi's sockets in your implementation?

@itowlson
Copy link
Contributor Author

Thanks for the pointers @pchickey. I'll try to implement that pattern.

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