partial graphics writes to kitty lock it up

[dankamongmen]

It is easy to effectively lock up kitty by writing a partial graphic, i.e.:

echo -e '\e_Gf=32,q=1,s=789,v=1379,i=1,a=T,m=1;AA'

this is pretty much unrecoverable from the keyboard. We ought go to some effort to ensure that we never partially write a graphic. It would be unfortunate, for instance, for a SIGSEGV to hit us while spooling out graphics chunks. This is probably best addressed in a signal handler. It'll be messy either way.

[kovidgoyal]

It's trivially easy to lock up any terminal with partial escape codes. In kitty you can always recover by pressing the shortcut for reset terminal which is ctrl+shift+delete by default.

[dankamongmen]

Thanks for that pointer, @kovidgoyal . Nonetheless, I want to protect against this where I can, though you raise a good point -- it's probably best to abort any outstanding escape code, not just graphics. Is there any safe sequence I can emit that will abort any outstanding escape? If so, I've got an excellent place to slot it in unconditionally.

[kovidgoyal]

I dont think there is anything that does that today. However, the best
candidate would probably be NUL 0x0. Currently I think in kitty that
does not abort either APC or PM escape codes, but that could be changed.
The real difficulty is you would need cross terminal support for it.

Your best bet is to read ECMA 48 and see if that standard supports using
NUL for terminating all escape codes, if so rely on it and I will be
happy to add support for it to kitty.

[kovidgoyal]

kovidgoyal/kitty@6179cfc

[dankamongmen]

kovidgoyal/kitty@6179cfc

nice! thanks.

[dankamongmen]

There's an argument to be made, I think, for masking signals while writing out the frame. That won't work for SIGKILL or SIGSTOP, but nothing else will, either. The only problem I see here is that if we ever have a hard lock/loop in our writeout, we won't be able to be killed with regular signals. There's also the issue that we would need use pthread_sigmask() in our writing thread, and if there are other threads, they still presumably might see the signal and abort. But that would again be true for any signal handler we put in here, emitting an escape buster upon signal receipt. Hrmm.

Also, I've no idea how signals work on Windows.

[dankamongmen]

I can no longer lock up kitty in notcurses-demo's xray (graphics-intensive) demo with a ctrl+c, nor with a SIGINT/SIGTERM/SIGQUIT sent via kill(2) externally. We mask these upon entry to blocking_write(), and restore the old signal mask upon exit (at which point we will process any queued signal). We also no longer remove them from the sigset_t passed to notcurses_getc(), and instead add them there. This seems a good fix. The only worry is that multithreaded client programs will need to have these signals in all threads' masks, or else some other thread can handle the signal, and the signal handler will interfere negatively with the writing thread. This was always a potential problem, but it was unlikely (not impossible) to happen, since the writing thread would generally be active and unmasked. As it is now masked, other threads are more likely to see the signal. I think this is best resolved by moving the current cleanup code out of the signal handler (where it doesn't belong any damn way), setting a global flag instead, and checking that somehow. That's for another day.