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

From: Pádraig Brady <[email-masked]>
Date: Wed, 28 Sep 2016 14:27:42 +0100

On 27/09/16 21:16, Bernhard Voelker wrote:
> On 09/21/2016 08:15 PM, Pádraig Brady wrote:
>> 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.
> 
> The new behavior is nice, but it would really be better to have
> consistent behavior in inotify and polling mode.

Yes consistency would be nice. Looking at the polling behavior I see it was
not fundamental to the initial implementation and only changed later:
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=FILEUTILS-4_0q-68-g54d12f7

The attached patch how has the preferred behavior option 2
both with and without inotify.

thanks,
Pádraig
From d76beda19d6c181915bb5323a19d965b899b20dc 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: -F now always processes initially untailable files

This was not the case when inotify was not available.

* src/tail.c (any_live_files): Simplify, since the IGNORE
flag is now only set when a file should be ignored indefinitely.
(recheck): Only output the "giving up on name" message
when that's actually the case.  Only set the IGNORE flag
when ignoring a file indefinitely.
(tail_file): Likewise.
* tests/tail-2/retry.sh: Add a test case.  Also run
all existing test cases with and without inotify.
NEWS: Mention the fix.
Fixes http://bugs.gnu.org/24495
---
 NEWS                  |  5 +++++
 src/tail.c            | 41 ++++++++++++++++++++---------------------
 tests/tail-2/retry.sh | 34 +++++++++++++++++++++++++++++-----
 3 files changed, 54 insertions(+), 26 deletions(-)

diff --git a/NEWS b/NEWS
index beba774..c3554d0 100644
--- a/NEWS
+++ b/NEWS
@@ -42,6 +42,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   seq now immediately exits upon write errors.
   [This bug was present in "the beginning".]
 
+  tail -F now continues to process initially untailable files that are replaced
+  by a tailable file.  This was handled correctly when inotify was available,
+  and is now handled correctly in all cases.
+  [bug introduced in fileutils-4.0h]
+
   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..c2982f5 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,20 @@ 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)));
-      f->ignore = true;
+      f->tailable = false;
+      if (! (reopen_inaccessible_files && follow_mode == Follow_name))
+        f->ignore = true;
+      if (was_tailable || prev_errnum != f->errnum)
+        error (0, 0, _("%s has been replaced with an untailable file%s"),
+               quoteaf (pretty_name (f)),
+               f->ignore ? _("; giving up on this name") : "");
     }
   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,20 +1078,14 @@ any_live_files (const struct File_spec *f, size_t n_files)
 {
   size_t i;
 
-  if (reopen_inaccessible_files && follow_mode == Follow_name)
-    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 && reopen_inaccessible_files)
+            return true;
         }
     }
 
@@ -1930,12 +1927,14 @@ 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)));
               ok = false;
               f->errnum = -1;
-              f->ignore = true;
+              f->tailable = false;
+              f->ignore = ! (reopen_inaccessible_files
+                             && follow_mode == Follow_name);
+              error (0, 0, _("%s: cannot follow end of this type of file%s"),
+                     quotef (pretty_name (f)),
+                     f->ignore ? _("; giving up on this name") : "");
             }
 
           if (!ok)
diff --git a/tests/tail-2/retry.sh b/tests/tail-2/retry.sh
index 37e01ed..a4cff11 100755
--- a/tests/tail-2/retry.sh
+++ b/tests/tail-2/retry.sh
@@ -55,11 +55,14 @@ tail --retry missing > out 2>&1 && fail=1
 [ "$(countlines_)" = 2 ]                     || { cat out; fail=1; }
 grep -F 'tail: warning: --retry ignored' out || { cat out; fail=1; }
 
+for mode in '' '---disable-inotify'; do
+
 # === Test:
 # Ensure that "tail --retry --follow=name" waits for the file to appear.
 # Clear 'out' so that we can check its contents without races
 >out                            || framework_failure_
-timeout 10 tail $fastpoll --follow=name --retry missing >out 2>&1 & pid=$!
+timeout 10 \
+  tail $mode $fastpoll --follow=name --retry missing >out 2>&1 & pid=$!
 # Wait for "cannot open" error.
 retry_delay_ wait4lines_ .1 6 1 || { cat out; fail=1; }
 echo "X" > missing              || framework_failure_
@@ -76,7 +79,8 @@ rm -f missing out          || framework_failure_
 # === Test:
 # Ensure that "tail --retry --follow=descriptor" waits for the file to appear.
 # tail-8.21 failed at this (since the implementation of the inotify support).
-timeout 10 tail $fastpoll --follow=descriptor --retry missing >out 2>&1 & pid=$!
+timeout 10 \
+  tail $mode $fastpoll --follow=descriptor --retry missing >out 2>&1 & pid=$!
 # Wait for "cannot open" error.
 retry_delay_ wait4lines_ .1 6 2 || { cat out; fail=1; }
 echo "X" > missing              || framework_failure_
@@ -95,7 +99,8 @@ rm -f missing out          || framework_failure_
 # === Test:
 # Ensure that tail --follow=descriptor --retry exits when the file appears
 # untailable. Expect exit status 1.
-timeout 10 tail $fastpoll --follow=descriptor --retry missing >out 2>&1 & pid=$!
+timeout 10 \
+  tail $mode $fastpoll --follow=descriptor --retry missing >out 2>&1 & pid=$!
 # Wait for "cannot open" error.
 retry_delay_ wait4lines_ .1 6 2 || { cat out; fail=1; }
 mkdir missing                   || framework_failure_  # Create untailable
@@ -116,16 +121,35 @@ rm -fd missing out                             || framework_failure_
 # Ensure that --follow=descriptor (without --retry) does *not wait* for the
 # file to appear.  Expect 2 lines in the output file ("cannot open" +
 # "no files remaining") and exit status 1.
-tail --follow=descriptor missing >out 2>&1 && fail=1
+tail $mode --follow=descriptor missing >out 2>&1 && fail=1
 [ "$(countlines_)" = 2 ]         || { fail=1; cat out; }
 grep -F 'cannot open' out        || { fail=1; cat out; }
 grep -F 'no files remaining' out || { fail=1; cat out; }
 
 # === Test:
 # Likewise for --follow=name (without --retry).
-tail --follow=name missing >out 2>&1 && fail=1
+tail $mode --follow=name missing >out 2>&1 && fail=1
 [ "$(countlines_)" = 2 ]         || { fail=1; cat out; }
 grep -F 'cannot open' out        || { fail=1; cat out; }
 grep -F 'no files remaining' out || { fail=1; cat out; }
 
+# === Test:
+# Ensure that tail -F retries when the file is initally untailable.
+mkdir untailable
+timeout 10 \
+  tail $mode $fastpoll -F untailable >out 2>&1 & pid=$!
+# Wait for "cannot open" error.
+retry_delay_ wait4lines_ .1 6 2 || { cat out; fail=1; }
+{ rmdir untailable; echo foo > untailable; }   || framework_failure_
+# Wait for the expected output.
+retry_delay_ wait4lines_ .1 6 4 || { cat out; fail=1; }
+cleanup_
+[ "$(countlines_)" = 4 ]                       || { fail=1; cat out; }
+grep -F 'cannot follow' out                    || { fail=1; cat out; }
+grep -F 'has become accessible' out            || { fail=1; cat out; }
+grep -F 'foo' out                              || { fail=1; cat out; }
+rm -fd untailable out                          || framework_failure_
+
+done
+
 Exit $fail
-- 
2.5.5