diff options
author | Pádraig Brady <P@draigBrady.com> | 2015-05-05 22:11:24 +0100 |
---|---|---|
committer | Pádraig Brady <P@draigBrady.com> | 2015-05-11 23:03:32 +0100 |
commit | cf06a872e74b45588c2e64903f7fc99cf2aafe27 (patch) | |
tree | 56ab22a4de51b29f1f49f2e02f506be416b06244 /tests/tail-2/inotify-race2.sh | |
parent | 9069f4ec850364e778c51bfa6f1ab761be67caec (diff) | |
download | coreutils-cf06a872e74b45588c2e64903f7fc99cf2aafe27.tar.xz |
tail: fix inotify startup races
The previous fixes to races in the various tail tests,
identified actual races in the tail inotify implementation.
With --follow=descriptor, if the tailed file was replaced before
the inotify watch was added, then any subsequent changes were ignored.
Similarly in --follow=name mode, all changes to a new name were
effectively ignored if that name was created after the original open()
but before the inotify_add_watch().
* src/tail.c (tail_forever_inotify): Fix 3 cases.
1. With -f, don't stop tailing when file removed before watch.
2. With -f, watch right file when file replaced before watch.
3. With -F, inspect correct file when replaced before watch.
Existing tests identify these when tail compiled with TAIL_TEST_SLEEP.
* tests/tail-2/inotify-rotate-resources.sh:
This test also identifies the issue with --follow=name
when TAIL_TEST_SLEEP is used. Adjust so the test is immune
to such races, and also fail quicker on remote file systems.
* tests/tail-2/inotify-race2.sh: A new test using GDB,
based on inotify-race.sh, which tests the -F race
without needed recompilation with sleeps.
* tests/local.mk: Reference the new test.
* NEWS: Mention the bug.
Diffstat (limited to 'tests/tail-2/inotify-race2.sh')
-rwxr-xr-x | tests/tail-2/inotify-race2.sh | 103 |
1 files changed, 103 insertions, 0 deletions
diff --git a/tests/tail-2/inotify-race2.sh b/tests/tail-2/inotify-race2.sh new file mode 100755 index 000000000..6d996eba7 --- /dev/null +++ b/tests/tail-2/inotify-race2.sh @@ -0,0 +1,103 @@ +#!/bin/sh +# Ensure that tail does not ignore a tailed-forever file that has been +# replaced between tail's initial read-to-EOF, and when the inotify watches +# are established in tail_forever_inotify. That new file would be ignored +# indefinitely. + +# Copyright (C) 2015 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/>. + +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src +print_ver_ tail + +# Terminate any background gdb/tail process +cleanup_() { + kill $pid 2>/dev/null && wait $pid + kill $sleep 2>/dev/null && wait $sleep +} + +touch file || framework_failure_ +touch tail.out || framework_failure_ + +( timeout 10s gdb --version ) > gdb.out 2>&1 +case $(cat gdb.out) in + *'GNU gdb'*) ;; + *) skip_ "can't run gdb";; +esac + +# Break on a line rather than a symbol, to cater for inline functions +break_src="$abs_top_srcdir/src/tail.c" +break_line=$(grep -n ^tail_forever_inotify "$break_src") || framework_failure_ +break_line=$(echo "$break_line" | cut -d: -f1) || framework_failure_ + + +# Note we get tail to monitor a background sleep process +# rather than using timeout(1), as timeout sends SIGCONT +# signals to its monitored process, and gdb (7.9 at least) +# has _intermittent_ issues with this. +# Sending SIGCONT resulted in either delayed child termination, +# or no child termination resulting in a hung test. +# See https://sourceware.org/bugzilla/show_bug.cgi?id=18364 + +env sleep 10 & sleep=$! + +# See if gdb works and +# tail_forever_inotify is compiled and run +gdb -nx --batch-silent \ + --eval-command="break $break_line" \ + --eval-command="run --pid=$sleep -f file" \ + --eval-command='quit' \ + tail < /dev/null > gdb.out 2>&1 + +kill $sleep || skip_ 'breakpoint not hit' +wait $sleep + +# FIXME: The above is seen to _intermittently_ fail with: +# warning: .dynamic section for "/lib/libc.so.6" is not at the expected address +# warning: difference appears to be caused by prelink, adjusting expectations +compare /dev/null gdb.out || skip_ "can't set breakpoints in tail" + +env sleep 10 & sleep=$! + +echo never-seen-with-tail-8.23 > file.new || framework_failure_ + +# Run "tail -F file", stopping to replace with a new file before +# inotify initialization, and then continue. Before the fix, +# changes to the new file would effectively be ignored. +gdb -nx --batch-silent \ + --eval-command="break $break_line" \ + --eval-command="run --pid=$sleep -F file 2>tail.err >>tail.out" \ + --eval-command='shell mv file.new file' \ + --eval-command='continue' \ + --eval-command='quit' \ + tail < /dev/null > /dev/null 2>&1 & pid=$! + +# Note even updating the watched 'file' wouldn't have output +# anything between coreutils 7.5 and 8.23 inclusive as +# The old file descriptor (still held open by tail) was being fstat(). + +tail --pid=$pid -f tail.out | (read; kill $pid) + +# gdb has a bug in Debian's gdb-6.8-3 at least that causes it to not +# cleanup and exit correctly when it receives a SIGTERM, but +# killing sleep, should cause the tail process and thus gdb to exit. +kill $sleep +wait $sleep + +wait $pid + +compare /dev/null tail.out && { cat tail.err; fail=1; } + +Exit $fail |