Jim Meyering wrote: ... > While I do have a reproducer that will become a test suite addition > (coming soon), it relies on python (a first) and the python-inotify > package, so I'll have to be careful to skip the test when those > prerequisites aren't installed. Also, there's an inherent race condition, > so I'll have to find the right compromise between absolute test-robustness > (too expensive in time and inodes) and reasonableness.
Here's a complete patch, including the test. I would have preferred to avoid the one-second sleep. Maybe someone here can see a better way. >From 9bf47055f64efb56d022feca01ad901e85e21bc1 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Sat, 8 Jan 2011 17:44:55 +0100 Subject: [PATCH] du: don't abort when a subdir is renamed during traversal * NEWS (Bug fixes): Mention it. * src/du.c (prev_level): Move declaration "up" to file-scope global. (du_files): Reset prev_level to 0 upon abnormal fts_read termination. Reported by Johathan Nieder in http://bugs.debian.org/609049 Also, improve a diagnostic. * tests/du/move-dir-while-traversing: Test for the above. * tests/Makefile.am (TESTS): Add it. --- NEWS | 8 +++ src/du.c | 15 +++++-- tests/Makefile.am | 1 + tests/du/move-dir-while-traversing | 85 ++++++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 4 deletions(-) create mode 100755 tests/du/move-dir-while-traversing diff --git a/NEWS b/NEWS index 2a71ca6..5a70243 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,14 @@ GNU coreutils NEWS -*- outline -*- * Noteworthy changes in release ?.? (????-??-??) [?] +** Bug fixes + + du would abort with a failed assertion when two conditions are met: + part of the hierarchy being traversed is moved to a higher level in the + directory tree, and there is at least one more command line directory + argument following the one containing the moved sub-tree. + [bug introduced in coreutils-5.1.0] + * Noteworthy changes in release 8.9 (2011-01-04) [stable] diff --git a/src/du.c b/src/du.c index 77deb0c..671cac7 100644 --- a/src/du.c +++ b/src/du.c @@ -63,8 +63,11 @@ extern bool fts_debug; /* A set of dev/ino pairs. */ static struct di_set *di_set; -/* Define a class for collecting directory information. */ +/* Keep track of the preceding "level" (depth in hierarchy) + from one call of process_file to the next. */ +static size_t prev_level; +/* Define a class for collecting directory information. */ struct duinfo { /* Size of files in directory. */ @@ -399,7 +402,6 @@ process_file (FTS *fts, FTSENT *ent) struct duinfo dui; struct duinfo dui_to_print; size_t level; - static size_t prev_level; static size_t n_alloc; /* First element of the structure contains: The sum of the st_size values of all entries in the single directory @@ -582,10 +584,15 @@ du_files (char **files, int bit_flags) { if (errno != 0) { - /* FIXME: try to give a better message */ - error (0, errno, _("fts_read failed")); + error (0, errno, _("fts_read failed: %s"), + quotearg_colon (fts->fts_path)); ok = false; } + + /* When exiting this loop early, be careful to reset the + global, prev_level, used in process_file. Otherwise, its + (level == prev_level - 1) assertion could fail. */ + prev_level = 0; break; } FTS_CROSS_CHECK (fts); diff --git a/tests/Makefile.am b/tests/Makefile.am index 06d81f0..a5dbd3e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -370,6 +370,7 @@ TESTS = \ du/long-from-unreadable \ du/long-sloop \ du/max-depth \ + du/move-dir-while-traversing \ du/no-deref \ du/no-x \ du/one-file-system \ diff --git a/tests/du/move-dir-while-traversing b/tests/du/move-dir-while-traversing new file mode 100755 index 0000000..e42bc93 --- /dev/null +++ b/tests/du/move-dir-while-traversing @@ -0,0 +1,85 @@ +#!/bin/sh +# Trigger a failed assertion in coreutils-8.9 and earlier. + +# Copyright (C) 2011 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=.}/init.sh"; path_prepend_ ../src +print_ver_ du + +# We use a python-inotify script, so... +python -m pyinotify -h > /dev/null \ + || skip_ 'python-inotify package not installed' + +# Move a directory "up" while du is processing its sub-directories. +# While du is processing a hierarchy .../B/C/D/... this script +# detects when du opens D/, and then moves C/ "up" one level +# so that it is a sibling of B/. +# Given the inherent race condition, we have to add enough "weight" +# under D/ so that in most cases, the monitor performs the single +# rename syscall before du finishes processing the subtree under D/. + +cat <<'EOF' > inotify-watch-for-dir-access.py +#!/usr/bin/env python +from pyinotify import * +dir = sys.argv[1] +dest_parent = os.path.dirname(os.path.dirname(dir)) +dest = os.path.join(dest_parent, os.path.basename(dir)) + +class ProcessDir(ProcessEvent): + + def process_IN_OPEN(self, event): + os.rename(dir, dest) + sys.exit(0) + + def process_default(self, event): + pass + +wm = WatchManager() +notifier = Notifier(wm) +wm.watch_transient_file(dir, IN_OPEN, ProcessDir) +print 'started' +notifier.loop() +EOF +chmod a+x inotify-watch-for-dir-access.py + +long=d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z +t=T/U +mkdir -p $t/3/a/b/c/$long d2 || framework_failure +timeout 6 ./inotify-watch-for-dir-access.py $t/3/a/b > start-msg & + +# Wait for the watcher to start... +nonempty() { test -s start-msg && return 0; sleep $1; } +retry_delay_ nonempty .1 5 + +# The above delay is insufficient in ~50% of my trials. +# Sometimes, when under heavy load, a parallel "make check" would +# fail this test when sleeping just 0.1 seconds here. +sleep 1 + +# The above watches for an IN_OPEN event on $t/3/a/b, +# and when it triggers, moves the parent, $t/3/a, up one level +# so it's directly under $t. + +du -a $t d2 2> err +# Before coreutils-8.10, du would abort. +test $? = 1 || fail=1 + +# check for the new diagnostic +printf "du: fts_read failed: $t/3/a/b: No such file or directory\n" > exp \ + || fail=1 +compare err exp || fail=1 + +Exit $fail -- 1.7.3.5 -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
