From 21f57620b2e73d20f5c98006af756e0162eaee3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Fri, 6 Dec 2019 20:03:30 +0100 Subject: [PATCH 1/2] t9300-fast-import: store the PID in a variable instead of pidfile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The five tests running 'git fast-import' in the background in 't9300-fast-import.sh' store the PID of that background process in a pidfile, to be used to check whether that background process survived each test and then to kill it in test_when_finished commands. To achieve this all these five tests run three $(cat ) command substitutions each. Store the PID of the background 'git fast-import' in a variable to avoid those extra processes. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- t/t9300-fast-import.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index e707fb861e..6820ebbb63 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -3165,12 +3165,12 @@ background_import_then_checkpoint () { rm V.output git fast-import $options <&8 >&9 & - echo $! >V.pid + fi_pid=$! # We don't mind if fast-import has already died by the time the test # ends. test_when_finished " exec 8>&-; exec 9>&-; - kill $(cat V.pid) && wait $(cat V.pid) + kill $fi_pid && wait $fi_pid true" # Start in the background to ensure we adhere strictly to (blocking) @@ -3202,7 +3202,7 @@ background_import_then_checkpoint () { } background_import_still_running () { - if ! kill -0 "$(cat V.pid)" + if ! kill -0 "$fi_pid" then echo >&2 "background fast-import terminated too early" false From 0d9b0d78859b5a6d91d57609305b1c6eb326bf7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Fri, 6 Dec 2019 20:03:31 +0100 Subject: [PATCH 2/2] t9300-fast-import: don't hang if background fast-import exits too early MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- 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"