[BUG] Termion backend drops some characters for subsequent reads from stdin

[arxanas]

Describe the bug

When using the termion back-end and then reading from stdin, up to two characters are dropped.

To Reproduce

Simple modification of the pause example at https://github.com/gyscos/cursive/blob/main/examples/src/bin/pause.rs:

fn main() {
    let mut siv = cursive::termion();
    siv.add_layer(
        cursive::views::Dialog::text("Please write your message.")
            .button("Ok", |s| s.quit()),
    );
    siv.run();

    println!("Enter your message here:");
    let mut line = String::new();
    std::io::stdin().read_line(&mut line).unwrap();

    println!("Your message was: {:?}", line);
}

Expected behavior:

Enter your message here:
hello
Your message was: "hello\n"

Actual behavior:

Enter your message here:
hello
Your message was: "llo\n"

Note that the first two characters of the provided message have been dropped.

Environment

• Operating system used: macOS 10.15.7
• Backend used: termion
• Current locale:

LANG="en_US.UTF-8"
LC_COLLATE="en_US.UTF-8"
LC_CTYPE="en_US.UTF-8"
LC_MESSAGES="en_US.UTF-8"
LC_MONETARY="en_US.UTF-8"
LC_NUMERIC="en_US.UTF-8"
LC_TIME="en_US.UTF-8"
LC_ALL=

• Cursive version: 0.16.3

Analysis

I believe the main problem is this loop in combination with the implementation of termion:

cursive/cursive/src/backends/termion.rs

Lines 93 to 108 in 7cf597f

	// We want nonblocking input, but termion is blocking by default
	// Read input from a separate thread
	thread::spawn(move || {
	let mut events = input_file.events();
	// Take all the events we can
	while let Some(Ok(event)) = events.next() {
	// If we can't send, it means the receiving side closed,
	// so just stop.
	if input_sender.send(event).is_err() {
	break;
	}
	}
	running.store(false, Ordering::Relaxed);
	});

When you ask termion for the next event, it reads up to two bytes, which is why specifically up to two characters are dropped:

https://github.com/redox-os/termion/blob/dce5e7500fd709987f9bf8f3911e4daa61d0ad14/src/input.rs#L60-L85

Even after the Cursive instance is dropped, the input thread is still running in the background. Once data has been written to stdin, this thread will then read up to two bytes and attempt to run input_sender.send(event), which will fail and then cause the thread to terminate. The calling code may then attempt to read from stdin, but the bytes are already gone.

The ideal fix would be to terminate the input thread once the Cursive instance has been dropped, but I can't figure out a good way to do that, since the call to events.next() is fundamentally blocking. It might be possible to use a variant of select to poll the input to see if it has data before calling events.next(), but that seems complicated and non-portable.

I haven't been able to figure out a good workaround for user code. In particular, I expected that creating the Termion back-end with /dev/tty would fix the issue, but it doesn't seem to.

Termion has an item called termion::async_stdin, which might be relevant towards fixing this issue.

[gyscos]

Hi, and thanks for the report!

Indeed, I had a feeling this was going to be a problem. Unless the low-level "reading from stdin" is non-blocking (or allow peeking, or allow "putting bytes back"), there is no way to ask for a byte, and then cancel the request.

termion::async_stdin() unfortunately is entirely unrelated to await/async, and instead has roughly the same behaviour as ours, spinning a thread reading from input in a loop, sending all bytes through a channel.

A partial workaround is to avoid starting the blocking call if we know we're about to exit. Here for example, after the Enter key is read, the cursive event loop terminates, and we don't need to read anything else, so it should be possible to exit without consuming anything extra. It doesn't work when Cursive::quit is called from a callback sent from another thread though. It would complicate a bit the backend interface, but if nothing else works it's one way we can go.

I mentioned above "putting bytes back" - I believe it's actually possible to write to the current process' stdin and inject any bytes we over-consumed there. Only whatever we write will be appended to what's already buffered there. Say we over-consume the "he" from your example. If "llo\n" is already buffered and we write back the "he" we read, we'll see "llo\nhe". The solution here would be to atomically read everything in stdin (not just the first couple bytes), then write it all back. But 1) I don't think we can actually do that atomically, we can just be fast and pray, and 2) I don't know how to "read everything currently buffered, then stop" because of the whole blocking read thing. :S

The ideal solution would be to have actual non-blocking input. I think I saw at some point some code opening stdin (or was it /dev/tty?) in non-blocking mode, I'll try to find that again.