Repository navigation
WASI-http needs a sequence diagram #818
Description
There's a number of functions that allocate/return values (e.g. get a stream for a response body) and methods which dispose of those values (e.g. drop_output_stream).
It's not very clear from the documentation or from the method signatures what is legal and what is a a problem.
e.g. if I call:
let body_stream = types::incoming_response_consume(incoming_response)
.map_err(|()| anyhow!("incoming response has no body stream"))?;
types::drop_incoming_response(incoming_response);
let mut body = Vec::new();
let mut eof = false;
while !eof {
let (mut body_chunk, stream_ended) = streams::read(body_stream, u64::MAX)?;
eof = stream_ended;
body.append(&mut body_chunk);
}
streams::drop_input_stream(body_stream);Note I dropped the response before I dropped the stream from the response, is that legal? Who owns the stream? Is it the caller or is it the response object?
Similarly, given the code:
let request =
types::new_outgoing_request(method, path, query, Some(scheme), authority, headers);
let request_body = types::outgoing_request_write(request)
.map_err(|_| anyhow!("outgoing request write failed"))?;
let mut body_cursor = 0;
while body_cursor < body.len() {
let written =
streams::write(request_body, &body[body_cursor..]).context("writing request body")?;
body_cursor += written as usize;
}
streams::drop_output_stream(request_body);
let future_response = default_outgoing_http::handle(request, None);
types::drop_outgoing_request(request);Note that I dropped the output stream before I sent the request, so that seems to imply that the request object owns the output stream.
Given the complexity of the various methods to acquire things like streams and responses and requests and all of the methods to drop them, it is very difficult to know the correct way to either call the code or implement the WIT interface. This means that clients will be confused, and different implementations will behave differently.
We need to either publish a set of acceptance tests that all implementations must be able to run successfully, and/or document the sequence of calls that are "expected" or "acceptable" so that we can have consistency.
Activity
lukewagner commentedon Apr 12, 2023
Yeah, we've got a long way to go to properly document this interface and sequence diagrams are a good idea.
Coincidentally, the thing I'm putting finishing touches on now (in the
child-handlesbranch in the component-model repo) is intended to answer exactly the two questions you're asking above, at the component-model level based solely on the handle type (as opposed to the per-interface level based on documentation).So given the two currently-proposed handle types (
ownandborrow),incoming-response.consumewould currently return anown<incoming-stream>, indicating that the caller has complete ownership of theincoming-streamand in particular is allowed to write the code you showed above. This causes two problems: (1) the implementation must internally keep theincoming-responsealive until theincoming-streamis dropped which adds complexity and perhaps bugs, and (2) it's raises questions of what happens if I transfer ownership of theincoming-response(by callinghandle) while still holding onto a derivedincoming-stream.To avoid both of these problems, I think we instead want the
incoming-streamreturned byconsumeto be treated as a child resource of theincoming-responseso that if you destroy or transfer theincoming-response, you can't still be holding onto a childincoming-stream. The proposal in thechild-handlesbranch adds achild-owntype that says this directly, allowing you to write:(exact Wit syntax TBD) which says that the returned
incoming-streamis a child of theincoming-requestpassed as theselfparameter. The runtime rules of the component-model enforce this by trapping if the handle to theincoming-requestis dropped before the handle to theincoming-stream, thus it's not even a question the HTTP proposal has to answer explicitly; it's implied by the use ofchild-own.In the interim, I'd suggest having the host implementation of
drop-incoming-requestcause a guest trap if it's called while there are any outstanding un-dropped streams.WDYT?
brendandburns commentedon Apr 15, 2023
I'm not sure what the value of
child-ownis. If I own the parent, then I should probably not have to worry about owning the children also, right? I think I'd favor actually just deletingdrop-incoming-streamentirely because what's the point of forcing the developer to drop a reference to a child resource when they are also going to drop the reference to the parent. It feels like forcing the developer to implement deep-delete on their own (and it also forces the implementation to do book-keeping which is effectively reference counting and as you noted prone to bugs).lukewagner commentedon Apr 17, 2023
child-ownis useful when the child resource can be destroyed well before the parent resource and thus needs a separate resource with an independent lifetime. This is most clearly the case for resources like files where you can have multiple reading/writing streams at the same time and over time for the same parent file resource.For requests/responses, there's at most 1 stream and so it's less clear. In general, it's useful for all the different uses of streams in WASI to be symmetric with how they use streams so that the bindings for wasi-io (and later Preview 3 native streams) can be shared. But also, in the future, it might be useful to relax the rules to allow
consumeto be called again after the stream returned by a previous call toconsumewas destroyed (picking up where the last stream left off); this could perhaps be useful in service-chaining types of scenarios where one component consumes one portion of the stream before passing the request off to another component that consumes the rest.To avoid burdening the programmer, the guest bindings can maintain a list of child resources and automatically close them when the parent handle is destroyed or transferred. This is what I was assuming would happen for scripting language bindings (where each handle is given an object wrapper that switches to a throw-on-access state when the underlying handle goes away). Having the guest maintain this bookkeeping instead of the host makes sense b/c often, in addition to dropping the child handles, there is some amount of guest state that needs to be updated.
brendandburns commentedon Apr 17, 2023
I'm good with the "closing the parent, closes the children" behavior with "closing the child" being an optional optimization that allows the client to release resources.
I will send a PR to update the docs to add clarity.