eylles / pywal16

16 colors fork of pywal
MIT License
157 stars 24 forks source link

Handling ttys not accepting sequences e.g when suspended by Flow Control #22

Closed luisbocanegra closed 4 months ago

luisbocanegra commented 1 year ago

Currently whenever a terminal is in suspended mode (and maybe for some other reasons) and pywal tries to send the color sequences to it the save_file here:

https://github.com/eylles/pywal16/blob/095b169fa8ddbcc0466ee44bbf93e5556194b13f/pywal/sequences.py#L92

it will get stuck trying to write indefinitelly, blocking the writing to the remaining ttys and program itself

I tried this PR Add --active-only flag to only run pywal on active ttys from dylanaraps repo. But unfortunately it didn't work for Kitty, which doesn't show up in who output for some reason and also doesn't handle the aforementioned suspend mode.

So I am wondering if there is a way to reliably detect whether a tty cannot receive input so it is skipped. Konsole links to this wiki page about Software Flow Control, could it be that there is a way to detect in which state a tty is? Or will it be okay putting a timeout to the write operation to abort when it gets stuck?

eylles commented 1 year ago

huh interesting, after checking some stuff it seems the issue may be on how the who(1) program determines the active terminals, a sure fire way to check if a terminal or any process for that matter is active is with ps -o state= -p <pid> which will return the state code with a single char (on the manpage for ps just check the 'state codes' section) however neither who nor checking the /dev/pts/ dir like pywal does can yield the pid of the terminals to feed ps.

thankfully ps has the -t option to specify a ttylist, then a command constructed like this ps -o uid,pid,state,tty -t <ttylist> where the ttylist is the list of pseudo terminals separated by a comma (for example pts/1,pts/2,pts/3,pts/4,pts/5) will produce output as follows:

  UID   PID S TT
 1000  3651 S pts/1
 1000  4195 S pts/1
 1000  4321 S pts/2
 1000  4518 S pts/3
 1000  4751 S pts/3
 1000  4753 S pts/4
 1000 32551 S pts/5

the only important fields there are the tty and the status columns, so i would remove the uid and pid params from the ps command and we would get:

S TT
S pts/1
S pts/1
S pts/2
S pts/3
S pts/3
S pts/4
S pts/5

with that we would be able to parse the output, if the 'S' column is a T we skip sending sequences to that terminal, however i'm not so sure what would be the best way to deal with duplicate entries and then there are the issues that this most likely won't work for mac nor openbsd which use a different scheme for their pseudo terminals.

i will try to have a branch ready to test this week

luisbocanegra commented 1 year ago

Hmm, the state is not changing for me when I enable suspend mode, it returns exactly the same and man page mentions job control and not flow control, very similar name it made me think it was the same thing at first.

So it sounds for me that the later requires a completely different check if it even exists?

Also, interestingly enough, job control to the shell or the parent (Konsole here) doesn't stop tty receiving input:

image

I've had it happen at random times but never tried to debug it and I don't make use of flow control myself, which is the only way i can manually reproduce this. So is either some software I use suspending a terminal (probably vscode) or a completely different issue perhaps?

For now I will wait for the issue to happen again and try to debug it.

Almamu commented 4 months ago

There seemed to be some reports of this also happening in the old pywal repo. https://github.com/dylanaraps/pywal/issues/40#issuecomment-314990634

I did come across this issue myself with WebStorm and I know I worked around it somehow, but I don't remember how. I updated to sway from i3 so I come across this daily again, I might be able to debug it or try things for y'all.

luisbocanegra commented 4 months ago

Hi, I think I found a way to handle this, by using fcntl to set os.O_NONBLOCK flag before trying to write to the file. This way it fails immediately and we can simply log the error?

image

Almamu commented 4 months ago

That sounds like a reasonable approach, maybe @eylles can weight in on whether this is acceptable or not?

eylles commented 4 months ago

checking the pr

eylles commented 4 months ago

it works so far so closing this for now.