Subject: bug#19760: closed (Re: bug#19760: [bug] "tail -f" with inotify fails to follow a file after a rename())

From: [email-masked] (GNU bug Tracking System)
Date: Tue, 31 Mar 2015 10:57:02 +0000

Your bug report

#19760: [bug] "tail -f" with inotify fails to follow a file after a rename()

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].

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

Subject: Re: bug#19760: [bug] "tail -f" with inotify fails to follow a file after a rename()
From: Pádraig Brady <[email-masked]>
To: Bernhard Voelker <[email-masked]>, Stephane Chazelas <[email-masked]>, [email-masked]
Date: Tue, 31 Mar 2015 11:56:40 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

On 31/03/15 07:30, Bernhard Voelker wrote:
> On 03/31/2015 05:15 AM, Pádraig Brady wrote:
>> +  tail -f continues to follow changes to a file even after it's renamed.
>> +  [bug introduced in coreutils-7.5]
>> +
> 
> It is not 100% clear to me by this sentence what was the actual change;
> maybe a little "again" or "now" would help?
> 
>> --- /dev/null
>> +++ b/tests/tail-2/f-vs-rename.sh
>> @@ -0,0 +1,51 @@
>> +#!/bin/sh
>> +# demonstrate that tail -f works when renaming the tailed files
> 
> s/^d/D/; s/$/./
> 
>> +# Before coreutils-8.24, tail -f a would stop tracking additions to b
>> +# after "mv a b".
>> +
>> +# Copyright (C) 2015 Free Software Foundation, Inc.
>> +
>> +# This program is free software: you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation, either version 3 of the License, or
>> +# (at your option) any later version.
>> +
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
>> +print_ver_ tail
>> +
>> +touch a || framework_failure_
>> +
>> +debug='---disable-inotify'
>> +debug=
>> +tail $debug -f -s.1 a > out 2>&1 & pid=$!
> 
> Shouldn't $debug be removed?  Otherwise maybe a loop over both the
> inotify and the non-inotify mode would make sense?
> 
>> +
>> +check_tail_output()
>> +{
>> +  local delay="$1"
>> +  grep "$tail_re" out > /dev/null ||
>> +    { sleep $delay; return 1; }
>> +}
> 
> Please don't discard grep's output: reading the test's log file is
> easier with this included.
> 
>> +
>> +# Wait up to 12.7s for tail to start
> 
> s/$/./
> 
>> +echo x > a
>> +tail_re='^x$' retry_delay_ check_tail_output .1 7 || fail=1
>> +
>> +mv a b || fail=1
>> +
>> +echo y >> b
>> +# Wait up to 12.7s for "y" to appear in the output:
>> +tail_re='^y$' retry_delay_ check_tail_output .1 7 || fail=1
>> +
>> +kill $pid
>> +
>> +wait
>> +
>> +Exit $fail
> 
> Otherwise +1 (including the changes in tail.c).

All good suggestions.
Latest attached.

thanks for the review!
Pádraig.

>From d313a0b24234d3366ec263111469f219f5b4634f Mon Sep 17 00:00:00 2001
From: Stephane Chazelas <[email-masked]>
Date: Tue, 3 Feb 2015 21:22:06 +0000
Subject: [PATCH] tail: fix -f to follow changes after a rename

* src/tail.c (tail_forever_inotify): Only monitor write()s and
truncate()s to files in --follow=descriptor mode, thus avoiding
the bug where we removed the watch on renamed files.
Also adjust the inotify event processing code that is
now significant only in --follow=name mode.
* tests/tail-2/F-vs-rename.sh: Improve this existing test by running
in both polling and inotify modes.
* tests/tail-2/f-vs-rename.sh: A new test based on the existing one.
* tests/local.mk: Reference the new test.
* NEWS: Mention the bug.
Fixes http://bugs.gnu.org/19760
---
 NEWS                        |  3 ++
 src/tail.c                  | 31 +++++++--------
 tests/local.mk              |  1 +
 tests/tail-2/F-vs-rename.sh | 94 +++++++++++++++++++++++----------------------
 tests/tail-2/f-vs-rename.sh | 51 ++++++++++++++++++++++++
 5 files changed, 117 insertions(+), 63 deletions(-)
 create mode 100755 tests/tail-2/f-vs-rename.sh

