GNU bug report logs - #26363
[PATCH] tail: 'tail -F dir/file' reverts to polling mode if 'dir' is removed

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 26363 in the body.
You can then email your comments to 26363 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Sebastian Kisela <skisela <at> redhat.com>
To: bug-coreutils <at> gnu.org
Subject: [PATCH] tail: 'tail -F dir/file' reverts to polling mode if 'dir' is
 removed
Date: Tue, 4 Apr 2017 18:47:46 +0200

[Message part 1 (text/plain, inline)]

* src/tail.c (tail_forever_inotify):  Add the IN_DELETE_SELF flag when
creating watch for the parent directory.  After the parent directory
is removed, an event is caught and then we switch from inotify to
polling mode.  Till now, inotify has always frozen because it waited for
an event from a watched dir, which has been already deleted and was not
added again.
* tests/tail-2/inotify-dir-recreate.sh: Add a test case.
* tests/local.mk: Reference the new test.
* NEWS: Mention the bug fix.

Reported at https://bugzilla.redhat.com/1283760
---
 NEWS                                 |  2 ++
 src/tail.c                           | 21 +++++++++++-
 tests/local.mk                       |  1 +
 tests/tail-2/inotify-dir-recreate.sh | 66
++++++++++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100755 tests/tail-2/inotify-dir-recreate.sh

diff --git a/NEWS b/NEWS
index e2f298f..f34d667 100644
--- a/NEWS
+++ b/NEWS
@@ -74,6 +74,8 @@ GNU coreutils NEWS                                    -*-
outline -*-
   'cp -fl A B' no longer remove B before creating the new link.
   That is, there is no longer a brief moment when B does not exist.

+  tail -F 'dir/file' reverts to polling mode in case 'dir' is removed.
+
 ** New features

   expand and unexpand now support specifying a tab size to use
diff --git a/src/tail.c b/src/tail.c
index d1552d4..e35ee1e 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1457,7 +1457,8 @@ tail_forever_inotify (int wd, struct File_spec *f,
size_t n_files,
                   In that case the same watch descriptor is returned.  */
               f[i].parent_wd = inotify_add_watch (wd, dirlen ? f[i].name :
".",
                                                   (IN_CREATE | IN_DELETE
-                                                   | IN_MOVED_TO |
IN_ATTRIB));
+                                                   | IN_MOVED_TO |
IN_ATTRIB
+                                                   | IN_DELETE_SELF));

               f[i].name[dirlen] = prev;

