Subject: bug#10243: closed (Re: bug#10243: 8.14: ls --color is uninterruptible with ctrl+c (and no network fs in use))

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 08 Dec 2011 10:07:02 +0000

Your bug report

#10243: 8.14: ls --color is uninterruptible with ctrl+c (and no network fs in use)

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
10243: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10243
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#10243: 8.14: ls --color is uninterruptible with ctrl+c (and no network fs in use)
From: Jim Meyering <[email-masked]>
To: Eric Blake <[email-masked]>
Cc: Pádraig Brady <[email-masked]>, Arkadiusz Miśkiewicz <[email-masked]>, [email-masked]
Date: Thu, 08 Dec 2011 11:05:52 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Eric Blake wrote:
> On 12/07/2011 03:04 PM, Jim Meyering wrote:
...
>> Thanks for the feedback.
>> Here's a more complete patch, but I haven't re-reviewed it yet,
>> so caveat emptor.
>
> Question - is ls processing entirely two-phase (collect all names, then
> format them), or is this a repetitive loop?  Seeing as how long listings
> of two directories on the command line produces differing column widths
> between the two directories, I have to think it is the latter (besides,
> that makes better sense from a memory management perspective - it's
> cheaper to sort one directory at a time than to store the sorting of all
> directories at once).  In which case, consider:
>
> ls --color=always largedir1 largedir2

Thanks, you're right.
This is my first use of the new Co-authored-by: syntax,
so I confirmed that our updated gitlog-to-changelog does produce
the expected ChangeLog in the distribution tarball.

Here's a simpler patch.  I've tested it by running

    ls -R --color big something-distinctive big2

where big has 500K entries and big2 has 2M (on a tmpfs).
Before (using my initial patch), I could interrupt any time
before the ls' readdir loop for big2, but not during that loop,
which would run for a few seconds after ls printed its "/t/big2:"
Now, it's interruptible, as one would expect.

>From aaf5b61e9999d0ece522faa4508a6565d51a8446 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Thu, 8 Dec 2011 10:49:03 +0100
Subject: [PATCH] ls: be responsive to interrupts when color-listing large
 directories
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Starting with commit adc30a83, when using --color, ls inhibited
interrupts to avoid corrupting the state of an output terminal.
However, for very large directories, that inhibition rendered ls
uninterruptible for too long, including a potentially long period
even before any output is generated.
* src/ls.c: There are two phases of processing that are time-
consuming enough that they can cause this sort of trouble:
the readdir loop and the printing loop.  The printing side of things
was nominally covered by a call to process_signals in
(print_name_with_quoting): ... but that call was mistakenly guarded
by a condition that might be false for many or even all files being
processed.  Call process_signals unconditionally.
(print_dir): Also call process_signals in the readdir loop.
* NEWS (Bug fixes): Mention it.
Reported by Arkadiusz Miśkiewicz in http://bugs.gnu.org/10243
Co-authored-by: Eric Blake <[email-masked]>
---
 NEWS      |    3 +++
 THANKS.in |    1 +
 src/ls.c  |    7 ++++++-
 3 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/NEWS b/NEWS
index de3888d..0d4c83b 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU coreutils NEWS                                    -*- outline -*-

 ** Bug fixes

+  ls --color many-entry-directory was uninterruptible for too long
+  [bug introduced in coreutils-5.2.1]
+
   ls's -k option no longer affects how ls -l outputs file sizes.
   It now affects only the per-directory block counts written by -l,
   and the sizes written by -s.  This is for compatibility with BSD
diff --git a/THANKS.in b/THANKS.in
index 5ecc29e..afed5d4 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -59,6 +59,7 @@ Anthony Thyssen                     [email-masked]
 Antonio Rendas                      [email-masked]
 Ariel Faigon                        [email-masked]
 Arjan Opmeer                        [email-masked]
+Arkadiusz Miśkiewicz                [email-masked]
 Arne Henrik Juul                    [email-masked]
 Arnold Robbins                      [email-masked]
 Arthur Pool                         [email-masked]
diff --git a/src/ls.c b/src/ls.c
index 8be9b6a..0d64bab 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -2595,6 +2595,11 @@ print_dir (char const *name, char const *realname, bool command_line_arg)
         }
       else
         break;
+
+      /* When processing a very large directory, and since we've inhibited
+         interrupts, this loop would take so long that ls would be annoyingly
+         uninterruptible.  This ensures that it handles signals promptly.  */
+      process_signals ();
     }

   if (closedir (dirp) != 0)
@@ -4060,9 +4065,9 @@ print_name_with_quoting (const struct fileinfo *f,
   if (stack)
     PUSH_CURRENT_DIRED_POS (stack);

+  process_signals ();
   if (used_color_this_time)
     {
-      process_signals ();
       prep_non_filename_text ();
       if (start_col / line_length != (start_col + width - 1) / line_length)
         put_indicator (&color_indicator[C_CLR_TO_EOL]);
--
1.7.8.110.g4cb5d1

Subject: 8.14: ls --color is uninterruptible with ctrl+c (and no network fs in use)
From: Arkadiusz Miśkiewicz <[email-masked]>
To: [email-masked]
Date: Wed, 7 Dec 2011 11:07:30 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello,

When doing "ls --color=tty" or "ls --color=auto" on directory then ls ignores 
(?) ctrl+c or ctrl+z signals. Basically I'm unable to interrupt ls in such 
case. Easily reproducible with bigger directories.

# ls --color=tty
^C^C^C^C^C^C^C^C^Z^Z^Z^Z^Z^Z^Z^Z^Z^Z^Z^Z^Z^Z^Z
(no reaction)

There is no such problem if ls is being straced (strace -f -F -s 200 ls) at 
that time or if pipe is used ("ls --color=tty | less") - in such cases ctrl+c 
works immediately. If --color is not used then there is no problem, too.

First I thought that this is filesystem issue but it is not. One of xfs 
filesystem developers was also able to reproduce the problem and confirm that 
most likely ls is doing something incorrect.

# LC_ALL=C ls --version
ls (GNU coreutils) 8.14
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Richard M. Stallman and David MacKenzie.

Linux 3.0.9, glibc 2.14 here.
-- 
Arkadiusz Miśkiewicz        PLD/Linux Team
arekm / maven.pl            http://ftp.pld-linux.org/