diff --git a/NEWS b/NEWS
index 81031c6..4b12e46 100644
--- a/NEWS
+++ b/NEWS
@@ -39,6 +39,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   resources with many files, or with -F if files were replaced many times.
   [bug introduced in coreutils-7.5]
 
+  tail -f again follows changes to a file after it's renamed.
+  [bug introduced in coreutils-7.5]
+
 ** New features
 
   chroot accepts the new --skip-chdir option to not change the working directory
diff --git a/src/tail.c b/src/tail.c
index c5380cb..f75d7a9 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -159,13 +159,6 @@ struct File_spec
   uintmax_t n_unchanged_stats;
 };
 
-#if HAVE_INOTIFY
-/* The events mask used with inotify on files.  This mask is not used on
-   directories.  */
-static const uint32_t inotify_wd_mask = (IN_MODIFY | IN_ATTRIB
-                                         | IN_DELETE_SELF | IN_MOVE_SELF);
-#endif
-
 /* Keep trying to open a file even if it is inaccessible when tail starts
    or if it becomes inaccessible later -- useful only with -f.  */
 static bool reopen_inaccessible_files;
@@ -1390,6 +1383,13 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
   if (! wd_to_name)
     xalloc_die ();
 
+  /* The events mask used with inotify on files (not directories).  */
+  uint32_t inotify_wd_mask = IN_MODIFY;
+  /* TODO: Perhaps monitor these events in Follow_descriptor mode also,
+     to tag reported file names with "deleted", "moved" etc.  */
+  if (follow_mode == Follow_name)
+    inotify_wd_mask |= (IN_ATTRIB | IN_DELETE_SELF | IN_MOVE_SELF);
+
   /* Add an inotify watch for each watched file.  If -F is specified then watch
      its parent directory too, in this way when they re-appear we can add them
      again to the watch list.  */
@@ -1641,20 +1641,17 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
 
       if (ev->mask & (IN_ATTRIB | IN_DELETE_SELF | IN_MOVE_SELF))
         {
-          /* For IN_DELETE_SELF, we always want to remove the watch.
-             However, for IN_MOVE_SELF (the file we're watching has
-             been clobbered via a rename), when tailing by NAME, we
-             must continue to watch the file.  It's only when following
-             by file descriptor that we must remove the watch.  */
-          if ((ev->mask & IN_DELETE_SELF)
-              || ((ev->mask & IN_MOVE_SELF)
-                  && follow_mode == Follow_descriptor))
+          /* Note for IN_MOVE_SELF (the file we're watching has
+             been clobbered via a rename) we leave the watch
+             in place since it may still be part of the set
+             of watched names.  */
+          if (ev->mask & IN_DELETE_SELF)
             {
               inotify_rm_watch (wd, fspec->wd);
               hash_delete (wd_to_name, fspec);
             }
-          if (follow_mode == Follow_name)
-            recheck (fspec, false);
+
+          recheck (fspec, false);
 
           continue;
         }
diff --git a/tests/local.mk b/tests/local.mk
index 56cba69..1be31ad 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -171,6 +171,7 @@ all_tests =					\
   tests/tail-2/inotify-hash-abuse2.sh		\
   tests/tail-2/F-vs-missing.sh			\
   tests/tail-2/F-vs-rename.sh			\
+  tests/tail-2/f-vs-rename.sh			\
   tests/tail-2/inotify-rotate.sh		\
   tests/tail-2/inotify-rotate-resources.sh	\
   tests/chmod/no-x.sh				\
diff --git a/tests/tail-2/F-vs-rename.sh b/tests/tail-2/F-vs-rename.sh
index cb6a3ed..f95b71a 100755
--- a/tests/tail-2/F-vs-rename.sh
+++ b/tests/tail-2/F-vs-rename.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
-# demonstrate that tail -F works when renaming the tailed files
-# Before coreutils-8.3, tail -F a b would stop tracking additions to b
-# after "mv a b".
+# Demonstrate that tail -F works when renaming the tailed files.
+# Between coreutils 7.5 and 8.2 inclusive, 'tail -F a b' would
+# stop tracking additions to b after 'mv a b'.
 
 # Copyright (C) 2009-2015 Free Software Foundation, Inc.
 
