Async::<UnixStream>::connect returning with io::ErrorKind::WouldBlock

[JayceFayne]

I am in the process of moving swayipc from async-std to async-io. While running the unit tests I encountered tests failing with io::ErrorKind::WouldBlock. After re-running the current tests with the latest version of async-std I encountered the same behaviour (since async-std moved to async-io). The unit tests run without failing on async-std version 1.5 since this is the last version using mio-uds. I extracted an example on how to reproduce this bug:

use async_io::Async;
use futures_lite::future;
use std::os::unix::net::UnixStream;
use std::thread;

fn main() {
    let mut handles = Vec::new();
    for _ in 1..100 {
        let handle = thread::spawn(|| {
            future::block_on(async {
                Async::<UnixStream>::connect("pathtosomeunixsocket").await
            })
        });
        handles.push(handle);
    }
    for handle in handles {
        if let Err(error) = handle.join().unwrap() {
            dbg!(error);
        }
    }
}

Thanks for the report! Can you see if this fixes the bug? #19

[JayceFayne]

I tried to debug and fix this bug as well since yesterday and came up with the same solution you propose in #19. Unfortunately this does not fix the problem. Calling connect does not error out with io::ErrorKind::WouldBlock anymore but a write to the socket results in io::ErrorKind::NotConnected. I believe that stream.writable().await? here does not wait until the socket is connected

Hmmm. I wonder what happens if you await writable() twice?

stream.writable().await?;
stream.writable().await?;

If this resolves the issue, then indeed there is a problem in writable().

Do you perhaps have a reproducible example for the issue that remains even after #19 is applied?

[JayceFayne]

Hmmm. I wonder what happens if you await writable() twice?

I tried it out but it did not change anything as far as I can tell. With #19 applied a write to the socket still sometimes fails with io::ErrorKind::NotConnected. The behaviour is pretty inconsistent. It does not happen on every connect/write but rather if there are multiple threads trying to establish a connection (e.g. in unit tests).

Do you perhaps have a reproducible example for the issue that remains even after #19 is applied?

I created a repo with an reproducible example here. It uses the swayipc socket, I have not tested it with other sockets yet.

Ok I was able to reproduce. The error eventually pops up even if I do this:

fn main() -> std::io::Result<()> {
    use futures_lite::io::AsyncWriteExt;
    future::block_on(async {
        for step in 0..500_000 {
            dbg!(step);
            let mut conn =
                Async::<UnixStream>::connect("/run/user/1000/sway-ipc.1000.1358.sock")
                .await?;
            conn.write(&[]).await?;
        }
        Ok(())
    })
}

Error:

[src/main.rs:11] step = 10703
Error: Os { code: 107, kind: NotConnected, message: "Transport endpoint is not connected" }

The same happens with tokio:

#[tokio::main]
async fn main() -> std::io::Result<()> {
    use tokio::prelude::*;
    for step in 0..500_000 {
        dbg!(step);
        let mut conn =
            tokio::net::UnixStream::connect("/run/user/1000/sway-ipc.1000.1358.sock")
            .await?;
        conn.write(&[]).await?;
    }
    Ok(())
}

Perhaps swaywm is just dropping connections occasionally? Maybe if it gets slammed with too many connections at once, it can't keep up so it has to drop some of them?

I also tried adding a sleep after the write() fails - it seems that the connection is never (re)established, which makes me believe that it is not writable() at fault. The socket is not in the "connecting" state". Rather, the connection is simply broken.

[JayceFayne]

Ok I was able to reproduce. The error eventually pops up even if I do this:

Hmm. Interesting, does not seem related to threads after all..

Perhaps swaywm is just dropping connections occasionally? Maybe if it gets slammed with too many connections at once, it can't keep up so it has to drop some of them?

It seem to me pretty unlikely that sway is dropping connections since the unit tests for the swayipc library are just 26 tests. At the moment some of them fail with and without #19. Furthermore establishing connections to sways IPC socket with async-std version 1.5 works without any errors. I also wrote a blocking version of that test here and everything went fine until something overflowed (40k threads/connections) 😄

[JayceFayne]