@@ -1628,6 +1629,24 @@ tail_forever_inotify (int wd, struct File_spec *f,
size_t n_files,
       ev = void_ev;
       evbuf_off += sizeof (*ev) + ev->len;

+      /* If a directory is deleted, IN_DELETE_SELF is emmited
+         with ev->name of length 0.
+         We need to catch it, otherwise it would wait forever,
+         as wd for directory becomes inactive. Revert to polling now.   */
+      if ((ev->mask & IN_DELETE_SELF) && !ev->len)
+        {
+          for (i = 0; i < n_files; i++)
+            {
+              if (ev->wd == f[i].parent_wd)
+                {
+                  hash_free (wd_to_name);
+                  error (0, 0,
+                      _("directory containing watched file was removed"));
+                  return true;
+                }
+            }
+        }
+
       if (ev->len) /* event on ev->name in watched directory.  */
         {
           size_t j;
diff --git a/tests/local.mk b/tests/local.mk
index 9f1a853..bcf70c2 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -176,6 +176,7 @@ all_tests =                    \
   tests/tail-2/descriptor-vs-rename.sh        \
   tests/tail-2/inotify-rotate.sh        \
   tests/tail-2/inotify-rotate-resources.sh    \
+  tests/tail-2/inotify-dir-recreate.sh        \
   tests/chmod/no-x.sh                \
   tests/chgrp/basic.sh                \
   tests/rm/dangling-symlink.sh            \
diff --git a/tests/tail-2/inotify-dir-recreate.sh
b/tests/tail-2/inotify-dir-recreate.sh
new file mode 100755
index 0000000..ed5f339
--- /dev/null
+++ b/tests/tail-2/inotify-dir-recreate.sh
@@ -0,0 +1,66 @@
+#!/bin/sh
+# Makes sure, inotify will switch to polling mode if directory
+# of the watched file was removed and recreated.
+# (...instead of getting stuck forever)
+
+# Copyright (C) 2006-2017 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
+
+
+# Terminate any background tail process
+cleanup_() { kill $pid 2>/dev/null && wait $pid; }
+
+# Check for existence of the important message
+check_tail_output_ ()
+{
+  grep "$1" out &>/dev/null ||  fail=1
+}
+
+# Prepare the file to be watched
+mkdir dir && touch dir/file
+sleep 1
+
+timeout 60 tail --follow=name --retry dir/file &>out  & pid=$!
+sleep 1
+
+# Remove the directory, should get the massage about the deletion
+rm -r dir
+sleep 1
+
+# Recreate the dir, should get a message about recreation
+mkdir dir && touch dir/file
+sleep 1
+
+cleanup_
+rm -r dir
+
+# Expected result for the whole process
+cat <<\EOF > exp
+tail: 'dir/file' has become inaccessible: No such file or directory
+tail: directory containing watched file was removed
+tail: inotify cannot be used, reverting to polling: No such file or
directory
+tail: 'dir/file' has appeared;  following new file
+EOF
+
+# If contains 'has appeared' string, than
+# the polling mode has been reactivated succesfully
+check_tail_output_ 'has appeared'
+
+compare exp out || fail=1
+
+Exit $fail
-- 
2.9.3

[Message part 2 (text/html, inline)]

Message #8 received at 26363 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Sebastian Kisela <skisela <at> redhat.com>, 26363 <at> debbugs.gnu.org
Subject: Re: bug#26363: [PATCH] tail: 'tail -F dir/file' reverts to polling
 mode if 'dir' is removed
Date: Tue, 4 Apr 2017 19:16:28 -0700

On 04/04/17 09:47, Sebastian Kisela wrote:
> * src/tail.c (tail_forever_inotify):  Add the IN_DELETE_SELF flag when
> creating watch for the parent directory.  After the parent directory
> is removed, an event is caught and then we switch from inotify to
> polling mode.  Till now, inotify has always frozen because it waited for
> an event from a watched dir, which has been already deleted and was not
> added again.
> * tests/tail-2/inotify-dir-recreate.sh: Add a test case.
> * tests/local.mk: Reference the new test.
> * NEWS: Mention the bug fix.
> 
> Reported at https://bugzilla.redhat.com/1283760

Excellent. That looks like a fine fall back.

Not having looked at the C code,
have you considered if another dir/files are already setup with inotify watches,
and if files are deleted but still open?
Other comments below...

> @@ -1628,6 +1629,24 @@ tail_forever_inotify (int wd, struct File_spec *f,
> size_t n_files,
>        ev = void_ev;
>        evbuf_off += sizeof (*ev) + ev->len;
> 
> +      /* If a directory is deleted, IN_DELETE_SELF is emmited
> +         with ev->name of length 0.
> +         We need to catch it, otherwise it would wait forever,
> +         as wd for directory becomes inactive. Revert to polling now.   */
> +      if ((ev->mask & IN_DELETE_SELF) && !ev->len)
> +        {
> +          for (i = 0; i < n_files; i++)
> +            {
> +              if (ev->wd == f[i].parent_wd)
> +                {
> +                  hash_free (wd_to_name);
> +                  error (0, 0,
> +                      _("directory containing watched file was removed"));
> +                  return true;
> +                }
> +            }
> +        }
> +
>        if (ev->len) /* event on ev->name in watched directory.  */
>          {
>            size_t j;

> +
> +# Check for existence of the important message
> +check_tail_output_ ()
> +{
> +  grep "$1" out &>/dev/null ||  fail=1
> +}
> +
> +# Prepare the file to be watched
> +mkdir dir && touch dir/file
> +sleep 1

Why this sleep?

> +timeout 60 tail --follow=name --retry dir/file &>out  & pid=$!
> +sleep 1

We can't depend on absolute sleeps like this. See:
http://www.pixelbeat.org/docs/coreutils-testing.html#backoff
or the existing tail-2/ tests

cheers,
Pádraig

Message #11 received at 26363 <at> debbugs.gnu.org (full text, mbox):

From: Sebastian Kisela <skisela <at> redhat.com>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: 26363 <at> debbugs.gnu.org
Subject: Re: bug#26363: [PATCH] tail: 'tail -F dir/file' reverts to polling
 mode if 'dir' is removed
Date: Wed, 5 Apr 2017 11:04:11 +0200

[Message part 1 (text/plain, inline)]

> Excellent. That looks like a fine fall back.
>
> Not having looked at the C code,
> have you considered if another dir/files are already setup with inotify
> watches,
> and if files are deleted but still open?
> Other comments below...
>

I changed the behaviour of tail to exit inotify mode if at least one of the
watched directories is deleted.

If I understand you correctly, there should not be problem with files that
are deleted while being open.
The files are tailed again in the polling mode, after the path leading to
them is recreated(tail -F uses following by name not by fd).

I also thought about entering inotify mode again after all watched
directories(and the files) are found by polling mode.
What I made so far, contained too many deleted and recreated watches due to
exiting inotify(deleting watches to all
inotify watch descriptors) mode immediately after at least one watched
directory is removed.

I have not found any discussion about such solution, therefore I am unsure
about the safety of the solution so far.


> Why this sleep?
>
> > +timeout 60 tail --follow=name --retry dir/file &>out  & pid=$!
> > +sleep 1
>
> We can't depend on absolute sleeps like this. See:
> http://www.pixelbeat.org/docs/coreutils-testing.html#backoff
> or the existing tail-2/ tests
>

I apologize, thanks for the tip.
See V2 patch attached with correct usage of sleep by

*retry_delay_ ().*

*Sebastian.*

[Message part 2 (text/html, inline)]

[inotify-dir-recreate-V2.patch (text/x-patch, attachment)]

Message #16 received at 26363-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Sebastian Kisela <skisela <at> redhat.com>
Cc: 26363-done <at> debbugs.gnu.org
Subject: Re: bug#26363: [PATCH] tail: 'tail -F dir/file' reverts to polling
 mode if 'dir' is removed
Date: Wed, 5 Apr 2017 17:46:56 -0700

Pushed with some NEWS and test tweaks at:
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=ba5fe2d

Marking this as done.

thanks!
Pádraig.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.