@@ -21,55 +21,57 @@
 . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
 print_ver_ tail
 
-touch a b || framework_failure_
-
-debug='---disable-inotify'
-debug=
-tail $debug -F -s.1 a b > out 2>&1 & pid=$!
-
 check_tail_output()
 {
   local delay="$1"
-  grep "$tail_re" out > /dev/null ||
+  grep "$tail_re" out ||
     { sleep $delay; return 1; }
 }
 
-# Wait up to 12.7s for tail to start
-echo x > a
-tail_re='^x$' retry_delay_ check_tail_output .1 7 || fail=1
-
-mv a b || fail=1
-
-# Wait 12.7s for this diagnostic:
-# tail: 'a' has become inaccessible: No such file or directory
-tail_re='inaccessible' retry_delay_ check_tail_output .1 7 || fail=1
-
-echo x > a
-# Wait up to 12.7s for this to appear in the output:
-# "tail: '...' has appeared;  following end of new file"
-tail_re='has appeared' retry_delay_ check_tail_output .1 7 ||
-  { echo "$0: a: unexpected delay?"; cat out; fail=1; }
-
-echo y >> b
-# Wait up to 12.7s for "y" to appear in the output:
-tail_f_vs_rename_2() {
-  local delay="$1"
-  tr '\n' @ < out | grep '@@==> b <==@y@$' > /dev/null ||
-    { sleep $delay; return 1; }
-}
-retry_delay_ tail_f_vs_rename_2 .1 7 ||
-  { echo "$0: b: unexpected delay?"; cat out; fail=1; }
-
-echo z >> a
-# Wait up to 12.7s for "z" to appear in the output:
-tail_f_vs_rename_3() {
-  local delay="$1"
-  tr '\n' @ < out | grep '@@==> a <==@z@$' > /dev/null ||
-    { sleep $delay; return 1; }
-}
-retry_delay_ tail_f_vs_rename_3 .1 7 ||
-  { echo "$0: a: unexpected delay?"; cat out; fail=1; }
+touch a b || framework_failure_
 
-kill -HUP $pid
+for mode in '' '---disable-inotify'; do
+  tail $mode -F -s.1 a b > out 2>&1 & pid=$!
+
+  # Wait up to 12.7s for tail to start.
+  echo x > a
+  tail_re='^x$' retry_delay_ check_tail_output .1 7 || fail=1
+
+  mv a b || fail=1
+
+  # Wait 12.7s for this diagnostic:
+  # tail: 'a' has become inaccessible: No such file or directory
+  tail_re='inaccessible' retry_delay_ check_tail_output .1 7 || fail=1
+
+  echo x > a
+  # Wait up to 12.7s for this to appear in the output:
+  # "tail: '...' has appeared;  following end of new file"
+  tail_re='has appeared' retry_delay_ check_tail_output .1 7 ||
+    { echo "$0: a: unexpected delay?"; cat out; fail=1; }
+
+  echo y >> b
+  # Wait up to 12.7s for "y" to appear in the output:
+  tail_f_vs_rename_2() {
+    local delay="$1"
+    tr '\n' @ < out | grep '@@==> b <==@y@$' > /dev/null ||
+      { sleep $delay; return 1; }
+  }
+  retry_delay_ tail_f_vs_rename_2 .1 7 ||
+    { echo "$0: b: unexpected delay?"; cat out; fail=1; }
+
+  echo z >> a
+  # Wait up to 12.7s for "z" to appear in the output:
+  tail_f_vs_rename_3() {
+    local delay="$1"
+    tr '\n' @ < out | grep '@@==> a <==@z@$' > /dev/null ||
+      { sleep $delay; return 1; }
+  }
+  retry_delay_ tail_f_vs_rename_3 .1 7 ||
+    { echo "$0: a: unexpected delay?"; cat out; fail=1; }
+
+  kill $pid
+
+  wait $pid
+done
 
 Exit $fail
