All of lore.kernel.org
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>
Subject: [PATCH v2 2/2] t9300-fast-import: don't hang if background fast-import exits too early
Date: Fri,  6 Dec 2019 20:03:31 +0100	[thread overview]
Message-ID: <20191206190331.29443-3-szeder.dev@gmail.com> (raw)
In-Reply-To: <20191206190331.29443-1-szeder.dev@gmail.com>

The five tests checking 'git fast-import's checkpoint handling in
't9300-fast-import.sh', all with the prefix "V:" in their test
description, can hang indefinitely if 'git fast-import' unexpectedly
dies early in any of these tests.

These five tests run 'git fast-import' in the background, while
feeding instructions to its standard input through a fifo (fd 8) from
a background subshell, and reading and verifying its standard output
through another fifo (fd 9) in the test script's main shell process.
This "reading and verifying" is basically a 'while read ...' shell
loop iterating until 'git fast-import' outputs the expected line,
ignoring any other output.  This doesn't work very well when 'git
fast-import' dies before printing that particular line, because the
'read' builtin doesn't get EOF after the death of 'git fast-import',
as their input and output are not connected directly but through a
fifo.  Consequently, that 'read' hangs waiting for the next line from
the already dead 'git fast-import', leaving the test script and in
turn the whole test suite hanging.

Avoid this hang by checking whether the background 'git fast-import'
process exited unexpectedly early, and interrupt the 'while read' loop
if it did.  We have to jump through some hoops to achive that, though:

  - Start the background 'git fast-import' in another background
    subshell, which then:

      - prints the PID of that 'git fast-import' process to the fifo,
	to be read by the main shell process, so it will know which
	process to kill when the test is finished.

      - waits until that 'git fast-import' process exits.  If it does
	exit, then report its exit code, and write a message to the
	fifo used for 'git fast-import's standard output, thus
	un-block the 'read' builtin in the main shell process.

  - Modify that 'while read' loop to break the loop upon seeing that
    message, and fail the test in the usual way.

  - Once the test is finished kill that background subshell as well,
    and do so before killing the background 'git fast-import'.
    Otherwise the background 'git fast-import' and subshell processes
    would die racily, and if 'git fast-import' were to die sooner,
    then we might get some undesired and potentially confusing
    messages in the test's output.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 t/t9300-fast-import.sh | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 6820ebbb63..8f6f80f021 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -3164,12 +3164,21 @@ background_import_then_checkpoint () {
 	exec 9<>V.output
 	rm V.output
 
-	git fast-import $options <&8 >&9 &
-	fi_pid=$!
+	(
+		git fast-import $options <&8 >&9 &
+		echo $! >&9
+		wait $!
+		echo >&2 "background fast-import terminated too early with exit code $?"
+		# Un-block the read loop in the main shell process.
+		echo >&9 UNEXPECTED
+	) &
+	sh_pid=$!
+	read fi_pid <&9
 	# We don't mind if fast-import has already died by the time the test
 	# ends.
 	test_when_finished "
 		exec 8>&-; exec 9>&-;
+		kill $sh_pid && wait $sh_pid
 		kill $fi_pid && wait $fi_pid
 		true"
 
@@ -3190,6 +3199,9 @@ background_import_then_checkpoint () {
 		then
 			error=0
 			break
+		elif test "$output" = "UNEXPECTED"
+		then
+			break
 		fi
 		# otherwise ignore cruft
 		echo >&2 "cruft: $output"
-- 
2.24.0.801.g241c134b8d


      parent reply	other threads:[~2019-12-06 19:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-30 10:46 [PATCH] t9300-fast-import: don't hang if background fast-import exits too early SZEDER Gábor
2019-11-30 21:16 ` Junio C Hamano
2019-12-01  9:31   ` SZEDER Gábor
2019-12-01 15:38     ` Junio C Hamano
2019-12-06 19:03 ` [PATCH v2 0/2] " SZEDER Gábor
2019-12-06 19:03   ` [PATCH v2 1/2] t9300-fast-import: store the PID in a variable instead of pidfile SZEDER Gábor
2019-12-06 19:03   ` SZEDER Gábor [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191206190331.29443-3-szeder.dev@gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.