Add rebind_with_abstract_socket

[ghost]

1432

[djc]

Can you add a bit more context about what you need this for/what your use case is?

[ghost]

i will not using std::net::UdpSocket because std is not async

[djc]

I still don't understand what problem you're trying to solve.

[ghost]

Converting a synchronous socket to an asynchronous socket is weird, why not just use an asynchronous socket.

[ghost]

pub fn tokio_server(
    config: quinn::ServerConfig,
    socket: tokio::net::UdpSocket,
) -> std::io::Result<quinn::Endpoint> {
    quinn::Endpoint::new_with_abstract_socket(
        create_ep_config(),
        Some(config),
        AsyncSocket {
            io: socket,
            inner: quinn::udp::UdpSocketState::new(),
        },
        Arc::new(AsyncRuntime),
    )
}

#[derive(Debug)]
struct AsyncRuntime;

impl quinn::Runtime for AsyncRuntime {
    fn new_timer(&self, i: std::time::Instant) -> std::pin::Pin<Box<dyn quinn::AsyncTimer>> {
        Box::pin(tokio::time::sleep_until(i.into()))
    }

    fn spawn(&self, future: std::pin::Pin<Box<dyn std::future::Future<Output = ()> + Send>>) {
        tokio::spawn(future);
    }

    fn wrap_udp_socket(
        &self,
        _: std::net::UdpSocket,
    ) -> std::io::Result<Box<dyn quinn::AsyncUdpSocket>> {
        unimplemented!()
    }
}

[ghost]

This may prove that wrap_udp_socket api is not needed

[ghost]

I still can't understand why a protocol library needs to call the system api directly instead of changing the requirement of asynchronous sockets

[djc]

tokio's TcpStream has an into_std() method as well, so if you have a tokio TcpStream that you'd like to convert to a dyn AsyncUdpSocket I think that should make that easier? IIRC the main reasons Quinn has its own abstraction on top of the system APIs are (a) support for Explicit Congestion Notifications (ECN) and (b) performance.

[ghost]

If you want to support ecn, why not extend the AsyncUdpSocket trait

[ghost]

pub fn tokio_server(
    config: quinn::ServerConfig,
    socket: tokio::net::UdpSocket,
) -> std::io::Result<quinn::Endpoint> {
    quinn::Endpoint::new_with_abstract_socket(
        create_ep_config(),
        Some(config),
        AsyncSocket {
            io: socket,
            inner: quinn::udp::UdpSocketState::new(),
        },
        Arc::new(AsyncRuntime),
    )
}

#[derive(Debug)]
struct AsyncRuntime;

impl quinn::Runtime for AsyncRuntime {
    fn new_timer(&self, i: std::time::Instant) -> std::pin::Pin<Box<dyn quinn::AsyncTimer>> {
        Box::pin(tokio::time::sleep_until(i.into()))
    }

    fn spawn(&self, future: std::pin::Pin<Box<dyn std::future::Future<Output = ()> + Send>>) {
        tokio::spawn(future);
    }

    fn wrap_udp_socket(
        &self,
        _: std::net::UdpSocket,
    ) -> std::io::Result<Box<dyn quinn::AsyncUdpSocket>> {
        unimplemented!()
    }
}

I wrote this code simply because I don't want to use std udp sockets

[djc]

I don't think there's anything actionable for us to do with this issue.

[ghost]

pub fn rebind_with_abstract_socket(&self, socket: Box<dyn AsyncUdpSocket>) -> io::Result<()> {
    let addr = socket.local_addr()?;
    let mut inner = self.inner.state.lock().unwrap();
    inner.socket = socket;
    inner.ipv6 = addr.is_ipv6();

    // Generate some activity so peers notice the rebind
    for sender in inner.connections.senders.values() {
        // Ignoring errors from dropped connections
        let _ = sender.send(ConnectionEvent::Ping);
    }

    Ok(())
}

[ghost]

Is this code actionable?

[djc]

It's at least clearer to me what you're asking, but "I don't want to use std udp sockets" doesn't seem like enough justification for us to maintain some API you want.

[ghost]

The wrap_udp_socket function is called in four places, all of which can be changed to the following mode Maybe we shoud put runtime-specific code in crate quinn-tokio quinn-asyncstd ...

async fn api_foo(socket:std::net::UdpSocket ...) -> ... {
    let socket = self.runtime.wrap_udp_socket(socket)?;
    ...
}

async fn api_foo_with_abstract_socket(socket:Box<dyn AsyncUdpSocket> ...) -> ... {
    ...
}

[ghost]

This can make it easier for other runtimes to support quinn. This removes the runtime-specific parts of the quinn library. Is this a good reason?