tenox7 / ttyplot

a realtime plotting utility for terminal/console with data input from stdin
Apache License 2.0
957 stars 42 forks source link

Stop using broken-on-Linux `pselect` :cry: (alternative to #132) #130

Closed hartwork closed 7 months ago

hartwork commented 7 months ago

With both aggressive stdin input and signals coming in, pselect sometimes delays signal delivery for multiple seconds, to the point where even holding Ctrl+C down for seconds will not be successful in terminating the process. I have witnessed this symptom for a duration of 20+ seconds in practice.

To reproduce:

# ./ttyplot < /dev/zero
[hold Ctrl+C or press repeatedly]
[see it take multiple seconds for the process to terminate (if at all)]

Related: https://stackoverflow.com/questions/62315082/pselect-on-linux-does-not-deliver-signals-if-events-are-pending

With regard to alternative approaches:

What do you think?

edgar-bonet commented 7 months ago

I wasn't aware of that. Thanks for pointing it out!

First I would like to note that, while this is a genuine defect in ttyplot and deserves fixing, it is not something I would consider urgent. We experience this kind of behavior every day and, we are so used to it, that we don't event get annoyed. Example: type man pselect, then try to control-C your way of of there. It is not annoying because we can simply hit q to get out of the pager... or out of ttyplot.

Documenting the q key binding would certainly be a more urgent fix.

Since this is touching the parts that are rewritten in #115, let's get that merged first, then come back to this issue.

Regarding the possible solutions, calling pselect twice seems prone to race conditions. Shortening the timeout makes the program do useless work in the most common (non pathological) case. signalfd sounds like a good approach: it is Linux-specific but, if I understood correctly, so is the pselect issue.

hartwork commented 7 months ago

I wasn't aware of that. Thanks for pointing it out!

@edgar-bonet me neither, this came as a surprise to me also.

We experience this kind of behavior every day and, we are so used to it, that we don't event get annoyed. Example: type man pselect, then try to control-C your way of of there. It is not annoying because we can simply hit q to get out of the pager... or out of ttyplot.

That's an interesting point. While we are used to quitting man with q — we likely never learned otherwise? — with ttyplot however all existing users learned to quit ttyplot through Ctrl+C and that only, so that changes the picture not just a bit. I'm still shocked that man is broken that way, I would not say I got used to it being broken, I got used to quitting it through q.

Documenting the q key binding would certainly be a more urgent fix.

I don't agree about urgent but a pull request on that would be great. We could add a line to -h output at the bottom also to promote this feature more. The current rate toggle r seems to have unexpected issues though breaking the scale, we may need to fix that, haven't gotten to debugging it yet or send a proper report.

Regarding the possible solutions, calling pselect twice seems prone to race conditions. Shortening the timeout makes the program do useless work in the most common (non pathological) case. signalfd sounds like a good approach: it is Linux-specific but, if I understood correctly, so is the pselect issue.

I addresses those in #132 now. I will close #130 here for now because after sleeping over it I consider two calls to pselect (and no changes in timout) the best way to go now, and getting back to plain select with all the block-do-unblock wrapping elsewhere seems to be worse, it just happened to be what I thought was needed before I came to calling pselect twice. So closing #130 for now.