jrnxf / thokr

✨ sleek typing tui with visualized results and historical logging
MIT License
519 stars 18 forks source link

Fix unwrap() causing panic at exit on Windows. #5

Closed dmitr101 closed 2 years ago

dmitr101 commented 2 years ago

What: Fix process always panicking on exit on Windows.

Why: This bug isn't critical as it mainly occurs when the process is already exiting, but I think it would be better for the process to exit nominally instead of panicking so as not to confuse the user.

How: The problem is caused by resize event arriving when the main thread and, therefore, the receiver end of the channel is already dead. All I did was replacing unwrap with a conditional break. Even though the problem presents itself only with the resize event at the moment, I figured it would be better and more consistent to change all three cases to a conditional break. Another option would be to return the join handles main and join them in the end, but since the code inside both threads is just an infinite cycle, breaking seems like a more straightforward and cleaner choice.

Checklist:

jrnxf commented 2 years ago

thanks for bringing this to light! I wonder why I haven't run into this before -- could it be something specific to windows? Can you explain what you do to trigger the error? Is it on every exit, or just when you resize?

dmitr101 commented 2 years ago

It reproduces 100% of the time in Windows Terminal with PowerShell 7 and classic cmd by simply pressing Esc. I only tested on the Win10, though. I guess the issue is that Windows sends a resize event as a part of the process exit procedure, or maybe it's just a quirk of crossterm on Windows. I figure, though, that with some buffering behind the scenes or an incredibly unlucky timing, both keypress events and tick events can arrive when the main thread is already dead as well.

jrnxf commented 2 years ago

Not gonna cut a release quit yet because I have a few minor style changes / related fixes I want to max, but merging! Thanks for the add @dmitr101 !