Bug: stream needs to be spawned in new thread

[emhane]

The connect method on UtpSocket<P> fails when called in the same thread that we bound to the UtpSocket.
This code with a stream created in tokio::spawn ran in stream.rs passes

#[cfg(test)]
mod test {
    use crate::conn::ConnectionConfig;
    use crate::socket::UtpSocket;
    use std::net::SocketAddr;
    #[tokio::test]
    async fn test_start_stream() {
        // set-up test
        let sender_addr = SocketAddr::from(([127, 0, 0, 1], 3400));
        let receiver_address = SocketAddr::from(([127, 0, 0, 1], 3401));

        let config = ConnectionConfig::default();
        let sender = UtpSocket::bind(sender_addr).await.unwrap();

        tokio::spawn(async move {
            let mut tx_stream = sender.connect(receiver_address, config).await.unwrap();
            // write 100k bytes data to the remote peer over the stream.
            let data = vec![0xef; 100_000];
            tx_stream
                .write(data.as_slice())
                .await
                .expect("Should send 100k bytes");
        });
    }
}

and this code creating a stream in the same thread fails with error thread 'stream::test::test_start_stream panicked at 'called Result::unwrap() on an Err value: Kind(TimedOut)', src/stream.rs:137:76, on my branch improving error handling the error is more helpful thread 'stream::test::test_start_stream panicked at 'called Result::unwrap() on an Err value: ConnInit(MaxConnAttempts)', src/stream.rs:146:76.

#[cfg(test)]
mod test {
    use crate::conn::ConnectionConfig;
    use crate::socket::UtpSocket;
    use std::net::SocketAddr;
    #[tokio::test]
    async fn test_start_stream() {
        // set-up test
        let sender_addr = SocketAddr::from(([127, 0, 0, 1], 3400));
        let receiver_address = SocketAddr::from(([127, 0, 0, 1], 3401));

        let config = ConnectionConfig::default();
        let sender = UtpSocket::bind(sender_addr).await.unwrap();

        let mut tx_stream = sender.connect(receiver_address, config).await.unwrap();
        // write 100k bytes data to the remote peer over the stream.
        let data = vec![0xef; 100_000];
        tx_stream
            .write(data.as_slice())
            .await
            .expect("Should send 100k bytes");
    }
}

The reason for why this is the case needs to be identified, it might be the root of several bugs. It could be that the explanation is straight forward and then the thread spawning code should simply be abstracted away into the connect function since connect is part of the public API and this peculiar requirement is not documented.

[njgheorghita]

Looking into this, it seems like the reason the initial case (where we spawn the connect in a separate thread) is passing is only b/c we're not awaiting the handle. At least locally, if we await the handle in the first case, then we get the same error as emitted in the second case.

[njgheorghita]

I was able to modify the above test to reach a passing state

    #[tokio::test]
    async fn test_transfer_100k_bytes() {
        // set-up test
        tracing_subscriber::fmt::init();
        let sender_addr = SocketAddr::from(([127, 0, 0, 1], 3400));
        let receiver_addr = SocketAddr::from(([127, 0, 0, 1], 3401));

        let sender = UtpSocket::bind(sender_addr).await.unwrap();
        let receiver = UtpSocket::bind(receiver_addr).await.unwrap();

        let config = ConnectionConfig::default();

        // write 100k bytes data to the remote peer over the stream.
        let data = vec![0xef; 100_000];
        let rx_handle = tokio::spawn(async move {
            receiver.accept(config).await.unwrap();
        });

        let mut tx_stream = sender.connect(receiver_addr, config).await.unwrap();
        tx_stream
            .write(data.as_slice())
            .await
            .expect("Should send 100k bytes");

        rx_handle.await.unwrap();
    }

Imo, it doesn't appear as though this issue identified a bug, but it is an indication that this library needs better documentation on how to use it's public api.

1. @emhane In your example, I don't believe that we can expect for the sender to be able to connect with the receiver, without the receiver accepting the connection (as is done in my example)
2. As to whether should connect() be responsible for spawning a thread, or is the user responsible for managing this appears to be a design decision to me. Right now, I don't have a reasoned opinion as to which way is better. Just that the way in which this api is intended to be used should be clearly & correctly shown via examples in the README.

[emhane]

Great work! Standard is to document public API. @njgheorghita

[emhane]

Closed for #53