[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug #34976] find: Failed to save working directory in order to [...
From: |
Bernhard Voelker |
Subject: |
Re: [bug #34976] find: Failed to save working directory in order to [...]: Too many open files |
Date: |
Sun, 03 Feb 2013 20:27:10 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130105 Thunderbird/17.0.2 |
On 01/07/2013 07:27 PM, Bernhard Voelker wrote:
> On 01/07/2013 05:20 PM, Aaron Burgemeister wrote:
>> <http://savannah.gnu.org/bugs/?34976>
>
> The following fixes the issue. Unfortunately, the test-suite is not
> very comprehensive, so I can't tell 100% if it is free of side effects.
> Comments?
Here comes v2 of the fix which includes a test for the failure.
It is also rebased against latest Git master.
Have a nice day,
Berny
>From 3d6cf9278aa122142fe12f236c28a8685e38d800 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <address@hidden>
Date: Sun, 3 Feb 2013 20:26:06 +0100
Subject: [PATCH] find: fix fd leak with --execdir option (bug#34976)
Prevent "Failed to save working dir[...]: Too many open files"
error by closing the file descriptor of the working directory.
* find/exec.c (impl_pred_exec): Free the working directory if find
executes the command in the local dir, i.e. if it has been saved
by record_exec_dir(). Re-indent code.
* find/testsuite/sv-34976-execdir-fd-leak.sh: Add test.
* find/testsuite/Makefile.am (test_shell_progs): Mention the test.
* NEWS: Mention the fix.
---
NEWS | 7 ++
find/exec.c | 94 ++++++++++++++--------------
find/testsuite/Makefile.am | 9 ++-
find/testsuite/sv-34976-execdir-fd-leak.sh | 82 ++++++++++++++++++++++++
4 files changed, 144 insertions(+), 48 deletions(-)
create mode 100755 find/testsuite/sv-34976-execdir-fd-leak.sh
diff --git a/NEWS b/NEWS
index 184722a..526baa4 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,12 @@
GNU findutils NEWS - User visible changes. -*- outline -*- (allout)
+* Major changes in release 4.5.??, ????-??-??
+
+** Bug Fixes
+
+#34976: find -execdir leaks file descriptors for the working directory
+
+
* Major changes in release 4.5.11, 2013-02-02
** Documentation Changes
diff --git a/find/exec.c b/find/exec.c
index aa69fe3..a191d36 100644
--- a/find/exec.c
+++ b/find/exec.c
@@ -112,8 +112,8 @@ record_exec_dir (struct exec_val *execp)
bool
impl_pred_exec (const char *pathname,
- struct stat *stat_buf,
- struct predicate *pred_ptr)
+ struct stat *stat_buf,
+ struct predicate *pred_ptr)
{
struct exec_val *execp = &pred_ptr->args.exec_vec;
char *buf = NULL;
@@ -127,36 +127,36 @@ impl_pred_exec (const char *pathname,
if (local)
{
/* For -execdir/-okdir predicates, the parser did not fill in
- the wd_for_exec member of sturct exec_val. So for those
- predicates, we do so now.
+ the wd_for_exec member of sturct exec_val. So for those
+ predicates, we do so now.
*/
if (!record_exec_dir (execp))
- {
- error (EXIT_FAILURE, errno,
- _("Failed to save working directory in order to "
- "run a command on %s"),
- safely_quote_err_filename (0, pathname));
- /*NOTREACHED*/
- }
+ {
+ error (EXIT_FAILURE, errno,
+ _("Failed to save working directory in order to "
+ "run a command on %s"),
+ safely_quote_err_filename (0, pathname));
+ /*NOTREACHED*/
+ }
target = buf = base_name (state.rel_pathname);
if ('/' == target[0])
- {
- /* find / execdir ls -d {} \; */
- prefix = NULL;
- pfxlen = 0;
- }
+ {
+ /* find / execdir ls -d {} \; */
+ prefix = NULL;
+ pfxlen = 0;
+ }
else
- {
- prefix = "./";
- pfxlen = 2u;
- }
+ {
+ prefix = "./";
+ pfxlen = 2u;
+ }
}
else
{
/* For the others (-exec, -ok), the parser should
- have set wd_for_exec to initial_wd, indicating
- that the exec should take place from find's initial
- working directory.
+ have set wd_for_exec to initial_wd, indicating
+ that the exec should take place from find's initial
+ working directory.
*/
assert (execp->wd_for_exec == initial_wd);
target = pathname;
@@ -171,14 +171,14 @@ impl_pred_exec (const char *pathname,
* depending on the command line length limits.
*/
bc_push_arg (&execp->ctl,
- &execp->state,
- target, strlen (target)+1,
- prefix, pfxlen,
- 0);
+ &execp->state,
+ target, strlen (target)+1,
+ prefix, pfxlen,
+ 0);
/* remember that there are pending execdirs. */
if (execp->state.todo)
- state.execdirs_outstanding = true;
+ state.execdirs_outstanding = true;
/* POSIX: If the primary expression is punctuated by a plus
* sign, the primary shall always evaluate as true
@@ -190,29 +190,31 @@ impl_pred_exec (const char *pathname,
int i;
for (i=0; i<execp->num_args; ++i)
- {
- bc_do_insert (&execp->ctl,
- &execp->state,
- execp->replace_vec[i],
- strlen (execp->replace_vec[i]),
- prefix, pfxlen,
- target, strlen (target),
- 0);
- }
+ {
+ bc_do_insert (&execp->ctl,
+ &execp->state,
+ execp->replace_vec[i],
+ strlen (execp->replace_vec[i]),
+ prefix, pfxlen,
+ target, strlen (target),
+ 0);
+ }
/* Actually invoke the command. */
bc_do_exec (&execp->ctl, &execp->state);
if (WIFEXITED(execp->last_child_status))
- {
- if (0 == WEXITSTATUS(execp->last_child_status))
- result = true; /* The child succeeded. */
- else
- result = false;
- }
+ {
+ if (0 == WEXITSTATUS(execp->last_child_status))
+ result = true; /* The child succeeded. */
+ else
+ result = false;
+ }
else
- {
- result = false;
- }
+ {
+ result = false;
+ }
+ if (local)
+ free_cwd (execp->wd_for_exec);
}
if (buf)
{
diff --git a/find/testsuite/Makefile.am b/find/testsuite/Makefile.am
index a1e49b8..71c82e3 100644
--- a/find/testsuite/Makefile.am
+++ b/find/testsuite/Makefile.am
@@ -246,8 +246,13 @@ find.posix/user-missing.exp
EXTRA_DIST_GOLDEN = \
test_escapechars.golden
-test_shell_progs = sv-bug-32043.sh test_escapechars.sh test_escape_c.sh \
- test_inode.sh sv-34079.sh
+test_shell_progs = \
+sv-bug-32043.sh \
+test_escapechars.sh \
+test_escape_c.sh \
+test_inode.sh \
+sv-34079.sh \
+sv-34976-execdir-fd-leak.sh
EXTRA_DIST = $(EXTRA_DIST_EXP) $(EXTRA_DIST_XO) $(EXTRA_DIST_GOLDEN) \
$(test_shell_progs) binary_locations.sh
diff --git a/find/testsuite/sv-34976-execdir-fd-leak.sh
b/find/testsuite/sv-34976-execdir-fd-leak.sh
new file mode 100755
index 0000000..f22ef80
--- /dev/null
+++ b/find/testsuite/sv-34976-execdir-fd-leak.sh
@@ -0,0 +1,82 @@
+#! /bin/sh
+# Copyright (C) 2013 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/>.
+#
+
+# This test verifies that find does not leak a file descriptor for the working
+# directory specified by the -execdir option [Savannah bug #34976].
+
+testname="$(basename $0)"
+
+. "${srcdir}"/binary_locations.sh
+
+# Test if restricting the number of file descriptors via ulimit -n works.
+test_ulimit() {
+ n="$1" # number of files
+ l="$2" # limit to use
+ (
+ ulimit -n "$l" || exit 1
+ for i in $(seq $n) ; do
+ printf "exec %d> /dev/null || exit 1\n" $i
+ done | sh ;
+ ) 2>/dev/null
+}
+# Opening 30 files with a limit of 40 should work.
+test_ulimit 30 40 || { echo "SKIP: ulimit does not work" >&2; exit 0 ; }
+# Opening 30 files with a limit of 20 should fail.
+test_ulimit 30 20 && { echo "SKIP: ulimit does not work" >&2; exit 0 ; }
+
+die() {
+ echo "$@" >&2
+ exit 1
+}
+
+# Create test files, each 100 in the directories ".", "one" and "two".
+make_test_data() {
+ d="$1"
+ (
+ cd "$1" || exit 1
+ {
+ for j in $(seq 0 100) ; do
+ printf "%03d " $i
+ done
+ for i in one two ; do
+ mkdir $i
+ for j in $(seq 0 100) ; do
+ printf "%s/%03d " $i $j
+ done
+ done
+ } | xargs sh -c 'touch "$@" || exit 255' fnord || exit 1
+ ) \
+ || die "failed to set up the test in ${outdir}"
+}
+
+outdir="$(mktemp -d)" || die "FAIL: could not create a test files."
+
+# Create some test files.
+make_test_data "${outdir}" || die "FAIL: failed to set up the test in
${outdir}"
+
+fail=0
+for exe in "${ftsfind}" "${oldfind}"; do
+ ( ulimit -n 30 && \
+ ${exe} "${outdir}" -type f -execdir cat '{}' \; >/dev/null; ) \
+ || { \
+ echo "Option -execdir of ${exe} leaks file descriptors" >&2 ; \
+ fail=1 ; \
+ }
+done
+
+rm -rf "${outdir}" || exit 1
+exit $fail
--
1.7.7
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [bug #34976] find: Failed to save working directory in order to [...]: Too many open files,
Bernhard Voelker <=