Re: [PATCH] update-ref: fix streaming of status updates
From: Jeff King <hidden>
Date: 2021-09-15 17:13:27
quoted hunk ↗ jump to hunk
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 4506cd435b..1e754e258f 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh@@ -1598,6 +1598,38 @@ test_expect_success 'transaction cannot restart ongoing transaction' ' test_must_fail git show-ref --verify refs/heads/restart ' +test_expect_success PIPE 'transaction flushes status updates' ' + mkfifo in out && + (git update-ref --stdin <in >out &) && + + exec 9>in && + test_when_finished "exec 9>&-" && + + echo "start" >&9 && + echo "start: ok" >expected && + read line <out && + echo "$line" >actual && + test_cmp expected actual && + + echo "create refs/heads/flush $A" >&9 && + + echo prepare >&9 && + echo "prepare: ok" >expected && + read line <out && + echo "$line" >actual && + test_cmp expected actual &&
I think this test may be racy. I saw a strange failure from it in CI: https://github.com/peff/git/runs/3605506649?check_suite_focus=true#step:5:6734 I can't reproduce the problem locally with "--stress", but the failure there is on macOS (and likewise, a nearby run failed with a timeout just for macOS, which could be caused by a racy deadlock). I'm guessing the problem may be the continued opening and closing of "out" by the read calls from the shell. The update-ref process may get SIGPIPE as a result, depending on the write timing. The solution is to use open a descriptor for "out" like we do for "in", and then read from that. See 4783e7ea83 (t0008: avoid SIGPIPE race condition on fifo, 2013-07-12) for some prior art. -Peff