I also tried adding a sleep after the write() fails - it seems that the connection is never (re)established, which makes me believe that it is not writable() at fault. The socket is not in the "connecting" state". Rather, the connection is simply broken.

Since it is kinda hard to debug this issue by relying on sways IPC I rewrote the example I posted earlier to be "self contained". The test can be run by passing a flag e.g. RUST_LOG=async_io_unixstream_error=trace cargo r -- async-io. blocking and async-std behave (the same and) like expected and do not return io::ErrorKind::NotConnected while async-io does. I hope this helps fixing the io::ErrorKind::NotConnected error.

Ah I see. So async-std v1.5.0 creates a UnixStream connection in a blocking way (using spawn_blocking()) and then creates an async UnixStream handle from a connected sync UnixStream handle.

I believe that when the socket is in blocking mode and the OS exhausts its internal resources, the UnixStream::connect() call will just block until it can connect. And in non-blocking mode connecting fails right away. That's my theory at least, and failing immediately is probably the right thing to do here :)

I was able to reproduce this behavior on my linux VM, but not on macOS. On macOS I get the NotConnected error in all cases.

[JayceFayne]

Ah I see. So async-std v1.5.0 creates a UnixStream connection in a blocking way (using spawn_blocking()) and then creates an async UnixStream handle from a connected sync UnixStream handle.

Yep

I believe that when the socket is in blocking mode and the OS exhausts its internal resources, the UnixStream::connect() call will just block until it can connect. And in non-blocking mode connecting fails right away. That's my theory at least, and failing immediately is probably the right thing to do here :)

Currently calling Async::<UnixStream>::connect does not fail immediately. It rather fails with NotConnected on the next write call. I was able to make it fail fast by adding this line to the connect function. With this patch applied I was also able to fix the example posted earlier by connecting in a loop. Not sure if that's the best way to do this but...it works 😄

I was able to reproduce this behavior on my linux VM, but not on macOS. On macOS I get the NotConnected error in all cases.

I tested all above mentioned changes on linux.

Hmmm, that's interesting! Could you perhaps do me a favor and try one more thing?

Instead of using stream.get_ref().peer_addr(), try stream.get_ref().take_error() like so:

        // The stream becomes writable when connected.
        stream.writable().await?;

        // Check if there was an error while connecting.
        match stream.get_ref().take_error()? {
            None => Ok(stream),
            Some(err) => Err(err),
        }

That should be the cleanest solution.

In fact, if you look at how Async::<TcpStream>::connect() is implemented, it contains this exact same take_error() check at the end. But, I believed this is not necessary when connecting with a UnixStream because mio doesn't do it and I couldn't find any indication that this is necessary in the man pages. But it may be!

[JayceFayne]

Hmmm, that's interesting! Could you perhaps do me a favor and try one more thing?

Instead of using stream.get_ref().peer_addr(), try stream.get_ref().take_error() like so:

        // The stream becomes writable when connected.
        stream.writable().await?;

        // Check if there was an error while connecting.
        match stream.get_ref().take_error()? {
            None => Ok(stream),
            Some(err) => Err(err),
        }

That should be the cleanest solution.

Yes I thought so as well but while testing yesterday I noticed that at least on linux (might be different on macOS?) take_error does not return an error. I saw that the peer field on the debugging output of UnixStream was missing. That's why I ended up using peer_addr instead of take_error. I also tried to add a timeout like you did to see if the socket eventually reaches a connected state (basically by checking peer_addr in a loop) but for some reason it always errors out with NotConnected :/

That's really odd - could be just a quirk in the Linux implementation of Unix domain sockets. :/ But, I would be fine with adding this workaround using peer_addr() anyways. It seems like we need it.

[JayceFayne]

That's really odd - could be just a quirk in the Linux implementation of Unix domain sockets. :/

Maybe...idk. You mentioned that you have a macOS system? I would be really interested if it behaves the same way 😄

But, I would be fine with adding this workaround using peer_addr() anyways. It seems like we need it.

Alright, I will close this issue for now since #19 fixed the initial bug anyway.
Thanks for your help!

I opened #21 regarding the the peer_addr workaround ;)