[Libguestfs] [nbdkit PATCH v2 16/17] sh: Test for fd leaks

Eric Blake eblake at redhat.com
Fri Aug 2 19:26:17 UTC 2019


Various patches before this one plugged fd leaks of files opened
within nbdkit that the shell script need not see.  It's time to add
testsuite coverage to ensure we don't regress.

However, a user may intentionally want to expose an open fd to nbdkit,
which in turn remains available all the way through to the shell
script, so nbdkit does not close unrecognized inherited fds at
startup.  This makes the test more interesting - if you run 'exec
17>/dev/null && make check', then there will be one more inherited fd
in the environment of both nbdkit and the shell script, but this is
not an nbdkit leak.  Furthermore, checking how many fds are open in a
shell is tricky: many actions in the shell (such as a glob or
pipeline) increase the number of temporarily-open file descriptors; I
settled on creating a subshell, then using ls to check the files still
open in $$ (the subshell prevents the parent from using any more fds,
all without changing $$).

This addition is rather fragile - if it causes testsuite failures in
some environments, we may end up reverting it.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 tests/test-parallel-sh.sh | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tests/test-parallel-sh.sh b/tests/test-parallel-sh.sh
index 21bc010a..ddbdf90c 100755
--- a/tests/test-parallel-sh.sh
+++ b/tests/test-parallel-sh.sh
@@ -53,12 +53,35 @@ qemu-io -f raw -c "aio_write -P 1 0 512" -c "aio_write -P 2 512 512" \
 # happens). This test may have spurious failures under heavy loads on
 # the test machine, where tuning the delays may help.

+# Also test for leaked fds when possible: nbdkit does not close fds it
+# inherited, but other than 0, 1, 2, and the fd associated with the
+# script, the child shell should not see any new fds not also present
+# in nbdkit's parent environment.  When testing for the count of open
+# fds, use ls in a subshell (rather than a glob directly within the
+# shell under test), to avoid yet another fd open on the /proc/self/fd
+# directory.
+
+curr_fds=
+if test -d /proc/$$/fd; then
+    curr_fds=$(/usr/bin/env bash -c '(ls /proc/$$/fd)' | wc -w)
+fi
+echo "using curr_fds=$curr_fds"
+
 cat > test-parallel-sh.script <<EOF
 #!/usr/bin/env bash
 f=test-parallel-sh.data
 if ! test -f \$f; then
   echo "can't locate test-parallel-sh.data" >&2; exit 5
 fi
+if test -d /proc/\$\$/fd; then
+  (
+    if test \$( ls /proc/\$\$/fd | wc -w ) -ne \$(($curr_fds + 1)); then
+      echo "there seem to be leaked fds, curr_fds=$curr_fds" >&2
+      ls -l /proc/\$\$/fd >&2
+      exit 1
+    fi
+  ) || exit 5
+fi
 case \$1 in
   get_size) stat -L -c %s \$f || exit 1 ;;
   pread) dd iflag=skip_bytes,count_bytes skip=\$4 count=\$3 if=\$f || exit 1 ;;
-- 
2.20.1




More information about the Libguestfs mailing list