Subject: Re: bug#74653: tail --follow=name doesn't exit when using inotify and the file is moved

From: Pádraig Brady <[email-masked]>
Date: Thu, 5 Dec 2024 12:55:29 +0000

On 04/12/2024 11:55, Pádraig Brady wrote:
> On 02/12/2024 16:19, Göran Uddeborg wrote:
>> When using "tail --follow=name", but without "--retry", on a file
>> supporting inotify, the command doesn't finish if the file is moved to
>> a new name.
>>
>> Repeat this way on a local filesystem:
>>
>>       echo apa > apa
>>       tail --follow=name apa &
>>       mv apa bepa
>>
>> "tail" will print an error message saying "No such file or directory",
>> but it will continue running. I would expect a following message "no
>> files remaining" and "tail" to exit.
>>
>> If the file system doesn't support inotify, if "apa" is on NFS for
>> example, "tail" exits as expected.
>>
>> If the file is removed rather than moved ("rm apa") "tail" also exits
>> as expected.
>>
>> This is tested on Fedora 41 using
>>
>> coreutils-9.5-11.fc41.x86_64
>> kernel-6.11.6-300.fc41.x86_64
> 
> Yes that is a bug.
> The info docs state this:
> 
> "In that [renamed file] case, use ‘--follow=name’ to track the named file,
>    perhaps by reopening it periodically to see if it has been removed
>    and recreated by some other program.  Note that the inotify-based
>    implementation handles this case without the need for any periodic reopening."
> 
> But that description only alludes to the implementation,
> and not any functional difference I would say.
> I.e. inotify should not imply --retry.
> 
> Interestingly if you `rm depa` before recreating "apa" in the example above,
> then tail does exit.  Also tail will exit immediately if "apa" is not present
> at startup.  Also tail will exit if you move "apa" to a different directory.
> All of those existing behaviors gives us latitude to adjust this behavior I think,
> so that tail does in fact exit for the case above with inotify.
> 
> BTW one can test the non-inotify behavior on any file system
> with the undocumented ---disable-inotify option.

I'll push the attached later to fix this.

Marking this as done.

thanks,
Pádraig

From fd01fc8075a0df4e9036fdb962b589e60c134e26 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Wed, 4 Dec 2024 19:40:55 +0000
Subject: [PATCH] tail: ensure --follow=name unfollows renamed files

Require --retry to continue to track files upon rename.
We already unfollowed a file if it was renamed
to another file system (unlinked), so this makes the behavior
consistent if renaming to a file in the same file system.
I.e. --follow=name without --retry, means unfollow if the
name is unlinked or moved, so this change ensures that
behavior for all rename cases.
Related commits: v8.0-121-g3b997a9bc, v8.23-161-gd313a0b24

* src/tail.c (tail_forever_notify): Remove watch for a renamed file
if --retry is not specified.
* tests/tail/F-vs-rename.sh: Related test cleanup.
* tests/tail/follow-name.sh: Add a test case.
* NEWS: Mention the bug fix.
Fixes https://bugs.gnu.org/74653
---
 NEWS                      |  4 ++++
 src/tail.c                |  5 +++--
 tests/tail/F-vs-rename.sh | 10 +++++-----
 tests/tail/follow-name.sh | 13 ++++++++++---
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index 5bd9199e4..c1e604ffa 100644
--- a/NEWS
+++ b/NEWS
@@ -28,6 +28,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   'shuf' generates more-random output when the output is small.
   [bug introduced in coreutils-8.6]
 
+  `tail --follow=name` no longer waits indefinitely for watched
+  file names that are moved elsewhere within the same file system.
+  [bug introduced in coreutils-8.24]
+
   'tail -c 4096 /dev/zero' no longer loops forever.
   [This bug was present in "the beginning".]
 
