bug#24232: ls --color cannot be interrupted by a signal
On 15/08/16 16:52, Kamil Dudka wrote: > On Monday, August 15, 2016 15:55:13 Pádraig Brady wrote: >> The signal catching functionality originated trying to restore terminal >> color: >> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v4.5.3-89-g854 >> 9068 >> >> That was adjusted to only outputting reset chars once for multiple signals, >> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v5.2.1-357-geae >> 1b7f >> >> and not outputting reset chars at arbitrary places as that messes up >> multi-byte chars: >> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v5.2.1-368-gad >> c30a8 >> >> Maybe we should just buffer internally at put_indicator >> (_indicator[C_LEFT]); and output only at put_indicator >> (_indicator[C_RIGHT]) or equivalent. That wouldn't introduce >> significant overhead I think. > > Internal buffering is certainly doable. I guess there is some gnulib module > that already implements such a buffer? Not that I noticed. The conditions for flushing tend to vary per app, so we've slightly different variations in various utils, like lbuf_flush() in factor(1), and buffer_or_output() in relpath(1). We could take advantage of a PATH_MAX+A_BIT sized buffer to avoid realloc complexities. Actually we already buffer in quote_name(), so rather than outputting the color sequences separately in print_name_with_quoting(), we could pass those into quote_name() and it could write to its buffer, and output the START+name+END atomically. > However, what to do with signal handling then? Drop the SA_RESTART flag and > implement EINTR loop only for the fwrite() call that writes data with escape > sequences to the terminal? Hrm, the buffering above would avoid needing to explicitly output the color reset sequence upon exit, but it would not avoid the need to fflush(), since stdio might actually output the start and end color sequences in separate writes. Drats, so if we had to manage signals anyway to do the fflush() the buffering probably isn't worth it. I'm not sure how to improve this. Perhaps we should look at only enabling the signal handling before printing, in the common case of outputting the sorted entries of a single directory. In that way the open(), stat() etc. would be done with standard signal handling.
bug#24232: ls --color cannot be interrupted by a signal
On Monday, August 15, 2016 15:55:13 Pádraig Brady wrote: > The signal catching functionality originated trying to restore terminal > color: > http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v4.5.3-89-g854 > 9068 > > That was adjusted to only outputting reset chars once for multiple signals, > http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v5.2.1-357-geae > 1b7f > > and not outputting reset chars at arbitrary places as that messes up > multi-byte chars: > http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v5.2.1-368-gad > c30a8 > > Maybe we should just buffer internally at put_indicator > (_indicator[C_LEFT]); and output only at put_indicator > (_indicator[C_RIGHT]) or equivalent. That wouldn't introduce > significant overhead I think. Internal buffering is certainly doable. I guess there is some gnulib module that already implements such a buffer? However, what to do with signal handling then? Drop the SA_RESTART flag and implement EINTR loop only for the fwrite() call that writes data with escape sequences to the terminal? Kamil > Pádraig
bug#24232: ls --color cannot be interrupted by a signal
On 15/08/16 14:55, Kamil Dudka wrote: > If 'ls --color' outputs to a terminal and a syscall blocks (e.g. while > reading > a directory from unresponsive network file system), it cannot be interrupted > by a signal. > > This seems to be caused by the code of ls, which sets the SA_RESTART flag on > terminating signals. A possible solution would be to reset the flag prior to > calling certain blocking syscalls and process the signals synchronously in an > EINTR loop. Sounds expensive > Alternatively, we can document this behavior as intended (or broken by > design) > and suggest users to disable color output or redirect the output off terminal > to work around the issue. > > Any other idea how to solve it? > > Originally reported at: https://bugzilla.redhat.com/1365933 The signal catching functionality originated trying to restore terminal color: http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v4.5.3-89-g8549068 That was adjusted to only outputting reset chars once for multiple signals, http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v5.2.1-357-geae1b7f and not outputting reset chars at arbitrary places as that messes up multi-byte chars: http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v5.2.1-368-gadc30a8 Maybe we should just buffer internally at put_indicator (_indicator[C_LEFT]); and output only at put_indicator (_indicator[C_RIGHT]) or equivalent. That wouldn't introduce significant overhead I think. Pádraig
bug#24232: ls --color cannot be interrupted by a signal
If 'ls --color' outputs to a terminal and a syscall blocks (e.g. while reading a directory from unresponsive network file system), it cannot be interrupted by a signal. This seems to be caused by the code of ls, which sets the SA_RESTART flag on terminating signals. A possible solution would be to reset the flag prior to calling certain blocking syscalls and process the signals synchronously in an EINTR loop. Alternatively, we can document this behavior as intended (or broken by design) and suggest users to disable color output or redirect the output off terminal to work around the issue. Any other idea how to solve it? Originally reported at: https://bugzilla.redhat.com/1365933 Kamil
