[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: tail aborts while following by name if using inotify
From: |
Jim Meyering |
Subject: |
Re: tail aborts while following by name if using inotify |
Date: |
Tue, 29 Dec 2009 12:05:38 +0100 |
Pádraig Brady wrote:
> On 28/12/09 14:52, Jim Meyering wrote:
>> +for i in $(seq 9); do touch $i; done || framework_failure
>
> I'd bump to 129 to be sure to be sure
> (or higher, keeping the max command line size in consideration)
> BTW this is a bit more efficient:
>
> seq 129 | xargs touch || framework_failure
But then it no longer triggers the bug.
I've added a comment that 9 is magic.
I may write another test along the lines of Rob's first example.
Here it is rewritten to use kill in place of ps:
touch f; tail -F f & pid=$!
while kill -0 $pid; do mv f g; touch f; done
>> +tail -qF $(seq 9)> out 2>&1& pid=$!
>> +
>> +# Wait until tail has started...
>> +echo x> 9
>> +while :; do grep '^x$' out>/dev/null 2>&1&& break; done
>
> Do you want to sleep a bit like:
>
> until grep '^x$' out >/dev/null 2>&1; do
> sleep .1
> done
Not really. I figured the fork/exec-induced delay would be enough.
>> +mv 1 f || fail=1
>> +
>> +# Wait for this diagnostic:
>> +# tail: `1' has become inaccessible: No such file or directory
>> +while :; do grep 'inaccessible' out>/dev/null 2>&1&& break; done
>> +echo a> 1 || fail=1
>> +
>> +# Before the fix, the above creation of a moved-aside file
>> +# would have caused tail to abort. In that case, appending to
>> +# a tailed file and waiting for that new line to appear in tail's
>> +# output would have been an infloop.
>> +if kill -0 $pid; then
>> + # Wait for tail to detect the changes.
>> + echo y>> 8
>> + while false ; do
>> + grep '^y$' out>/dev/null 2>&1&& break
>> + done
>
> I'm a bit confused by the comment above.
> Do you not want to execute the `while false` at all?
Thanks for the review.
That "while false" was a debugging artifact.
Turns out there's a much better way to handle that part of the test.
>> +wait $pid
>
> Is there a `kill $pid` missing for when tail actually doesn't fall over?
Yes <blush> ;-)
Here's a better version:
>From fbc1dab0434b61754d7d73e81c125e7e61c9ca2a Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 28 Dec 2009 15:42:10 +0100
Subject: [PATCH] tail: add a test to exercise abort-inducing flaw in tail -F
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* tests/tail-2/inotify-hash-abuse: New file, derived from
a report by Rob Wortman.
* tests/Makefile.am (TESTS): Add it.
Improved by: Pádraig Brady.
---
THANKS | 1 +
tests/Makefile.am | 1 +
tests/tail-2/inotify-hash-abuse | 68 +++++++++++++++++++++++++++++++++++++++
3 files changed, 70 insertions(+), 0 deletions(-)
create mode 100755 tests/tail-2/inotify-hash-abuse
diff --git a/THANKS b/THANKS
index 668d59f..86be252 100644
--- a/THANKS
+++ b/THANKS
@@ -512,6 +512,7 @@ Richard Sharman address@hidden
Rick Sladkey address@hidden
Rik Faith address@hidden
Risto Kankkunen address@hidden
+Rob Wortman address@hidden
Robert H. de Vries address@hidden
Robert Lindgren address@hidden
Robert Millan address@hidden
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 93d4275..35ea8b2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -80,6 +80,7 @@ TESTS = \
rm/ext3-perf \
rm/cycle \
cp/link-heap \
+ tail-2/inotify-hash-abuse \
tail-2/inotify-rotate \
chmod/no-x \
chgrp/basic \
diff --git a/tests/tail-2/inotify-hash-abuse b/tests/tail-2/inotify-hash-abuse
new file mode 100755
index 0000000..9fe656f
--- /dev/null
+++ b/tests/tail-2/inotify-hash-abuse
@@ -0,0 +1,68 @@
+#!/bin/sh
+# Exercise an abort-inducing flaw in inotify-enabled tail -F.
+
+# Copyright (C) 2009 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/>.
+
+if test "$VERBOSE" = yes; then
+ set -x
+ tail --version
+fi
+
+. $srcdir/test-lib.sh
+
+# 9 is a magic number, related to internal details of tail.c and hash.c
+n=9
+seq $n | xargs touch || framework_failure
+
+debug='---disable-inotify -s .01'
+debug=
+tail $debug -qF $(seq $n) > out 2>&1 & pid=$!
+
+# Wait until tail has started...
+echo x > $n
+until grep '^x$' out >/dev/null 2>&1; do :; done
+
+mv 1 f || fail=1
+
+# Wait for this diagnostic:
+# tail: `1' has become inaccessible: No such file or directory
+until grep inaccessible out >/dev/null 2>&1; do :; done
+
+# Trigger the bug. Before the fix, this would provoke the abort.
+echo a > 1 || fail=1
+
+# Wait up to 2s for the buggy tail to die,
+# or for the "tail: `1' has appeared; following end of new file" output
+dead=0
+for i in $(seq 10); do
+ kill -0 $pid || { dead=1; break; }
+ grep 'has appeared;' out > /dev/null && break
+done
+
+# Fixed tail will not have aborted. Kill it.
+test $dead = 0 && kill -HUP $pid
+
+wait $pid
+st=$?
+
+case $st in
+ 129) ;;
+ *) echo tail died via unexpected signal: $st; fail=1;;
+esac
+
+cat out
+
+Exit $fail
--
1.6.6.301.g5794