diff --git a/tests/tail-2/f-vs-rename.sh b/tests/tail-2/f-vs-rename.sh
new file mode 100755
index 0000000..4bff41c
--- /dev/null
+++ b/tests/tail-2/f-vs-rename.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+# Demonstrate that tail -f works when renaming the tailed files.
+# Between coreutils 7.5 and 8.23 inclusive, 'tail -f a' would
+# stop tracking additions to b after 'mv a b'.
+
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ tail
+
+check_tail_output()
+{
+  local delay="$1"
+  grep "$tail_re" out ||
+    { sleep $delay; return 1; }
+}
+
+touch a || framework_failure_
+
+for mode in '' '---disable-inotify'; do
+  tail $mode -f -s.1 a > out 2>&1 & pid=$!
+
+  # Wait up to 12.7s for tail to start.
+  echo x > a
+  tail_re='^x$' retry_delay_ check_tail_output .1 7 || fail=1
+
+  mv a b || fail=1
+
+  echo y >> b
+  # Wait up to 12.7s for "y" to appear in the output:
+  tail_re='^y$' retry_delay_ check_tail_output .1 7 || fail=1
+
+  kill $pid
+
+  wait $pid
+done
+
+Exit $fail
-- 
2.1.0

Subject: [bug] "tail -f" with inotify fails to follow a file after a rename()
From: Stephane Chazelas <[email-masked]>
To: [email-masked]
Date: Tue, 3 Feb 2015 22:04:11 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello,

On Linux, when inotify is used,

   tail -f file

follows a file only until it's renamed. After it is renamed, the
inotify watch is removed, which means tail sits there doing
nothing and any further modifications to the file are ignored.

To reproduce:

    echo 1 > file
    tail -f file &
    exec 3>> file
    echo 2 >&3
    sleep 1
    mv file file2
    sleep 1
    echo 3 >&3
    sleep 1
    : > file2

"3" is not displayed. No message about the file being truncated
either.

Work arounds:

   tail ---disable-inotify -f file
   tail -f < file # effectively disables inotify

   or rename the file with a link() followed by an unlink()
   ln file newfile && rm -f file

Note that the IN_DELETED_SELF event is not reached in
follow-descriptor mode because tail has the file open preventing
it from being deleted even after it's unlinked from the last
directory.

Path attached (on the current git head).

I don't think IN_DELETED_SELF is useful in follow-name mode
either, but I've not removed it.

-- 
Stephane
>From f97355fa31f5e13f95667a50908506388d62df64 Mon Sep 17 00:00:00 2001
From: Stephane Chazelas <[email-masked]>
Date: Tue, 3 Feb 2015 21:22:06 +0000
Subject: [PATCH] tail: fix rename in follow-descriptor mode

In follow-descriptor mode, even when using inotify, we don't care if the
file has been renamed or deleted.  We still want to watch it.
---
 src/tail.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/tail.c b/src/tail.c
index b512c2a..864c71f 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -159,13 +159,6 @@ struct File_spec
   uintmax_t n_unchanged_stats;
 };
 
-#if HAVE_INOTIFY
-/* The events mask used with inotify on files.  This mask is not used on
-   directories.  */
-static const uint32_t inotify_wd_mask = (IN_MODIFY | IN_ATTRIB
-                                         | IN_DELETE_SELF | IN_MOVE_SELF);
-#endif
-
 /* Keep trying to open a file even if it is inaccessible when tail starts
    or if it becomes inaccessible later -- useful only with -f.  */
 static bool reopen_inaccessible_files;
@@ -1385,11 +1378,22 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
   char *evbuf;
   size_t evbuf_off = 0;
   size_t len = 0;
+  uint32_t inotify_wd_mask;
 
   wd_to_name = hash_initialize (n_files, NULL, wd_hasher, wd_comparator, NULL);
   if (! wd_to_name)
     xalloc_die ();
 
+  /* The events mask used with inotify on files.  This mask is not used on
+     directories.  */
+  if (follow_mode == Follow_name)
+    inotify_wd_mask = (IN_MODIFY | IN_ATTRIB | IN_DELETE_SELF | IN_MOVE_SELF);
+  else
+    /* if following by descriptor, we're watching the same file even if
+       unlinked or renamed.  */
+    inotify_wd_mask = IN_MODIFY;
+
+
   /* Add an inotify watch for each watched file.  If -F is specified then watch
      its parent directory too, in this way when they re-appear we can add them
      again to the watch list.  */
-- 
1.9.1