BiagioFesta / wtransport

Async-friendly WebTransport implementation in Rust
Apache License 2.0
460 stars 31 forks source link

Error when using my own Tokio runtime #137

Closed cBournhonesque closed 6 months ago

cBournhonesque commented 9 months ago

Hi,

When I use the library inside the main tokio runtime created by #[tokio::main] everything was working fine.

But now I've had a case where I need to create the wtransport connection in a different thread, so I would like to create my own tokio runtime.

I tried using

 let rt = tokio::runtime::Runtime::new().expect("Failed building the Runtime");
        let _guard = rt.enter();
        let endpoint = wtransport::Endpoint::client(config).unwrap();
        tokio::spawn(async move {
            info!("before wtransport connect");
            let connection = endpoint
                .connect(&server_url)
                .await
                .inspect_err(|e| {
                    error!("failed to connect to server: {:?}", e);
                })
                .unwrap();
             ...
        })

but it panics with

thread 'tokio-runtime-worker' panicked at wtransport-0.1.10/src/endpoint.rs:325:14:
QUIC connection parameters must be validated: EndpointStopping

I've looked a bit through the code, and I see that this EndpointDriver must be kept around; but I'm not sure what I'm doing wrong

BiagioFesta commented 9 months ago

Hi, Thank you for reporting this.

I've tried to create a MRE (minimum reproducible example), but with no success.

I've tried the following code, but I understand you probably have multi thread context.

use tracing::error;
use tracing::info;

fn main() {
    tracing_subscriber::fmt::init();

    let config = wtransport::ClientConfig::builder()
        .with_bind_default()
        .with_no_cert_validation()
        .build();

    let rt = tokio::runtime::Runtime::new().expect("Failed building the Runtime");
    let _guard = rt.enter();
    let endpoint = wtransport::Endpoint::client(config).unwrap();

    let t = tokio::spawn(async move {
        info!("before wtransport connect");

        let _connection = endpoint
            .connect("https://localhost:4433")
            .await
            .inspect_err(|e| {
                error!("failed to connect to server: {:?}", e);
            })
            .unwrap();

        info!("Done");
    });

    rt.block_on(t).unwrap();
}

Would it be possible to have a minimum example I can try to reproduce with? Thank you!

BiagioFesta commented 9 months ago

I've looked a bit through the code, and I see that this EndpointDriver must be kept around; but I'm not sure what I'm doing wrong

Yeah, it seems the driver task (loop of the QUIC connection) terminated. However, in your snippet of code you are moving the Endpoint inside the async block, so that makes me think it's more a failure in the driver, and thus it terminates itself.

Probably it is related to the async context and where stuff are created, that's why having a reproducible example would help me a lot

cBournhonesque commented 9 months ago

Thanks for looking into this; I don't really know if it's easy to provide a minimal example that is simple to run. I ran into it here: https://github.com/cBournhonesque/lightyear/tree/main/examples/simple_box after I made some changes that means that I create the connection outside of the #[tokio::main] thread.

I actually found a solution that works for me: using the async_compat (https://docs.rs/async-compat/latest/async_compat/) crate in combination with a non-tokio runtime works! Something like:

IoTaskPool::get()
            .spawn(Compat::new(async move {
                let endpoint = wtransport::Endpoint::client(config)
                    .inspect_err(|e| error!("could not create endpoint: {:?}", e))
                    .unwrap();
                    ...
                    }));

I can try creating a minimal example

MOZGIII commented 7 months ago

The expect that produces the QUIC connection parameters must be validated is a mistake imo - I should be able to handle it. It can fail for numerous runtime reasons - like to routing table for an interface being present. A lot can't be sensibly validated without doing what the fn under expect (quinn::Endpoint::connect) does.

BiagioFesta commented 7 months ago

Agreed, gonna fix that

BiagioFesta commented 7 months ago

The expect that produces the QUIC connection parameters must be validated is a mistake imo - I should be able to handle it. It can fail for numerous runtime reasons - like to routing table for an interface being present. A lot can't be sensibly validated without doing what the fn under expect (quinn::Endpoint::connect) does.

Fixed here: https://github.com/BiagioFesta/wtransport/commit/3b549107606d5bb00e0aa080aa0e33d90a176abb