Re: tail aborts while following by name if using inotify

[Jim Meyering]

Giuseppe Scrivano wrote:
>> However, while looking at it, I discovered another problem.
>> When tail-F'd files may be renamed, tail may fail to track
>> the target of a rename.
>
> good catch and very useful test case!  I am going to like the test
> driven development we we had today :-)
>
> This patch should fix the problem, other tests remain green.

Looks good.  Thanks!

I've adjusted the log and pushed that, followed by the new
test and a NEWS update:

>From 2ead2365e01f8e103ddce6f34558eee3648de5b4 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <address@hidden>
Date: Wed, 30 Dec 2009 00:20:24 +0100
Subject: [PATCH 1/3] tail -F: don't stop following the target of a rename

This fixes a bug whereby tail -F would fail to track changes
to a file that was a target of a rename, and when the source of
the rename was another tailed file.

* src/tail.c (tail_forever_inotify): Ensure the wd is not already
present in the hash table before trying to add it.  When a new watch
descriptor is added to the `wd_to_name' hash table, check that it is
not already present.  If it is present then remove the previous element.
---
 src/tail.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/src/tail.c b/src/tail.c
index 3d5e221..28a0e26 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1486,11 +1486,24 @@ tail_forever_inotify (int wd, struct File_spec *f, 
size_t n_files,
           /* Remove `fspec' and re-add it using `new_fd' as its key.  */
           hash_delete (wd_to_name, fspec);
           fspec->wd = new_wd;
+
+          /* If the file was moved then inotify will use the source file wd for
+             the destination file.  Make sure the key is not present in the
+             table.  */
+          struct File_spec *prev = hash_delete (wd_to_name, fspec);
+          if (prev && prev != fspec)
+            {
+              if (follow_mode == Follow_name)
+                recheck (prev, false);
+              prev->wd = -1;
+              close_fd (prev->fd, pretty_name (prev));
+            }
+
           if (hash_insert (wd_to_name, fspec) == NULL)
             xalloc_die ();

           if (follow_mode == Follow_name)
-            recheck (&(f[j]), false);
+            recheck (fspec, false);
         }
       else
         {
--
1.6.6.rc3.271.g3d40f


>From fe062d563a09481162f5c11b154d047331708b7f Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 29 Dec 2009 16:37:04 +0100
Subject: [PATCH 2/3] tail: test for a bug in inotify-enabled tail -F

tail -F a b would stop tracking additions to b after "mv a b".
* tests/tail-2/F-vs-rename: New file.
* tests/Makefile.am (TESTS): Add it.
---
 tests/Makefile.am        |    1 +
 tests/tail-2/F-vs-rename |   78 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 0 deletions(-)
 create mode 100755 tests/tail-2/F-vs-rename

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6ef7ad8..33b9df4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -82,6 +82,7 @@ TESTS =                                               \
   cp/link-heap                                 \
   tail-2/inotify-hash-abuse                    \
   tail-2/inotify-hash-abuse2                   \
+  tail-2/F-vs-rename                           \
   tail-2/inotify-rotate                                \
   chmod/no-x                                   \
   chgrp/basic                                  \
diff --git a/tests/tail-2/F-vs-rename b/tests/tail-2/F-vs-rename
new file mode 100755
index 0000000..75d3c69
--- /dev/null
+++ b/tests/tail-2/F-vs-rename
@@ -0,0 +1,78 @@
+#!/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".
+
+# Copyright (C) 2009 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/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  tail --version
+fi
+
+. $srcdir/test-lib.sh
+
+touch a b || framework_failure
+
+debug='---disable-inotify -s .01'
+debug=
+tail $debug -F a b > out 2>&1 & pid=$!
+
+# Wait until tail has started...
+echo x > a
+until grep '^x$' out >/dev/null 2>&1; do :; done
+
+mv a b || fail=1
+
+# Wait for the diagnostic:
+# tail: `a' has become inaccessible: No such file or directory
+until grep inaccessible out >/dev/null 2>&1; do :; done
+
+echo x > a
+# Wait up to 4s for this to appear in the output:
+# "tail: `...' has appeared;  following end of new file"
+found=false
+for i in $(seq 20); do
+  grep 'has appeared;' out > /dev/null && { found=true; break; }
+  sleep .2
+done
+$found || { echo "$0: a: unexpected delay?"; cat out; fail=1; }
+
+echo y >> b
+# Wait up to 4s for "y" to appear in the output:
+found=false
+for i in $(seq 20); do
+  case $(tr '\n' @ < out) in
+    *'@@==> b <address@hidden@') found=true; break 2;;
+  esac
+  sleep .2
+done
+$found || { echo "$0: b: unexpected delay?"; cat out; fail=1; }
+
+echo z >> a
+# Wait up to 4s for "z" to appear in the output:
+found=false
+for i in $(seq 20); do
+  case $(tr '\n' @ < out) in
+    *'@@==> a <address@hidden@') found=true; break 2;;
+  esac
+  sleep .2
+done
+$found || { echo "$0: b: unexpected delay?"; cat out; fail=1; }
+
+kill -HUP $pid
+
+Exit $fail
--
1.6.6.rc3.271.g3d40f


>From d3a4bc86cd989ad7262545ae7e49868a2d1b59d4 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 30 Dec 2009 11:16:23 +0100
Subject: [PATCH 3/3] doc: mention two tail -F bug fixes in NEWS

* NEWS (Bug fixes): Two tail -F fixes.
---
 NEWS |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index 6128e3f..dd2ad6d 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,15 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   files that was not done from the local system would go unnoticed.
   [bug introduced in coreutils-7.5]

+  tail -F (inotify-enabled) would abort when a tailed file is repeated
+  renamed-aside and then recreated.
+  [bug introduced in coreutils-7.5]
+
+  tail -F (inotify-enabled) could fail to follow renamed files.
+  E.g., given a "tail -F a b" process, running "mv a b" would
+  make tail stop tracking additions to "b".
+  [bug introduced in coreutils-7.5]
+
   touch -a once again guarantees that a file's change time is
   adjusted, working around a bug in current Linux kernels.
   [bug introduced in coreutils-8.1]
--
1.6.6.rc3.271.g3d40f