crossterm: Make read sync block for windows systems

The documentation of TerminalInput states for TerminalInput::read_sync:

Readings will be blocking calls.

On windows this will resolve in call to ReadConsoleInput and according to MSDN it blocks.

The function does not return until at least one input record has been read.

But there is a problematic fast return, causing ReadConsoleInput not to be called.

https://github.com/TimonPost/crossterm/blob/8223cc9e4a5e35e15a0287bd9d8aa53dc84d1c34/crossterm_winapi/src/console.rs#L170-L173

Also some events return None, even though further events can be received:

https://github.com/TimonPost/crossterm/blob/8223cc9e4a5e35e15a0287bd9d8aa53dc84d1c34/crossterm_input/src/input/windows_input.rs#L230-L233

While this is valid behaviour for iterators, this causes early returns when iterating with for or while. This should be either changed or documented.

About this issue

Original URL
State: closed
Created 5 years ago
Comments: 15 (4 by maintainers)

Most upvoted comments

Just ran into this issue on the current master branch. I’ve been digging into it (stymied a bit by the difficulty of debugging Rust on Windows, but I’m doing my best) and wanted to share what I’ve found.

Simply removing the length check and early return in read_single_input_event does not resolve the issue. Even with this change, @MSleepyPanda’s toy example continues to exit without going through a single loop of the iteration.

read_single_event (called by SyncReader::next) doesn’t block either when called on its own so the problem lives there somewhere. The console.read_single_input_event() call in read_single_event returns a KeyEvent even if no key is pressed—I don’t know the Windows API so I don’t know whether this is normal or not, but it seems weird. When this pseudo-keypress comes in, read_single_event calls handle_key_event which attempts to parse the pseudo-keypress but fails to do so, and returns an Ok(None) which is what actually appears to lead the iterator to terminate early.

I haven’t checked it out yet but I would speculate that there’s something wrong with how the FFI boundary is getting crossed, leading to the appearance of an event where none exists, specifically pub unsafe fn KeyEvent(&self) -> &KEY_EVENT_RECORD on winapi::um::wincon::INPUT_RECORD_Event, called by read_single_event. But the code gets very macro-y right about there and I haven’t pursued it deeper than that.

I thought I’d share my progress thus far in case somebody can take it farther.

joshhansen on Sep 25, 2019

Iirc this can be resolved by solely removing the length check, as it’s not even used further down in the code, at least in the master branch.

Backporting it to 0.9 might also make sense.

I’m on vacation over the next two weeks and thus not able to create a PR in the meantime, but if its still unresolved by then i’ll give it a shot. If someone is interested, feel free to take it 😃

matprec on Sep 15, 2019