gyscos / cursive

A Text User Interface library for the Rust programming language
MIT License
4.25k stars 244 forks source link

[BUG] Panic on console resize after cursive.quit #571

Open Szpadel opened 3 years ago

Szpadel commented 3 years ago

Describe the bug I need to take over terminal in the middle of the application (shell) therefore I'm executing cursive.quit(). Only event loop is stopped correctly, and other loops are still working causing issues like this one. That is causing another bug: first key is consumed by cursive inside input handling loop before it shuts down. Loop responsible for handling terminal resize panics on next resize and causes infinite loop somewhere (I didn't investigated it) and hangs one of the threads in 100% cpu usage. It looks like after reentering cursive again window resizing is still working, therefore I assume there might be some resource leak (except this hanged thread).

To Reproduce This can be reproduced in pause example. Just resize window when you are redirected to terminal.

Expected behavior It should not crash and hand threads in background, ideally all loops should be closed, i'm not sure what exactly is causing first key to be consumed by cursive that do not happen in pause example, but probably making sure that all internal loops are canceled should solve also that issue.

Screenshots thread '' panicked at 'called Result::unwrap() on an Err value: "SendError(..)"', .../cursive-0.16.3/src/backends/resize.rs:20:40

Environment

Additional context I found some hints inside termion that suggesting that stdin might not be canceled without getting input or EOF, therefore nice thing to have would be to be able to take or borrow running stdin handled from cursive.

gyscos commented 3 years ago

Hi, and thanks for the report!

Indeed, the pause mechanism is still relatively new, and the termion backend in particular uses a few separate threads that apparently aren't properly stopped when the backend finishes.

I just a patch which should help at least no crash in this case. I think we might consume one "resize" signal past the end of the backend, which could potentially interfere with the code running while cursive is paused.

Szpadel commented 3 years ago

That's why I think exposing those could be better alternative.

Or probably providing both options to choice from, since someone code might depend on accessing stdin directly, but therefore might be no other choice with first event.

Regarding resize event, it looks like terminals send few of those with every resize, and it was handled correctly (except this panic) even in tmux terminal

PS: Thanks for your work on this library, it's amazing!