Re: [PATCH] Portability: returning void
On Tue, Mar 29, 2011 at 03:02:48PM -0500, Jonathan Nieder wrote:
> Next step is to figure out the longstanding mysterious bash + prove
> hang in t0081.
This patch solves it for me:
diff --git a/t/t0081-line-buffer.sh b/t/t0081-line-buffer.sh
index 1dbe1c9..7ea1317 100755
--- a/t/t0081-line-buffer.sh
+++ b/t/t0081-line-buffer.sh
@@ -50,7 +50,7 @@ long_read_test () {
{
generate_tens_of_lines $tens_of_lines "$line" &&
sleep 100
- } >input &
+ } >input 5>/dev/null &
} &&
test-line-buffer input <<-EOF >output &&
binary $readsize
@@ -84,7 +84,7 @@ test_expect_success PIPE '0-length read, no input available' '
rm -f input &&
mkfifo input &&
{
- sleep 100 >input &
+ sleep 100 >input 5>/dev/null &
} &&
test-line-buffer input <<-\EOF >actual &&
binary 0
@@ -113,7 +113,7 @@ test_expect_success PIPE '1-byte read, no input available' '
printf "%s" a &&
printf "%s" b &&
sleep 100
- } >input &
+ } >input 5>/dev/null &
} &&
test-line-buffer input <<-\EOF >actual &&
binary 1
The problem is that the sleeps hang around for 100 seconds, and they are
connected to the test script's stdout. It works to run "./t0081-*"
because bash sees the SIGCHLD and knows the script is done. But the
prove program actually ignore the SIGCHLD and waits until stdout and
stderr on the child are closed.
I'm not sure why it hangs with bash and not dash. Perhaps it has to do
with one of them using an internal "sleep" and the other not.
Double-weird is that if you "strace" the prove process, it will still
hang. But if you "strace -f", it _won't_ hang. Which makes no sense,
because the only extra thing happening is strace-ing the now-zombie bash
process and the sleeps which are, well, sleeping.
So there is definitely some deep mystery, but I think the fix can be
simpler: we should not leave processes hanging around that might have
descriptors open. Though the above works, I don't like scripts having to
know about the descriptor 5 magic above. The right solution to me is
either:
1. Close descriptor 5 before running test code. But I'm not sure that
will work, since we actually execute the test code in the _current_
shell, not a subshell. Meaning we would be closing it for good.
It also doesn't address descriptor 4, which might also go to
stdout (if "-v" was used). I think the saving grace is that "prove"
is the only thing that cares, and it doesn't use "-v".
2. Tests should kill their backgrounded sleeps themselves. I think I
saw some "kill $!" lines in there, but maybe we are missing one.
-Peff