git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] Portability: returning void

From
Jeff King <peff@peff.net>
Date
Mar 29, 2011, 22:16 UTC
Message-ID
<20110329221652.GB23510@sigill.intra.peff.net>
In-Reply-To
<20110329200230.GA377@elie>
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
Previous: Jonathan NiederNext: Jeff King
Message 3 of 18 in “Portability: returning void”
  1. Portability: returning voidMichael Witten, Mar 29, 2011
  2. Jonathan NiederMar 29, 2011
  3. Jeff KingMar 29, 2011
  4. Jeff KingMar 29, 2011
  5. Jonathan NiederMar 29, 2011
  6. Jeff KingMar 30, 2011
  7. Jonathan NiederMar 30, 2011
  8. Jeff KingMar 30, 2011
  9. Jonathan NiederMar 30, 2011
  10. Jeff KingMar 30, 2011
  11. Johannes SixtMar 30, 2011
  12. tests: introduce helper to fill a pipe in the backgroundJonathan Nieder, Mar 30, 2011
  13. Jonathan NiederMar 30, 2011
  14. Jeff KingMar 30, 2011
  15. Jonathan NiederMar 30, 2011
  16. [PULL svn-fe] Re: Portability: returning voidJonathan Nieder, Mar 30, 2011
  17. Junio C HamanoMar 30, 2011
  18. Jonathan NiederMar 30, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.