diff --git a/src/tail.c b/src/tail.c
index 33acf9ab3..8cefb9c07 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1812,10 +1812,11 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
       if (ev->mask & (IN_ATTRIB | IN_DELETE | IN_DELETE_SELF | IN_MOVE_SELF))
         {
           /* Note for IN_MOVE_SELF (the file we're watching has
-             been clobbered via a rename) we leave the watch
+             been clobbered via a rename) without --retry we leave the watch
              in place since it may still be part of the set
              of watched names.  */
-          if (ev->mask & IN_DELETE_SELF)
+          if (ev->mask & IN_DELETE_SELF
+              || (!reopen_inaccessible_files && (ev->mask & IN_MOVE_SELF)))
             {
               inotify_rm_watch (wd, fspec->wd);
               hash_remove (wd_to_name, fspec);
diff --git a/tests/tail/F-vs-rename.sh b/tests/tail/F-vs-rename.sh
index df2422985..2b66033b8 100755
--- a/tests/tail/F-vs-rename.sh
+++ b/tests/tail/F-vs-rename.sh
@@ -63,26 +63,26 @@ for mode in '' '---disable-inotify'; do
     { cat out; fail=1; }
   # Wait up to 12.7s for "x" to be displayed:
   file='b' data='x' retry_delay_ check_tail_output .1 7 ||
-    { echo "$0: b: unexpected delay?"; cat out; fail=1; }
+    { echo "$0: b: unexpected delay 1?"; cat out; fail=1; }
 
   echo x2 > a
   # Wait up to 12.7s for this to appear in the output:
   # "tail: '...' has appeared;  following new file"
   tail_re='has appeared' retry_delay_ check_tail_output .1 7 ||
-    { echo "$0: a: unexpected delay?"; cat out; fail=1; }
+    { echo "$0: a: unexpected delay 2?"; cat out; fail=1; }
   # Wait up to 12.7s for "x2" to be displayed:
   file='a' data='x2' retry_delay_ check_tail_output .1 7 ||
-    { echo "$0: a: unexpected delay 2?"; cat out; fail=1; }
+    { echo "$0: a: unexpected delay 3?"; cat out; fail=1; }
 
   echo y >> b
   # Wait up to 12.7s for "y" to appear in the output:
   file='b' data='y' retry_delay_ check_tail_output .1 7 ||
-    { echo "$0: b: unexpected delay 2?"; cat out; fail=1; }
+    { echo "$0: b: unexpected delay 4?"; cat out; fail=1; }
 
   echo z >> a
   # Wait up to 12.7s for "z" to appear in the output:
   file='a' data='z' retry_delay_ check_tail_output .1 7 ||
-    { echo "$0: a: unexpected delay 3?"; cat out; fail=1; }
+    { echo "$0: a: unexpected delay 5?"; cat out; fail=1; }
 
   cleanup_
 done
diff --git a/tests/tail/follow-name.sh b/tests/tail/follow-name.sh
index 64864edfb..f92839c9e 100755
--- a/tests/tail/follow-name.sh
+++ b/tests/tail/follow-name.sh
@@ -23,13 +23,20 @@ cat <<\EOF > exp || framework_failure_
 tail: cannot open 'no-such' for reading: No such file or directory
 tail: no files remaining
 EOF
-
 returns_ 1 timeout 10 tail --follow=name no-such > out 2> err || fail=1
-
 # Remove an inconsequential inotify warning so
 # we can compare against the above error
 sed '/inotify cannot be used/d' err > k && mv k err
-
 compare exp err || fail=1
 
+# Between coreutils 8.34 and 9.5 inclusive, tail would have
+# waited indefinitely when a file was moved to the same file system
+cleanup_() { kill $pid 2>/dev/null && wait $pid; }
+touch file || framework_failure_
+timeout 10 tail --follow=name file & pid=$!
+sleep .1 # Usually in inotify loop here
+mv file file.unfollow || framework_failure_
+wait $pid
+test $? = 1 || fail=1
+
 Exit $fail
-- 
2.47.0