tail: avoid a race where we could miss new data with --pid

* src/tail.c (tail_forever, tail_forever_inotify): Close a race in
tail_forever_inotify where new data written after the file check by
a now dead process, but before the pid check, is not output.  We use
the POSIX guarantee that read() and write() are serialized wrt each
other even in separate processes, to assume full file consistency
after exit() and so poll for new data _after_ the writer has exited.
This also allows us to not redundantly _wait_ for new data if the
process is dead.
* tests/tail-2/pid: Remove the now partially invalid sub second sleep
check as we now don't unconditionally wait, and replace it with a check
for the redundant sleep.  Also clarify some of the existing comments.
* NEWS: Mention the fix.

1 files changed, 24 insertions, 22 deletions

diff --git a/src/tail.c b/src/tail.c
index f9f8c3904..f3fe6a341 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1135,9 +1135,6 @@ tail_forever (struct File_spec *f, size_t n_files, double sleep_interval)
           if (writer_is_dead)
             break;
 
-          if (xnanosleep (sleep_interval))
-            error (EXIT_FAILURE, errno, _("cannot read realtime clock"));
-
           /* Once the writer is dead, read the files once more to
              avoid a race condition.  */
           writer_is_dead = (pid != 0
@@ -1146,6 +1143,10 @@ tail_forever (struct File_spec *f, size_t n_files, double sleep_interval)
                                signal to the writer, so kill fails and sets
                                errno to EPERM.  */
                             && errno != EPERM);
+
+          if (!writer_is_dead && xnanosleep (sleep_interval))
+            error (EXIT_FAILURE, errno, _("cannot read realtime clock"));
+
         }
     }
 }
@@ -1179,6 +1180,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
   Hash_table *wd_table;
 
   bool found_watchable = false;
+  bool writer_is_dead = false;
   int prev_wd;
   size_t evlen = 0;
   char *evbuf;
@@ -1266,30 +1268,30 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
          indefinetely.  */
       if (pid)
         {
-          fd_set rfd;
-          struct timeval select_timeout;
-          int n_descriptors;
-
-          FD_ZERO (&rfd);
-          FD_SET (wd, &rfd);
+          if (writer_is_dead)
+            exit (EXIT_SUCCESS);
 
-          select_timeout.tv_sec = (time_t) sleep_interval;
-          select_timeout.tv_usec = 1000000 * (sleep_interval
-                                              - select_timeout.tv_sec);
+          writer_is_dead = (kill (pid, 0) != 0 && errno != EPERM);
 
-          n_descriptors = select (wd + 1, &rfd, NULL, NULL, &select_timeout);
+          struct timeval delay; /* how long to wait for file changes.  */
+          if (writer_is_dead)
+            delay.tv_sec = delay.tv_usec = 0;
+          else
+            {
+              delay.tv_sec = (time_t) sleep_interval;
+              delay.tv_usec = 1000000 * (sleep_interval - delay.tv_sec);
+            }
 
-          if (n_descriptors == -1)
-            error (EXIT_FAILURE, errno, _("error monitoring inotify event"));
+           fd_set rfd;
+           FD_ZERO (&rfd);
+           FD_SET (wd, &rfd);
 
-          if (n_descriptors == 0)
-            {
-              /* See if the process we are monitoring is still alive.  */
-              if (kill (pid, 0) != 0 && errno != EPERM)
-                exit (EXIT_SUCCESS);
+           int file_change = select (wd + 1, &rfd, NULL, NULL, &delay);
 
-              continue;
-            }
+           if (file_change == 0)
+             continue;
+           else if (file_change == -1)
+             error (EXIT_FAILURE, errno, _("error monitoring inotify event"));
         }
 
       if (len <= evbuf_off)