tail: handle kernel dentry unlink race

Avoid the intermittent loss of "... has become inaccessible" messages.
That would cause tests/tail-2/assert.sh to fail sometimes,
mainly on uniprocessor systems.

* src/tail.c (tail_forever_inotify): Also monitor IN_DELETE
events on the directory, to avoid a dentry unlink()..open() race,
where the open() on the deleted file was seen to succeed after an,
unlink() and a subsequent IN_ATTRIB, was sent to tail.  Note an
IN_ATTRIB is sent on the monitored file to indicate the change in
number of links, and we can't just use a decrease in the number of
links to determine the file being unlinked, due to the possibility
of the file having multiple links.

Reported by Assaf Gordon and Ludovic Courtès.
Fixes http://bugs.gnu.org/21460

diff --git a/src/tail.c b/src/tail.c
index f916d7460395f0cee52c592bc3d160ac94697e73..2fa0ae0480536274ffdba3eeab30a0ae5c913d59 100644 (file)
--- a/src/tail.c
+++ b/src/tail.c
@@ -1429,8 +1429,8 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
                /* It's fine to add the same directory more than once.
                   In that case the same watch descriptor is returned.  */
               f[i].parent_wd = inotify_add_watch (wd, dirlen ? f[i].name : ".",
-                                                  (IN_CREATE | IN_MOVED_TO
-                                                   | IN_ATTRIB));
+                                                  (IN_CREATE | IN_DELETE
+                                                   | IN_MOVED_TO | IN_ATTRIB));
 
               f[i].name[dirlen] = prev;
 
@@ -1619,9 +1619,16 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
 
           fspec = &(f[j]);
 
-          /* Adding the same inode again will look up any existing wd.  */
-          int new_wd = inotify_add_watch (wd, f[j].name, inotify_wd_mask);
-          if (new_wd < 0)
+          int new_wd = -1;
+          bool deleting = !! (ev->mask & IN_DELETE);
+
+          if (! deleting)
+            {
+              /* Adding the same inode again will look up any existing wd.  */
+              new_wd = inotify_add_watch (wd, f[j].name, inotify_wd_mask);
+            }
+
+          if (! deleting && new_wd < 0)
             {
               if (errno == ENOSPC || errno == ENOMEM)
                 {
@@ -1639,7 +1646,8 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
             }
 
           /* This will be false if only attributes of file change.  */
-          bool new_watch = fspec->wd < 0 || new_wd != fspec->wd;
+          bool new_watch;
+          new_watch = (! deleting) && (fspec->wd < 0 || new_wd != fspec->wd);
 
           if (new_watch)
             {
@@ -1683,7 +1691,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
       if (! fspec)
         continue;
 
-      if (ev->mask & (IN_ATTRIB | IN_DELETE_SELF | IN_MOVE_SELF))
+      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
@@ -1695,6 +1703,14 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
               hash_delete (wd_to_name, fspec);
             }
 
+          /* Note we get IN_ATTRIB for unlink() as st_nlink decrements.
+             The usual path is a close() done in recheck() triggers
+             an IN_DELETE_SELF event as the inode is removed.
+             However sometimes open() will succeed as even though
+             st_nlink is decremented, the dentry (cache) is not updated.
+             Thus we depend on the IN_DELETE event on the directory
+             to trigger processing for the removed file.  */
+
           recheck (fspec, false);
 
           continue;