TcpStream times out while connecting in tokio

[rupansh]

async-std does not have this issue as it automatically detects blocking code and optimizes it

[glebpom]

Timeout still occurs. Moving socket.set_nonblocking(true)?; behind socket.set_nodelay(true)?; fixes it

[glebpom]

Non-blocking tokio version (by converting to socket2 Socket)

        let socket: socket2::Socket = stream.into_std()?.into();
        socket.set_keepalive(Some(KEEPALIVE_TIME))?;
        let stream = TcpStream::from_std(socket.into())?;

[glebpom]

It seems like enabling keepalive on async-std asynchronous socket may be implemented like that:

    ...
    #[cfg(unix)]
        let socket = socket2::Socket::from_raw_fd(stream.into_raw_fd());
    #[cfg(windows)]
        let socket = socket2::Socket::from_raw_socket(stream.into_raw_socket());

    socket.set_keepalive(Some(KEEPALIVE_TIME))?;

    #[cfg(unix)]
        let stream = TcpStream::from_raw_fd(stream.into_raw_fd());
    #[cfg(windows)]
        let stream = TcpStream::from_raw_socket(stream.into_raw_socket());
    ...

[rupansh]

It seems like enabling keepalive on async-std asynchronous socket may be implemented like that:

    ...
    #[cfg(unix)]
        let socket = socket2::Socket::from_raw_fd(stream.into_raw_fd());
    #[cfg(windows)]
        let socket = socket2::Socket::from_raw_socket(stream.into_raw_socket());

    socket.set_keepalive(Some(KEEPALIVE_TIME))?;

    #[cfg(unix)]
        let stream = TcpStream::from_raw_fd(stream.into_raw_fd());
    #[cfg(windows)]
        let stream = TcpStream::from_raw_socket(stream.into_raw_socket());
    ...

are you facing timeouts on async-std too? Also apologies I am not home so I won't be able to push changes for a few days.

[glebpom]

I don't use async-std, but having blocking code running in a non-blocking reactor should be avoided if possible.

[rupansh]

I don't use async-std, but having blocking code running in a non-blocking reactor should be avoided if possible.

I dont see the need to fix something that doesn't need fixing.
async-std uses the normal Blocking std::net::TcpStream for its TcpStream and wraps it around a watcher: https://docs.rs/async-std/1.9.0/src/async_std/net/tcp/stream.rs.html#50

Also check: https://async.rs/blog/stop-worrying-about-blocking-the-new-async-std-runtime/
It should be doing something similar for the blocking part here.
As such I would like to limit the discussion to Tokio only, unless a change breaks async-std. Thanks

[glebpom]

As you wish, but the implementation of Async::<std::net::TcpStream>::connect(addr), is backed by async-io crate - https://docs.rs/async-io/1.3.1/src/async_io/lib.rs.html#1179 which relies on nb_connect crate - "Non-blocking TCP or Unix connect."

[seanpianka]

I don't use async-std, but having blocking code running in a non-blocking reactor should be avoided if possible.

I think this should be raised as a separate issue in the main stripe-rust repository. We should bring it to the attention of the maintainers, as if this is a problem, the maintainers should want it fixed in the main project repository.

[seanpianka]

Timeout still occurs. Moving socket.set_nonblocking(true)?; behind socket.set_nodelay(true)?; fixes it

@glebpom
I am using the version at 0e4d9bb that has one additional commit containing your timeout patch from the upstream repo's discussion, @glebpom. Where can I find your fork with this updated fix for the tokio timeouts?

The stripe-rust maintainers indicated that there was an issue with the original patch, IIRC.

[rupansh]

@glebpom Hi I have pushed your fix to the branch. Confirm it just in case and I will merge it with master.

[glebpom]

@rupansh this version doesn't timeout

[rupansh]

thanks for the help.