Subject: bug#24495: tail -F does not terminate when running out of names to watch

From: Pádraig Brady <[email-masked]>
Date: Wed, 21 Sep 2016 19:15:59 +0100

On 21/09/16 15:22, Julian Büning wrote:
> observed behavior:
> 
> $ mkdir foo
> $ tail -F foo &
> [1] 1000
> tail: error reading 'foo': Is a directory
> tail: foo: cannot follow end of this type of file; giving up on this name
> $ rmdir foo ; echo moo > foo
> $ jobs
> [1]+  Running                 tail -F foo &
> 
> expected behavior option 1:
> 
> $ mkdir foo
> $ tail -F foo &
> tail: error reading 'foo': Is a directory
> tail: foo: cannot follow end of this type of file; giving up on this name
> $ rmdir foo ; echo moo > foo
> [1]+  Done                 tail -F foo &
> 
> expected behavior option 2:
> 
> $ mkdir foo
> $ tail -F foo &
> [1] 1000
> tail: error reading 'foo': Is a directory
> tail: foo: cannot follow end of this type of file
> $ rmdir foo ; echo moo > foo
> tail: 'foo' has appeared;  following new file
> moo
> $ jobs
> [1]+  Running                 tail -F foo &
> 
> tail does neither terminate nor display any contents of the file foo.
> We would assume that either tail terminates after displaying the error
> message (because there is no other file left) or that tail would
> infinitely retry to read a file with the same name and if such a file
> is created.
> 
> We could reproduce this behavior with versions 8.25 on ArchLinux and
> 8.25 and 8.25.71-1db94 on Fedora (package and compiled from source)
> with and without ---disable-inotify. At least in version 8.25,
> tail_forever_inotify is not executed because any_non_remote_file
> returns false, which disables inotify in line 2361 (tail.c).
> 
> We can trace the cause of this behavior for the non-inotify version of
> tail_forever:
> For a directory, the ignore flag in the corresponding struct File_spec
> is set to true. Thus, tail_forever skips this file in line 1130 (tail.c)
> and does not modify this flag during any iteration of the enclosing
> while(1). Since no check is being done if any valid target remains,
> this leads to the observed non-terminating behavior.
> 
> This behavior was found using Symbolic Execution techniques developed in
> the course of the SYMBIOSYS research project at COMSYS, RWTH Aachen
> University.

We can get expected behavior option 1 with the attached patch.
Note that's inconsistent with current inotify behavior which does
_not_ actually give up on the name, as can be seen when starting
with a (non existent) file:

$ touch foo
$ tail -F foo&
[1] 13624
$ rm foo; mkdir foo
tail: ‘foo’ has become inaccessible: No such file or directory
tail: ‘foo’ has been replaced with an untailable file; giving up on this name
$ rmdir foo; echo foo > foo
tail: ‘foo’ has become inaccessible: No such file or directory
tail: ‘foo’ has appeared;  following new file
foo

The attached patch also removes the "; giving up on this name"
message in the inotify case as that's not the case.

Ideally we'd have expected behavior option 2
both with and without inotify.
I'll need to look a bit more as to why we have that
limitation without inotify.

I'll add a test case before applying anything.

thanks,
Pádraig
From e547e9d2122c35c8a48fce3504ee96eac423f7e5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Wed, 21 Sep 2016 18:42:40 +0100
Subject: [PATCH] tail: with -F exit if all files are ignored

Note ignoring of files is only effective
when inotify is not used, so we only exit in that case.

* src/tail.c (any_live_files): Change to return false when,
all files are ignored, and inotify is not used.
Note ignoring of files is only effective
when inotify is not used.
(recheck): Don't output the "giving up on name" message
when inotify is used, as that limitation is only
applicable when inotify isn't used.
NEWS: Mention the fix.
Fixes http://bugs.gnu.org/24495
---
 NEWS       |  3 +++
 src/tail.c | 38 +++++++++++++++++++++-----------------
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/NEWS b/NEWS
index beba774..44a2489 100644
--- a/NEWS
+++ b/NEWS
@@ -42,6 +42,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   seq now immediately exits upon write errors.
   [This bug was present in "the beginning".]
 
+  tail now exits when there are no more files to process and notify is not used.
+  [bug introduced in coreutils-8.22]
+
   yes now handles short writes, rather than assuming all writes complete.
   [bug introduced in coreutils-8.24]
 
diff --git a/src/tail.c b/src/tail.c
index caa5407..f0a23d8 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -956,8 +956,8 @@ recheck (struct File_spec *f, bool blocking)
       f->errnum = -1;
       f->ignore = true;
 
-      error (0, 0, _("%s has been replaced with a symbolic link. "
-                     "giving up on this name"), quoteaf (pretty_name (f)));
+      error (0, 0, _("%s has been replaced with a symbolic link"),
+             quoteaf (pretty_name (f)));
     }
   else if (fd == -1 || fstat (fd, &new_stats) < 0)
     {
@@ -986,17 +986,17 @@ recheck (struct File_spec *f, bool blocking)
     {
       ok = false;
       f->errnum = -1;
-      error (0, 0, _("%s has been replaced with an untailable file;\
- giving up on this name"),
-             quoteaf (pretty_name (f)));
+      error (0, 0, _("%s has been replaced with an untailable file%s"),
+             quoteaf (pretty_name (f)),
+             disable_inotify ? _("; giving up on this name") : "");
       f->ignore = true;
     }
   else if (!disable_inotify && fremote (fd, pretty_name (f)))
     {
       ok = false;
       f->errnum = -1;
-      error (0, 0, _("%s has been replaced with a remote file. "
-                     "giving up on this name"), quoteaf (pretty_name (f)));
+      error (0, 0, _("%s has been replaced with a remote file"),
+             quoteaf (pretty_name (f)));
       f->ignore = true;
       f->remote = true;
     }
@@ -1075,23 +1075,27 @@ any_live_files (const struct File_spec *f, size_t n_files)
 {
   size_t i;
 
-  if (reopen_inaccessible_files && follow_mode == Follow_name)
+  bool all_ignored = true;
+
+  /* We retry even ignored files if inotify is available.  */
+  if (reopen_inaccessible_files && follow_mode == Follow_name
+      && ! disable_inotify)
     return true;  /* continue following for -F option */
 
   for (i = 0; i < n_files; i++)
     {
       if (0 <= f[i].fd)
-        {
-          return true;
-        }
+        return true;
       else
         {
-          if (reopen_inaccessible_files && follow_mode == Follow_descriptor)
-            if (! f[i].ignore)
-              return true;
+          if (! f[i].ignore)
+            all_ignored = false;
         }
     }
 
+  if (reopen_inaccessible_files && ! all_ignored)
+    return true;  /* continue following for --retry option */
+
   return false;
 }
 
@@ -1930,9 +1934,9 @@ tail_file (struct File_spec *f, uintmax_t n_units)
             }
           else if (!IS_TAILABLE_FILE_TYPE (stats.st_mode))
             {
-              error (0, 0, _("%s: cannot follow end of this type of file;\
- giving up on this name"),
-                     quotef (pretty_name (f)));
+              error (0, 0, _("%s: cannot follow end of this type of file%s"),
+                     quotef (pretty_name (f)),
+                     disable_inotify ? _("; giving up on this name") : "");
               ok = false;
               f->errnum = -1;
               f->ignore = true;
-- 
2.5.5