Suggestion: Reading and writing streams simultaneously without threads

[janko]

Hello Piotr 👋

This is more of a feature request, as I haven't used tty-command yet and experienced an issue with it. However, I would like to suggest a reliability improvement for tty-command, which is to write to stdin and read from stdout/stderr simultaneously without threads, using the nonblocking APIs.

Let me explain. Some MiniMagick users have been experiencing deadlocks with the shell command execution. MiniMagick uses Open3.capture3, which spawns threads for reading stdout and stderr. The guess was that somewhere during thread context switching Ruby thinks it's in a deadlock, even though stdout and stderr pipes are being emptied.

The Basecamp team made pull request to use posix-spawn's simultaneous reading and writing from streams, which you can find in POSIX::Spawn::Child#read_and_write. That change fixed deadlocks for Basecamp, so I think it might fix potential deadlocks in tty-command. A bonus is that no additional threads need to be spawned, which is always nice.

What do you think?

[piotrmurach]

Hi Janko!

Thanks for getting in touch and sharing your experience!

The stdin and stdout/stderr streams are already non-blocking. I use stream select to read/write when any data becomes available. The stdin stream itself isn't consumed inside of any threads, only the stdout/stderr are processed in threads. I'm not sure how Open3.capture3 is implemented but it sounds fairly similar.

Some time ago there was an issue reported about threads deadlocking but it was due to me screwing up threads synchronization which has since been fixed.

I agree if the aim can be achieved without using threads then probably that's preferrable. I will take a look.

[janko]

Yeah, I noticed that you are using the nonblocking API, as opposed to the more primitive

stdout_reader = Thread.new { stdout_pipe.read }
stderr_reader = Thread.new { stderr_pipe.read }

approach that Open3.capture3 uses. I'm just not sure if that solves the potential issue, because the IO.select calls in the threads are still blocking (if I understand IO.select correctly). Not saying that there is an issue; maybe IO#read does something that would cause a deadlock while your code doesn't.

Anyway, it's just something that I really liked from posix-spawn and wanted to suggest here as well, thanks for considering it 😃