WASI-http needs a sequence diagram

[brendandburns]

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.

[lukewagner]

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-handles branch 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 (own and borrow), incoming-response.consume would currently return an own<incoming-stream>, indicating that the caller has complete ownership of the incoming-stream and in particular is allowed to write the code you showed above. This causes two problems: (1) the implementation must internally keep the incoming-response alive until the incoming-stream is dropped which adds complexity and perhaps bugs, and (2) it's raises questions of what happens if I transfer ownership of the incoming-response (by calling handle) while still holding onto a derived incoming-stream.

To avoid both of these problems, I think we instead want the incoming-stream returned by consume to be treated as a child resource of the incoming-response so that if you destroy or transfer the incoming-response, you can't still be holding onto a child incoming-stream. The proposal in the child-handles branch adds a child-own type that says this directly, allowing you to write:

  incoming-request-consume: func(self: borrow<incoming-request>) -> result<child-own<incoming-stream, self>>

(exact Wit syntax TBD) which says that the returned incoming-stream is a child of the incoming-request passed as the self parameter. The runtime rules of the component-model enforce this by trapping if the handle to the incoming-request is dropped before the handle to the incoming-stream, thus it's not even a question the HTTP proposal has to answer explicitly; it's implied by the use of child-own.

In the interim, I'd suggest having the host implementation of drop-incoming-request cause a guest trap if it's called while there are any outstanding un-dropped streams.

WDYT?

[brendandburns]

I'm not sure what the value of child-own is. 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 deleting drop-incoming-stream entirely 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]

child-own is 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 consume to be called again after the stream returned by a previous call to consume was 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]

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.