From: Phillip Wood Date: Fri, 11 Aug 2023 09:56:40 GMT Subject: Re: [PATCH] t4053: avoid race when killing background processes Message-ID: <2d9e9b59-bf78-8470-0731-3cd7fe15a727@gmail.com> In-Reply-To: On 10/08/2023 18:40, Junio C Hamano wrote: > "Phillip Wood via GitGitGadget" writes: > > While I can see that "kill" in the when-finished handler may or may > not find the backgrounded process by the time it is run, and > ignoring its exit status (hence keeping test_when_finished happy) > would be a reasonable thing to do. I can understand if this patch > is to fix a different symptom, namely, when-finished handler > sometimes fails and makes the test fail. > > But I am not sure how this causes the test to "hang", This is only a fix for the test failure that Peff saw when running with --stress. > which > presumably is a symptom that somebody is trying to read from > a pipe that nobody is making progress to write into? That or a process blocking when tying to open a fifo seems the mostly likely cause but I can't see where that is happening. As you say this patch does not obviously change anything that would be causing the test to hang. Best Wishes Phillip > We will > send a signal either way to the writers, and the only difference is > that we ignore the exit code. > > Granted, when-finished handlers are concatenated with "&&-", and one > "kill"s failure will cause the other "kill" not to run, so we may > send a signal to only one but not to the other, but that should all > happen after "diff --no-index" returns, so it still does not explain > the "hang". > > Puzzled... >> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1571%2Fphillipwood%2Fdiff-no-index-pipes-fixes-v1 >> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1571/phillipwood/diff-no-index-pipes-fixes-v1 >> Pull-Request: https://github.com/gitgitgadget/git/pull/1571 >> >> t/t4053-diff-no-index.sh | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh >> index a28b9ff2434..1fb7d334620 100755 >> --- a/t/t4053-diff-no-index.sh >> +++ b/t/t4053-diff-no-index.sh >> @@ -248,11 +248,11 @@ test_expect_success PIPE,SYMLINKS 'diff --no-index reads from pipes' ' >> { >> (test_write_lines a b c >old) & >> } && >> - test_when_finished "! kill $!" && >> + test_when_finished "kill $! || :" && >> { >> (test_write_lines a x c >new) & >> } && >> - test_when_finished "! kill $!" && >> + test_when_finished "kill $! || :" && >> >> cat >expect <<-EOF && >> diff --git a/old b/new-link >> >> base-commit: a82fb66fed250e16d3010c75404503bea3f0ab61