bytecodealliance / wasmtime

A fast and secure runtime for WebAssembly
https://wasmtime.dev/
Apache License 2.0
15.44k stars 1.3k forks source link

wasi-http: can we call the `blocking_write_and_flush` method of the `OutgoingRequest` body multiple times? #9653

Open iawia002 opened 3 days ago

iawia002 commented 3 days ago

My scenario is simple: I want to send a request with a large body, but the blocking_write_and_flush method can only write 4096 bytes at a time. Naturally, I decided to call this method multiple times to write the entire body:

let req = OutgoingRequest::new(...);

let outgoing_body = req.body().unwarp();
let request_body = outgoing_body.write().unwarp();

let chunks = buf.chunks(4096);
for chunk in chunks {
    request_body.blocking_write_and_flush(chunk).expect("writing response");
}

OutgoingBody::finish(outgoing_body, None).unwarp();

However, I found that the program gets stuck forever on the second call to blocking_write_and_flush. After debugging, I found that it actually gets stuck during the ready check in the second call:

https://github.com/bytecodealliance/wasmtime/blob/5af89308dc0229ca404cd7000eec694201022e2d/crates/wasi-http/src/body.rs#L662-L669

https://github.com/bytecodealliance/wasmtime/blob/5af89308dc0229ca404cd7000eec694201022e2d/crates/wasi-http/src/body.rs#L472

At this point, the writer has no capacity left, but my writing process hasn't finished, so the reader hasn't started consuming the data (?).

For my issue, I could stop using this method and instead combine check-write, subscribe, write, and flush manually to solve it. However, I'm curious whether we are inclined to allow or not allow this behavior. Because the current behavior is strange—the program doesn't report an error, it just hangs indefinitely.

pchickey commented 3 days ago

Your program getting hung forever on the second blocking_write_and_flush is a bug - you should be able to call that as many times as you want, though it may be less efficient than using check-write/write.

Can you provide a .wasm that reproduces this with the wasmtime cli and we can work on getting it fixed?

iawia002 commented 23 hours ago

you should be able to call that as many times as you want, though it may be less efficient than using check-write/write.

Agree. Here is a program that reproduces this issue:

http.wasm.zip

Source code:

use wasi::{
    http::{
        outgoing_handler,
        types::{Fields, Method, OutgoingBody, RequestOptions, Scheme},
    },
    io::streams,
};

wasi::cli::command::export!(Example);

struct Example;

impl wasi::exports::cli::run::Guest for Example {
    fn run() -> Result<(), ()> {
        let fields = Fields::new();
        let outgoing_request = outgoing_handler::OutgoingRequest::new(fields);
        outgoing_request.set_method(&Method::Post).unwrap();
        outgoing_request.set_scheme(Some(&Scheme::Https)).unwrap();
        outgoing_request.set_authority(Some("httpbin.org")).unwrap();
        outgoing_request.set_path_with_query(Some("/post")).unwrap();

        let outgoing_body = outgoing_request.body().unwrap();
        let body = [0; 5000];
        let request_body = outgoing_body.write().unwrap();

        let chunks = body.chunks(4096);
        for chunk in chunks {
            request_body
                .blocking_write_and_flush(chunk)
                .expect("writing response");
            println!("writing response");
        }
        drop(request_body);
        println!("finished");
        OutgoingBody::finish(outgoing_body, None).unwrap();

        let options = RequestOptions::new();
        let future_response = outgoing_handler::handle(outgoing_request, Some(options)).unwrap();

        let incoming_response = match future_response.get() {
            Some(result) => result.unwrap(),
            None => {
                let pollable = future_response.subscribe();
                pollable.block();

                future_response
                    .get()
                    .expect("incoming response available")
                    .unwrap()
            }
        }
        .unwrap();

        drop(future_response);

        let incoming_body = incoming_response.consume().unwrap();

        drop(incoming_response);

        let input_stream = incoming_body.stream().unwrap();
        let input_stream_pollable = input_stream.subscribe();

        let mut body = Vec::new();
        loop {
            input_stream_pollable.block();

            let mut body_chunk = match input_stream.read(1024 * 1024) {
                Ok(c) => c,
                Err(streams::StreamError::Closed) => break,
                Err(e) => panic!("input_stream read failed: {e:?}"),
            };

            if !body_chunk.is_empty() {
                body.append(&mut body_chunk);
            }
        }

        println!("body: {}", String::from_utf8(body).unwrap());
        Ok(())
    }
}
$ wasmtime -S http target/wasm32-wasip2/debug/http.wasm

writing response  // <- hung forever here

At this point, the writer has no capacity left, but my writing process hasn't finished, so the reader hasn't started consuming the data (?).

I'm not sure if my assumption is correct. This issue could be easily resolved by using an unbounded_channel or increasing the channel's capacity. If that's the case, I can submit a patch to fix it. However, I'm not certain if this is the most efficient solution.