Re: [PATCH] tests: tail-2/pid: use a 3-second timeout, not 1

[Jim Meyering]

Pádraig Brady wrote:
> Actually taking a closer look this morning shows
> a race in tail_forever_inotify().
> Hopefully the attached addresses both that and the redundant delay.
...
> Subject: [PATCH] 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.
> ---
>  NEWS             |    6 ++++++
>  gnulib           |    2 +-
>  src/tail.c       |   46 ++++++++++++++++++++++++----------------------
>  tests/tail-2/pid |   15 ++++++++-------
>  4 files changed, 39 insertions(+), 30 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index e7e4ddc..af5222c 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -22,6 +22,12 @@ GNU coreutils NEWS                                    -*- 
> outline -*-
>    from a failed stat/lstat.  For example ls -Lis now prints "?", not "0",
>    for the inode number and allocated size of a dereferenced dangling symlink.
>
> +  tail --follow --pid now avoids a race condition where data written by the
> +  specified pid right before exiting might not have been output by tail.
> +  Also we now don't delay if the specified pid is not present.
> +  [The race was introduced in coreutils-7.5,
> +   and the redundant delay was present since textutils-1.22o]

Good catch!
Thanks!

Please reword so that we don't imply that the specified PID
must be the writer.  Also, I prefer to say "tail no longer..."
than "we...":

   tail --follow --pid now avoids a race condition where data written
   just before the process dies might not have been output by tail.
   Also, tail no longer delays at all when the specified pid is not live.
   [The race was introduced in coreutils-7.5,
    and the unnecessary delay was present since textutils-1.22o]

> diff --git a/gnulib b/gnulib
> index e21985a..772a85a 160000
> --- a/gnulib
> +++ b/gnulib
> @@ -1 +1 @@
> -Subproject commit e21985ad14508137d75b0dccf064adfc4e5888c6
> +Subproject commit 772a85a912a2bd5a25406d9f24a587269ea3b709

> diff --git a/src/tail.c b/src/tail.c
...
> diff --git a/tests/tail-2/pid b/tests/tail-2/pid
...

The rest looks fine.
I confirmed it passes the test with the fixed version
and fails with the old.