[PATCH v2 2/2] t9300-fast-import: don't hang if background fast-import exits too early
- From
SZEDER Gábor <szeder.dev@gmail.com>
- Date
- Dec 6, 2019, 19:03 UTC
- Message-ID
- <20191206190331.29443-3-szeder.dev@gmail.com>
- 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