Re: [PATCH] Portability: returning void

On Tue, Mar 29, 2011 at 07:29:21PM -0500, Jonathan Nieder wrote:

Agreed, it's a horrible fix. I just wanted to make sure it fixed it for you, too. After thinking on it more, I figured out an elegant fix. Patch is below.

-- >8 --
Subject: [PATCH] t0081: kill backgrounded processes more robustly

This test creates several background processes that write to a fifo and then go to sleep for a while (so the reader of the fifo does not see EOF).

Each background process is made in a curly-braced block in the shell, and after we are done reading from the fifo, we use "kill $!" to kill it off.

For a simple, single-command process, this works reliably and kills the child sleep process. But for more complex commands like "make_some_output && sleep", the results are less predictable. When executing under bash, we end up with a subshell that gets killed by the $! but leaves the sleep process still alive.

This is bad not only for process hygeine (we are leaving random sleep processes to expire after a while), but also interacts badly with the "prove" command. When prove executes a test, it does not realize the test is done when it sees SIGCHLD, but rather waits until the test's stdout pipe is closed. The orphaned sleep process may keep that pipe open via test-lib's file descriptor 5, causing prove to hang for 100 seconds.

The solution is to explicitly use a subshell and to exec the final sleep process, so that when we "kill $!" we get the process id of the sleep process.

While we're at it, let's make the "kill" process a little more robust. Specifically:

  1. Wrap the "kill" in a test_when_finished, since we want
     to clean up the process whether the test succeeds or not.

  2. The "kill" is part of our && chain for test success. It
     probably won't fail, but it can if the process has
     expired before we manage to kill it. So let's mark it
     as OK to fail.

Signed-off-by: Jeff King <peff@peff.net>
---
Actually, my (2) above is unlikely to trigger, since the test would have
to hang for 100 seconds first, which probably means it is failing
anyway. But I did run across it when I screwed up my fix.

Also, is it just me, or does it seem weird that test_when_finished blocks failing can produce test failure? I would think you would be able to put any old cleanup crap into them and not care whether it worked or not.

 t/t0081-line-buffer.sh |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)