Re: [PATCH] Portability: returning void

[Jeff King <peff@peff.net>]

On Tue, Mar 29, 2011 at 06:16:52PM -0400, Jeff King wrote:

> 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.
>
> [...]
> 
>   2. Tests should kill their backgrounded sleeps themselves. I think I
>      saw some "kill $!" lines in there, but maybe we are missing one.

Indeed, those kill lines aren't doing what you expect. Try instrumenting
like this:

diff --git a/t/t0081-line-buffer.sh b/t/t0081-line-buffer.sh
index 1dbe1c9..3b2f8ce 100755
--- a/t/t0081-line-buffer.sh
+++ b/t/t0081-line-buffer.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 test_description="Test the svn importer's input handling routines.
 
@@ -56,6 +56,7 @@ long_read_test () {
 	binary $readsize
 	copy $copysize
 	EOF
+	echo >>/tmp/foo killing $!
 	kill $! &&
 	test_line_count = $lines output &&
 	tail -n 1 <output >actual &&
@@ -90,6 +91,7 @@ test_expect_success PIPE '0-length read, no input available' '
 	binary 0
 	copy 0
 	EOF
+	echo >>/tmp/foo killing $!
 	kill $! &&
 	test_cmp expect actual
 '
@@ -119,6 +121,7 @@ test_expect_success PIPE '1-byte read, no input available' '
 	binary 1
 	copy 1
 	EOF
+	echo >>/tmp/foo killing $!
 	kill $! &&
 	test_cmp expect actual
 '

And then run the test under prove, and check "ps" for remaining sleep
processes. You will see that the killed process IDs do not match up with
the sleep processes. Except for one sleep process, which actually got
killed. That is the second one in the list above. It works because it's:

  {
    sleep 100 >input &
  } &&
  ...
  kill $!

whereas the other ones are like:

  {
    do_something &&
    sleep 100
  } >input &

So my guess is that we have to start a subshell for the latter ones, and
_that_ is what gets killed. And that may explain the bash vs dash
behavior; dash, upon receiving the kill signal, presumably kills its
child, but bash does not (I didn't confirm that; it's just a theory).

I'm not sure what the best solution is. Perhaps putting the subshell
into its own process group and killing the whole PGRP?

-Peff