On 07/06/17 01:00, Pádraig Brady wrote:
> The following will hang indefinitely, due to no further data
> being written through the pipe, and thus no SIGPIPE generated:
>
> $ tail -f /etc/hosts | sleep 1
>
> A more practical use case might be:
>
> tail -f file.log | grep -q trigger &&
> process_immediately
>
> Another case where one might notice this is that sometimes
> (depending on how slow the reader is) the following will hang,
> and you may not notice the issue until data arrives
>
> tail -f file.log | grep_typo filter
>
> Below is a quick hack to support this,
> and tail now terminates promptly for the above cases.
>
> The implementation is a proof on concept done in a few minutes
> (and may change from poll() to select() if possible).
> A disadvantage is the extra syscalls to do the polling.
> Actually I may be able to combine the existing select()
> with this new poll(), and thus wait for changes in the
> output as well as the inotify descriptor.
>
> cheers,
> Pádraig.
>
>
> diff --git a/src/tail.c b/src/tail.c
> +/* If the output has gone away, then terminate
> + as we would if we had written to this output. */
> +static void
> +check_output (void)
> +{
> + struct pollfd pfd = {.fd = STDOUT_FILENO, .events = POLLERR};
> + if (poll (&pfd, 1, 0) >= 0 && (pfd.revents & POLLERR))
> + raise (SIGPIPE);
> +}
> +
The attached changes from using poll() to using select(),
which was already used in tail(1), and avoids the repeated
poll() calls in the normal case.
cheers,
Pádraig
>From de5659a015d4e3e2d5dce65e8fb16cebf2b09d26 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Wed, 7 Jun 2017 01:00:28 -0700
Subject: [PATCH] tail: exit promptly when output no longer writable
This will support use cases like:
tail -f file.log | grep -q trigger &&
process_immediately
* src/tail.c (check_output_alive): A new function that
uses select on fifos or pipes to detect if they're broken.
(tail_forever): Call check_output_alive() periodically.
(tail_forever_inotify): Merge the select() call from
check_output_alive() into the select() originally present
for the --pid case, and adjust accordingly.
* tests/tail-2/pipe-f.sh: Add test cases.
* NEWS: Mention the improvement.
---
NEWS | 3 ++
src/tail.c | 77 +++++++++++++++++++++++++++++++++++++++++---------
tests/tail-2/pipe-f.sh | 21 +++++++++++---
3 files changed, 84 insertions(+), 17 deletions(-)
diff --git a/NEWS b/NEWS
index 508c08f..d2672e8 100644
--- a/NEWS
+++ b/NEWS
@@ -53,6 +53,9 @@ GNU coreutils NEWS -*- outline -*-
mv --verbose now distinguishes rename and copy operations.
+ tail -f will now exit immediately if the output is piped
+ and the reader of the pipe terminates.
+
* Noteworthy changes in release 8.27 (2017-03-08) [stable]
diff --git a/src/tail.c b/src/tail.c
index 3582321..9329145 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -175,6 +175,9 @@ static enum Follow_mode follow_mode = Follow_descriptor;
/* If true, read from the ends of all specified files until killed. */
static bool forever;
+/* If true, monitor output so we exit if pipe reader terminates. */
+static bool monitor_output;
+
/* If true, count from start of file instead of end. */
static bool from_start;
@@ -327,6 +330,27 @@ named file in a way that accommodates renaming, removal and creation.\n\
exit (status);
}
+/* If the output has gone away, then terminate
+ as we would if we had written to this output. */
+static void
+check_output_alive (void)
+{
+ if (! monitor_output)
+ return;
+
+ struct timeval delay;
+ delay.tv_sec = delay.tv_usec = 0;
+
+ fd_set rfd;
+ FD_ZERO (&rfd);
+ FD_SET (STDOUT_FILENO, &rfd);
+
+ /* readable event on STDOUT is equivalent to POLLERR,
+ and implies an error condition on output like broken pipe. */
+ if (select (STDOUT_FILENO + 1, &rfd, NULL, NULL, &delay) == 1)
+ raise (SIGPIPE);
+}
+
static bool
valid_file_spec (struct File_spec const *f)
{
@@ -1244,6 +1268,8 @@ tail_forever (struct File_spec *f, size_t n_files, double sleep_interval)
if ((!any_input || blocking) && fflush (stdout) != 0)
die (EXIT_FAILURE, errno, _("write error"));
+ check_output_alive ();
+
/* If nothing was read, sleep and/or check for dead writers. */
if (!any_input)
{
@@ -1577,32 +1603,48 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
/* When watching a PID, ensure that a read from WD will not block
indefinitely. */
- if (pid && (len <= evbuf_off))
+ while (len <= evbuf_off)
{
- if (writer_is_dead)
- exit (EXIT_SUCCESS);
-
- writer_is_dead = (kill (pid, 0) != 0 && errno != EPERM);
-
struct timeval delay; /* how long to wait for file changes. */
- if (writer_is_dead)
- delay.tv_sec = delay.tv_usec = 0;
- else
+
+ if (pid)
{
- delay.tv_sec = (time_t) sleep_interval;
- delay.tv_usec = 1000000 * (sleep_interval - delay.tv_sec);
+ if (writer_is_dead)
+ exit (EXIT_SUCCESS);
+
+ writer_is_dead = (kill (pid, 0) != 0 && errno != EPERM);
+
+ 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);
+ }
}
fd_set rfd;
FD_ZERO (&rfd);
FD_SET (wd, &rfd);
+ if (monitor_output)
+ FD_SET (STDOUT_FILENO, &rfd);
- int file_change = select (wd + 1, &rfd, NULL, NULL, &delay);
+ int file_change = select (MAX (wd, STDOUT_FILENO) + 1,
+ &rfd, NULL, NULL, pid ? &delay: NULL);
if (file_change == 0)
continue;
else if (file_change == -1)
- die (EXIT_FAILURE, errno, _("error monitoring inotify event"));
+ die (EXIT_FAILURE, errno,
+ _("error waiting for inotify and output events"));
+ else if (FD_ISSET (STDOUT_FILENO, &rfd))
+ {
+ /* readable event on STDOUT is equivalent to POLLERR,
+ and implies an error on output like broken pipe. */
+ raise (SIGPIPE);
+ }
+ else
+ break;
}
if (len <= evbuf_off)
@@ -2350,6 +2392,15 @@ main (int argc, char **argv)
if (forever && ignore_fifo_and_pipe (F, n_files))
{
+ /* If stdout is a fifo or pipe, then monitor it
+ so that we exit if the reader goes away.
+ Note select() on a regular file is always readable. */
+ struct stat out_stat;
+ if (fstat (STDOUT_FILENO, &out_stat) < 0)
+ die (EXIT_FAILURE, errno, _("standard output"));
+ monitor_output = (S_ISFIFO (out_stat.st_mode)
+ || (HAVE_FIFO_PIPES != 1 && isapipe (STDOUT_FILENO)));
+
#if HAVE_INOTIFY
/* tailable_stdin() checks if the user specifies stdin via "-",
or implicitly by providing no arguments. If so, we won't use inotify.
diff --git a/tests/tail-2/pipe-f.sh b/tests/tail-2/pipe-f.sh
index 00b99d9..b2ea506 100755
--- a/tests/tail-2/pipe-f.sh
+++ b/tests/tail-2/pipe-f.sh
@@ -1,5 +1,5 @@
#!/bin/sh
-# ensure that :|tail -f doesn't hang, per POSIX
+# ensure that tail -f doesn't hang in various cases
# Copyright (C) 2009-2017 Free Software Foundation, Inc.
@@ -19,15 +19,28 @@
. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
print_ver_ tail
+# Speedup the non inotify case
+fastpoll='-s.1 --max-unchanged-stats=1'
+
+for mode in '' '---disable-inotify'; do
+# Ensure :|tail -f doesn't hang, per POSIX
echo oo > exp || framework_failure_
-echo foo | timeout 10 tail -f -c3 > out || fail=1
+echo foo | timeout 10 tail -f $mode $fastpoll -c3 > out || fail=1
compare exp out || fail=1
-
cat <<\EOF > exp || framework_failure_
==> standard input <==
ar
EOF
-echo bar | returns_ 1 timeout 10 tail -f -c3 - missing > out || fail=1
+echo bar | returns_ 1 \
+ timeout 10 tail -f $mode $fastpoll -c3 - missing > out || fail=1
compare exp out || fail=1
+# This would wait indefinitely before v8.28 due to no EPIPE being
+# generated due to no data written after the first small amount
+timeout 10 tail -f $mode $fastpoll out | sleep .1 || fail=1
+
+# This would wait indefinitely before v8.28 (until first write)
+returns_ 1 timeout 10 tail -f $mode $fastpoll /dev/null >&- || fail=1
+done
+
Exit $fail
--
2.9.3
