Fix hang in t5562, introduced in v2.21.0-rc1

From: Johannes Schindelin <[email protected]>

In cc95bc2025 (t5562: replace /dev/zero with a pipe from generate_zero_bytes, 2019-02-09), we replaced usage of /dev/zero (which is not available on NonStop, apparently) by a Perl script snippet to generate NUL bytes.

Sadly, it does not seem to work on NonStop, as t5562 reportedly hangs.
...
In the end, though, what counts is that this here change incidentally fixes that hang (maybe also on NonStop?). Even more positively, it gets rid of yet another unnecessary Perl invocation.

To: [email protected]
Cc: Randall Becker <[email protected]>; Junio C Hamano <[email protected]>
Subject: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1

The last-minute patch to replace /dev/zero with a Perl script snippet broke the Linux part of the CI builds on Azure Pipelines: it timed out. The culprit is the rb/no-dev-zero-in-test branch (see the build for this branch here
[https://dev.azure.com/gitgitgadget/git/_build/results?buildId=1727]).

All of master, next, jch and pu are broken that way. You might see it in the commit status of the active branches
[https://github.com/gitgitgadget/git/branches/active].

Turns out that it is that particular Perl script snippet which for some reason hangs the build. If you kill it, t5562.15 succeeds, if you don't kill it, it will hang indefinitely (or until killed).

Sadly, despite my earnest attempts, I could not figure out why it hangs in those Linux agents (I could not reproduce that hang locally), or for that matter, why it does not hang in the Windows and macOS agents.

Let's avoid that hang. This patch fixes things on Azure Pipelines, and my hope is that it also fixes the hang on NonStop.

Johannes Schindelin (1):
tests: teach the test-tool to generate NUL bytes and use it

 Makefile                               |  1 +
 t/helper/test-genzeros.c               | 22 ++++++++++++++++++++++
 t/helper/test-tool.c                   |  1 +
 t/helper/test-tool.h                   |  1 +
 t/t5562-http-backend-content-length.sh |  2 +-
 t/test-lib-functions.sh                |  8 +-------
 6 files changed, 27 insertions(+), 8 deletions(-)  create mode 100644

t/helper/test-genzeros.c

base-commit: 8989e1950a845ceeb186d490321a4f917ca4de47
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-
126%2Fdscho%2Ffix-t5562-hang-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-126/dscho/fix- t5562-hang-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/126

Unfortunately, subtest 13 still hangs on NonStop, even with this patch, so our Pipeline still hangs. I'm glad it's better on Azure, but I don't think this actually addresses the root cause of the hang.

possible this is not the test that is failing, but actually the git-http-backend? The code is not in a loop, if that helps. It is not consuming any significant cycles. I don't know that part of the code at all, sadly. The code is here:

* in the operating system from here up *
cleanup_children + 0x5D0 (UCr)
cleanup_children_on_exit + 0x70 (UCr)
git_atexit_dispatch + 0x200 (UCr)
__process_atexit_functions + 0xA0 (DLL zcredll)
CRE_TERMINATOR_ + 0xB50 (DLL zcredll)
exit + 0x2A0 (DLL zcrtldll)
die_webcgi + 0x240 (UCr)
die_errno + 0x360 (UCr)
write_or_die + 0x1C0 (UCr)
end_headers + 0x1A0 (UCr)
die_webcgi + 0x220 (UCr)
die + 0x320 (UCr)
inflate_request + 0x520 (UCr)
run_service + 0xC20 (UCr)
service_rpc + 0x530 (UCr)
cmd_main + 0xD00 (UCr)
main + 0x190 (UCr)

Best guess is that a signal (SIGCHLD?) is possibly getting eaten or neglected somewhere between the test, perl, and git-http-backend.

Unfortunately, subtest 13 still hangs on NonStop, even with this patch, so our Pipeline still hangs. I'm glad it's better on Azure, but I don't think this actually addresses the root cause of the hang. This is now the fourth attempt at fixing this. Is it possible this is not the test that is failing, but actually the git-http-backend? The code is not in a loop, if that helps. It is not consuming any significant cycles. I don't know that part of the code at all, sadly. The code is here:

* in the operating system from here up *
cleanup_children + 0x5D0 (UCr)

Quoted 9 lines · Randall S. Becker

Unfortunately, subtest 13 still hangs on NonStop, even with this patch, so our Pipeline still hangs. I'm glad it's better on Azure, but I don't think this actually addresses the root cause of the hang. This is now the fourth attempt at fixing this. Is it possible this is not the test that is failing, but actually the git-http-backend? The code is not in a loop, if that helps. It is not consuming any significant cycles. I don't know that part of the code at all, sadly. The code is here:

Quoted 2 lines

* in the operating system from here up *
cleanup_children + 0x5D0 (UCr)

... so does the process which the stack was taken from has any children processes still?

I could imagine if a child somehow manages to end up in uninterruptible sleep, then probably it would never complete this way, wouldn't it?

To: Randall S. Becker <[email protected]>
Cc: 'Johannes Schindelin via GitGitGadget' <[email protected]>; [email protected]; 'Max Kirillov' <[email protected]>
Subject: Re: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1

Quoted 3 lines · Randall S. Becker

Unfortunately, subtest 13 still hangs on NonStop, even with this patch, so our Pipeline still hangs. I'm glad it's better on Azure, but I don't think this actually addresses the root cause of the hang.

Sigh.

Quoted 24 lines

possible this is not the test that is failing, but actually the git-http-backend? The code is not in a loop, if that helps. It is not consuming any significant cycles. I don't know that part of the code at all, sadly. The code is here:

* in the operating system from here up *
cleanup_children + 0x5D0 (UCr)
cleanup_children_on_exit + 0x70 (UCr)
git_atexit_dispatch + 0x200 (UCr)
__process_atexit_functions + 0xA0 (DLL zcredll)
CRE_TERMINATOR_ + 0xB50 (DLL zcredll)
exit + 0x2A0 (DLL zcrtldll)
die_webcgi + 0x240 (UCr)
die_errno + 0x360 (UCr)
write_or_die + 0x1C0 (UCr)
end_headers + 0x1A0 (UCr)
die_webcgi + 0x220 (UCr)
die + 0x320 (UCr)
inflate_request + 0x520 (UCr)
run_service + 0xC20 (UCr)
service_rpc + 0x530 (UCr)
cmd_main + 0xD00 (UCr)
main + 0x190 (UCr)

Quoted 2 lines

Best guess is that a signal (SIGCHLD?) is possibly getting eaten or neglected somewhere between the test, perl, and git-http-backend.

So we are trying to die(), which actually happens in die_webcgi(), and

to write some message _but_ notice an error inside write_or_dir() and try to exit because we do not want to recurse forever trying to die, giving a message to say how/why we died, and die because failing to give that message, forever.

But in our attempt to exit(), we try to "cleanup children" and that is

stuck.

One big difference before and after the /dev/zero change is that the

is now on a downstream of the pipe. If we prepare a large file with a

size full of NULs and replace /dev/null with it, instead of feeding NULs

the pipe, would it change the equation?

To: Randall S. Becker <[email protected]>
Cc: 'Johannes Schindelin via GitGitGadget' <[email protected]>; [email protected]; 'Junio C Hamano' <[email protected]>; 'Max Kirillov' <[email protected]>
Subject: Re: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1

Quoted 9 lines · Randall S. Becker

Unfortunately, subtest 13 still hangs on NonStop, even with this patch, so our Pipeline still hangs. I'm glad it's better on Azure, but I don't think this actually addresses the root cause of the hang. This is now the fourth attempt at fixing this. Is it possible this is not the test that is failing, but actually the git-http-backend? The code is not in a loop, if that helps. It is not consuming any significant cycles. I don't know that part of the code at all, sadly. The code is here:

Quoted 2 lines

* in the operating system from here up *
cleanup_children + 0x5D0 (UCr)

... so does the process which the stack was taken from has any children processes still?

I could imagine if a child somehow manages to end up in uninterruptible sleep, then probably it would never complete this way, wouldn't it?

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 7 lines · writes:

From: Johannes Schindelin <[email protected]>

In cc95bc2025 (t5562: replace /dev/zero with a pipe from generate_zero_bytes, 2019-02-09), we replaced usage of /dev/zero (which is not available on NonStop, apparently) by a Perl script snippet to generate NUL bytes.

Quoted 5 lines

Sadly, it does not seem to work on NonStop, as t5562 reportedly hangs.
...
In the end, though, what counts is that this here change incidentally fixes that hang (maybe also on NonStop?). Even more positively, it gets rid of yet another unnecessary Perl invocation.

Thanks for a quick band-aid.

Will apply directly to 'master' so that we won't forget before -rc2.

In the meantime, perhaps somebody who knows Perl interpreter's quirks well can tell us what's different between the obvious and simple C program and an equivalent in Perl to convince us why this is a good solution to the problem.

Quoted 16 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 12 lines · writes:

From: Johannes Schindelin <[email protected]>

In cc95bc2025 (t5562: replace /dev/zero with a pipe from generate_zero_bytes, 2019-02-09), we replaced usage of /dev/zero (which is not available on NonStop, apparently) by a Perl script snippet to generate NUL bytes.

Sadly, it does not seem to work on NonStop, as t5562 reportedly hangs.
...
In the end, though, what counts is that this here change incidentally fixes that hang (maybe also on NonStop?). Even more positively, it gets rid of yet another unnecessary Perl invocation.

Quoted 3 lines

Thanks for a quick band-aid.

Will apply directly to 'master' so that we won't forget before -rc2.

Thank you, that will be good, as the builds still seem to fail. All of them.

Quoted 22 lines · Johannes Schindelin

Quoted 19 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 7 lines · writes:

From: Johannes Schindelin <[email protected]>

In cc95bc2025 (t5562: replace /dev/zero with a pipe from generate_zero_bytes, 2019-02-09), we replaced usage of /dev/zero (which is not available on NonStop, apparently) by a Perl script snippet to generate NUL bytes.

Quoted 5 lines

Sadly, it does not seem to work on NonStop, as t5562 reportedly hangs.
...
In the end, though, what counts is that this here change incidentally fixes that hang (maybe also on NonStop?). Even more positively, it gets rid of yet another unnecessary Perl invocation.

Thanks for a quick band-aid.

Will apply directly to 'master' so that we won't forget before -rc2.

Quoted 2 lines

Thank you, that will be good, as the builds still seem to fail. All of them.

Actually, I am really tempted to instead not apply this, but revert that genzerobytes Perl thing. This assumes that your Azure thing did not have the breakage before we applied that patchset. What do you think?

Trying four or more possible band-aids that may or may not work without knowing what the real cause of the hangs are is not something I want to see people spend excessive time of theirs on this close to the final. I'd rather avoid distraction and see

people spend their cycles on bugs that matter, instead of trying to chase test breakages that have always been present for those without /dev/zero. I am not fundamentally opposed to supporting those without /dev/zero but I'd prefer to see it happen in 'pu' until we identify and fix the real cause---which may well be a real bug in the http-backend stuff---and the time to do that is not during the rc period where we close the tree for new features and non-regression fixes.

Quoted 37 lines · Junio C Hamano

To: Randall S. Becker <[email protected]>
Cc: 'Johannes Schindelin via GitGitGadget' <[email protected]>; [email protected]; 'Max Kirillov' <[email protected]>
Subject: Re: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1

Quoted 3 lines · Randall S. Becker

Unfortunately, subtest 13 still hangs on NonStop, even with this patch, so our Pipeline still hangs. I'm glad it's better on Azure, but I don't think this actually addresses the root cause of the hang.

Sigh.

Quoted 24 lines

possible this is not the test that is failing, but actually the git-http-backend? The code is not in a loop, if that helps. It is not consuming any significant cycles. I don't know that part of the code at all, sadly. The code is here:

* in the operating system from here up *
cleanup_children + 0x5D0 (UCr)
cleanup_children_on_exit + 0x70 (UCr)
git_atexit_dispatch + 0x200 (UCr)
__process_atexit_functions + 0xA0 (DLL zcredll)
CRE_TERMINATOR_ + 0xB50 (DLL zcredll)
exit + 0x2A0 (DLL zcrtldll)
die_webcgi + 0x240 (UCr)
die_errno + 0x360 (UCr)
write_or_die + 0x1C0 (UCr)
end_headers + 0x1A0 (UCr)
die_webcgi + 0x220 (UCr)
die + 0x320 (UCr)
inflate_request + 0x520 (UCr)
run_service + 0xC20 (UCr)
service_rpc + 0x530 (UCr)
cmd_main + 0xD00 (UCr)
main + 0x190 (UCr)

Quoted 4 lines

Quoted 2 lines

Best guess is that a signal (SIGCHLD?) is possibly getting eaten or neglected somewhere between the test, perl, and git-http-backend.

So we are trying to die(), which actually happens in die_webcgi(), and

then try

Quoted 6 lines

to write some message _but_ notice an error inside write_or_dir() and try to exit because we do not want to recurse forever trying to die, giving a message to say how/why we died, and die because failing to give that message, forever.

But in our attempt to exit(), we try to "cleanup children" and that is

what gets

Quoted 3 lines

stuck.

One big difference before and after the /dev/zero change is that the

process

Quoted 1 line

is now on a downstream of the pipe. If we prepare a large file with a

finite

Quoted 1 line

size full of NULs and replace /dev/null with it, instead of feeding NULs

from

Quoted 1 line

the pipe, would it change the equation?

Doubtful. The processes are still around, and are waiting on read but not actively reading (CPU time is not going up, so we're not reading an infinite stream). To me, this is a pipe situation where there is simply nothing waiting on the pipe (maybe a flush missing?). I'm grasping are straws without knowing the actual process architecture of the test to debug it.

Quoted 7 lines · Randall S. Becker

Quoted 6 lines · Junio C Hamano

To: Randall S. Becker <[email protected]>
Cc: 'Johannes Schindelin via GitGitGadget' <[email protected]>; [email protected]; 'Max Kirillov' <[email protected]>
Subject: Re: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1

Quoted 5 lines

Quoted 5 lines

Quoted 3 lines · Randall S. Becker

Unfortunately, subtest 13 still hangs on NonStop, even with this patch, so our Pipeline still hangs. I'm glad it's better on Azure, but I don't think this actually addresses the root cause of the

Quoted 50 lines

Quoted 27 lines

Sigh.

Quoted 24 lines

possible this is not the test that is failing, but actually the git-http-backend? The code is not in a loop, if that helps. It is not consuming any significant cycles. I don't know that part of the code at all, sadly. The code is here:

* in the operating system from here up *
cleanup_children + 0x5D0 (UCr)
cleanup_children_on_exit + 0x70 (UCr)
git_atexit_dispatch + 0x200 (UCr)
__process_atexit_functions + 0xA0 (DLL zcredll)
CRE_TERMINATOR_ + 0xB50 (DLL zcredll)
exit + 0x2A0 (DLL zcrtldll)
die_webcgi + 0x240 (UCr)
die_errno + 0x360 (UCr)
write_or_die + 0x1C0 (UCr)
end_headers + 0x1A0 (UCr)
die_webcgi + 0x220 (UCr)
die + 0x320 (UCr)
inflate_request + 0x520 (UCr)
run_service + 0xC20 (UCr)
service_rpc + 0x530 (UCr)
cmd_main + 0xD00 (UCr)
main + 0x190 (UCr)

Quoted 5 lines

Quoted 2 lines

Best guess is that a signal (SIGCHLD?) is possibly getting eaten or neglected somewhere between the test, perl, and git-http-backend.

So we are trying to die(), which actually happens in die_webcgi(), and

then try

Quoted 7 lines

to write some message _but_ notice an error inside write_or_dir() and try to exit because we do not want to recurse forever trying to die, giving a message to say how/why we died, and die because failing to give that message, forever.

But in our attempt to exit(), we try to "cleanup children" and that is

what gets

Quoted 3 lines

stuck.

One big difference before and after the /dev/zero change is that the

process

Quoted 2 lines

is now on a downstream of the pipe. If we prepare a large file with a

finite

Quoted 2 lines

size full of NULs and replace /dev/null with it, instead of feeding NULs

Quoted 8 lines

from

Quoted 1 line

the pipe, would it change the equation?

Doubtful. The processes are still around, and are waiting on read but not actively reading (CPU time is not going up, so we're not reading an infinite stream). To me, this is a pipe situation where there is simply nothing waiting on the pipe (maybe a flush missing?). I'm grasping are straws without knowing the actual process architecture of

test to debug it.

So could you try with this patch?

-- snipsnap --

http-backend.c index d5cea0329a..7c1b4a2555 renamed from http-backend.c +0 -0

100644

http-backend.c +0 -0

@@ -427,6 +427,7 @@ static void inflate_request(const char *prog_name,

int out, int buffer_input, ss

done:

Quoted 10 lines · Johannes Schindelin

So could you try with this patch?

-- snipsnap --

http-backend.c index d5cea0329a..7c1b4a2555 renamed from http-backend.c +0 -0

100644

http-backend.c +0 -0

@@ -427,6 +427,7 @@ static void inflate_request(const char *prog_name,

int out, int buffer_input, ss

http-backend.c static void inflate_request(const char *prog_name, int out, int buffer_input, ss · line 433

Quoted 6 lines

done:

http-backend.c +1 -0

 	git_inflate_end(&stream);
+	close(0);
 	close(out);
 	free(full_request);
 }

In isolation or with the other fixes associated with t5562? Or, which baseline commit should I use? 8989e1950a or d92031209a or some other?

Quoted 10 lines · Johannes Schindelin

Quoted 8 lines · Randall S. Becker

Quoted 7 lines · Junio C Hamano

To: Randall S. Becker <[email protected]>
Cc: 'Johannes Schindelin via GitGitGadget' <[email protected]>; [email protected]; 'Max Kirillov' <[email protected]>
Subject: Re: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 4 lines · Randall S. Becker

Unfortunately, subtest 13 still hangs on NonStop, even with this patch, so our Pipeline still hangs. I'm glad it's better on Azure, but I don't think this actually addresses the root cause of the

hang.

Quoted 27 lines

Quoted 27 lines

Quoted 27 lines

Sigh.

Quoted 24 lines

possible this is not the test that is failing, but actually the git-http-backend? The code is not in a loop, if that helps. It is not consuming any significant cycles. I don't know that part of the code at all, sadly. The code is here:

* in the operating system from here up *
cleanup_children + 0x5D0 (UCr)
cleanup_children_on_exit + 0x70 (UCr)
git_atexit_dispatch + 0x200 (UCr)
__process_atexit_functions + 0xA0 (DLL zcredll)
CRE_TERMINATOR_ + 0xB50 (DLL zcredll)
exit + 0x2A0 (DLL zcrtldll)
die_webcgi + 0x240 (UCr)
die_errno + 0x360 (UCr)
write_or_die + 0x1C0 (UCr)
end_headers + 0x1A0 (UCr)
die_webcgi + 0x220 (UCr)
die + 0x320 (UCr)
inflate_request + 0x520 (UCr)
run_service + 0xC20 (UCr)
service_rpc + 0x530 (UCr)
cmd_main + 0xD00 (UCr)
main + 0x190 (UCr)

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Best guess is that a signal (SIGCHLD?) is possibly getting eaten or neglected somewhere between the test, perl, and git-http-

backend.

Quoted 23 lines

Quoted 23 lines

Quoted 3 lines

So we are trying to die(), which actually happens in die_webcgi(), and

then try

Quoted 7 lines

to write some message _but_ notice an error inside write_or_dir() and try to exit because we do not want to recurse forever trying to die, giving a message to say how/why we died, and die because failing to give that message, forever.

But in our attempt to exit(), we try to "cleanup children" and that is

what gets

Quoted 4 lines

stuck.

One big difference before and after the /dev/zero change is that the

process

Quoted 2 lines

is now on a downstream of the pipe. If we prepare a large file with a

finite

Quoted 2 lines

size full of NULs and replace /dev/null with it, instead of feeding NULs

from

Quoted 8 lines

Quoted 8 lines

Quoted 1 line

the pipe, would it change the equation?

Doubtful. The processes are still around, and are waiting on read but not actively reading (CPU time is not going up, so we're not reading an infinite stream). To me, this is a pipe situation where there is simply nothing waiting on the pipe (maybe a flush missing?). I'm grasping are straws without knowing the actual process architecture of

the

Quoted 13 lines

test to debug it.

So could you try with this patch?

-- snipsnap --

http-backend.c index renamed from http-backend.c +0 -0

d5cea0329a..7c1b4a2555
100644

http-backend.c +0 -0

@@ -427,6 +427,7 @@ static void inflate_request(const char *prog_name,

int out, int buffer_input, ss

http-backend.c static void inflate_request(const char *prog_name, int out, int buffer_input, ss · line 433

Quoted 6 lines

done:

http-backend.c +1 -0

 	git_inflate_end(&stream);
+	close(0);
 	close(out);
 	free(full_request);
 }

In isolation or with the other fixes associated with t5562? Or, which

commit should I use? 8989e1950a or d92031209a or some other?

Quoted 4 lines · Randall S. Becker

Unfortunately, subtest 13 still hangs on NonStop, even with this patch, so our Pipeline still hangs. I'm glad it's better on Azure, but I don't think this actually addresses the root cause of the hang.

Sigh.

Quoted 24 lines

possible this is not the test that is failing, but actually the git-http-backend? The code is not in a loop, if that helps. It is not consuming any significant cycles. I don't know that part of the code at all, sadly. The code is here:

* in the operating system from here up *
cleanup_children + 0x5D0 (UCr)
cleanup_children_on_exit + 0x70 (UCr)
git_atexit_dispatch + 0x200 (UCr)
__process_atexit_functions + 0xA0 (DLL zcredll)
CRE_TERMINATOR_ + 0xB50 (DLL zcredll)
exit + 0x2A0 (DLL zcrtldll)
die_webcgi + 0x240 (UCr)
die_errno + 0x360 (UCr)
write_or_die + 0x1C0 (UCr)
end_headers + 0x1A0 (UCr)
die_webcgi + 0x220 (UCr)
die + 0x320 (UCr)
inflate_request + 0x520 (UCr)
run_service + 0xC20 (UCr)
service_rpc + 0x530 (UCr)
cmd_main + 0xD00 (UCr)
main + 0x190 (UCr)

Quoted 3 lines

Best guess is that a signal (SIGCHLD?) is possibly getting eaten or neglected somewhere between the test, perl, and git-http-backend.

So we are trying to die(), which actually happens in die_webcgi(), and then try to write some message _but_ notice an error inside write_or_dir() and try to exit because we do not want to recurse forever trying to die, giving a message to say how/why we died, and die because failing to give that message, forever.

But in our attempt to exit(), we try to "cleanup children" and that is what gets stuck.

One big difference before and after the /dev/zero change is that the process is now on a downstream of the pipe. If we prepare a large file with a finite size full of NULs and replace /dev/null with it, instead of feeding NULs from the pipe, would it change the equation?

Quoted 2 lines

But in our attempt to exit(), we try to "cleanup children" and that is what gets stuck.

I have not paid enough attention to this thread to say if this is dumb, but just in case it's useful. For this class of problem where cleanup bites you for whatever reason in Perl, you can sometimes use this:

    use POSIX ();
    POSIX::_exit($code);

This will call "exit" from "stdlib" instead of Perl's "exit". So go away *now* and let the OS deal with the mess. Perl's will run around cleaning up stuff, freeing memory, running destructors etc, all of which might have side effects you don't want/care about, and might (as maybe in this case?) cause some hang.

To: Randall S. Becker <[email protected]>
Cc: 'Junio C Hamano' <[email protected]>; 'Johannes Schindelin via GitGitGadget' <[email protected]>; [email protected]; 'Max

<[email protected]>
Subject: RE: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1

Hi Randall,

Quoted 12 lines · Randall S. Becker

Quoted 11 lines · Junio C Hamano

To: Randall S. Becker <[email protected]>
Cc: 'Johannes Schindelin via GitGitGadget' <[email protected]>; [email protected]; 'Max Kirillov' <[email protected]>
Subject: Re: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1

Quoted 3 lines · Randall S. Becker

Unfortunately, subtest 13 still hangs on NonStop, even with this patch, so our Pipeline still hangs. I'm glad it's better on Azure, but I don't think this actually addresses the root cause of the

Quoted 50 lines

Quoted 27 lines

Sigh.

Quoted 24 lines

possible this is not the test that is failing, but actually the git-http-backend? The code is not in a loop, if that helps. It is not consuming any significant cycles. I don't know that part of the code at all, sadly. The code is here:

* in the operating system from here up *
cleanup_children + 0x5D0 (UCr)
cleanup_children_on_exit + 0x70 (UCr)
git_atexit_dispatch + 0x200 (UCr)
__process_atexit_functions + 0xA0 (DLL zcredll)
CRE_TERMINATOR_ + 0xB50 (DLL zcredll)
exit + 0x2A0 (DLL zcrtldll)
die_webcgi + 0x240 (UCr)
die_errno + 0x360 (UCr)
write_or_die + 0x1C0 (UCr)
end_headers + 0x1A0 (UCr)
die_webcgi + 0x220 (UCr)
die + 0x320 (UCr)
inflate_request + 0x520 (UCr)
run_service + 0xC20 (UCr)
service_rpc + 0x530 (UCr)
cmd_main + 0xD00 (UCr)
main + 0x190 (UCr)

Quoted 5 lines

Quoted 2 lines

Best guess is that a signal (SIGCHLD?) is possibly getting eaten or neglected somewhere between the test, perl, and git-http-backend.

So we are trying to die(), which actually happens in die_webcgi(), and

then try

Quoted 7 lines

to write some message _but_ notice an error inside write_or_dir() and try to exit because we do not want to recurse forever trying to die, giving a message to say how/why we died, and die because failing to give that message, forever.

But in our attempt to exit(), we try to "cleanup children" and that is

what gets

Quoted 3 lines

stuck.

One big difference before and after the /dev/zero change is that the

process

Quoted 2 lines

is now on a downstream of the pipe. If we prepare a large file with a

finite

Quoted 2 lines

size full of NULs and replace /dev/null with it, instead of feeding NULs

Quoted 8 lines

from

Quoted 1 line

the pipe, would it change the equation?

Doubtful. The processes are still around, and are waiting on read but not actively reading (CPU time is not going up, so we're not reading an infinite stream). To me, this is a pipe situation where there is simply nothing waiting on the pipe (maybe a flush missing?). I'm grasping are straws without knowing the actual process architecture of

test to debug it.

So could you try with this patch?

-- snipsnap --

http-backend.c index d5cea0329a..7c1b4a2555 renamed from http-backend.c +0 -0

100644

http-backend.c +0 -0

@@ -427,6 +427,7 @@ static void inflate_request(const char *prog_name,

int out, int buffer_input, ss

done:

Quoted 11 lines · Randall S. Becker

Quoted 10 lines · Johannes Schindelin

So could you try with this patch?

-- snipsnap --

http-backend.c index d5cea0329a..7c1b4a2555 renamed from http-backend.c +0 -0

100644

http-backend.c +0 -0

@@ -427,6 +427,7 @@ static void inflate_request(const char *prog_name,

int out, int buffer_input, ss

Quoted 9 lines

http-backend.c static void inflate_request(const char *prog_name, int out, int buffer_input, ss · line 433

Quoted 6 lines

done:

http-backend.c +1 -0

 	git_inflate_end(&stream);
+	close(0);
 	close(out);
 	free(full_request);
 }

In isolation or with the other fixes associated with t5562? Or, which baseline commit should I use? 8989e1950a or d92031209a or some other?

As far as I understand, it should be tried instead of
https://public-inbox.org/git/[email protected]/

Quoted 7 lines · Ævar Arnfjörð Bjarmason

Quoted 2 lines

But in our attempt to exit(), we try to "cleanup children" and that is what gets stuck.

I have not paid enough attention to this thread to say if this is dumb, but just in case it's useful. For this class of problem where cleanup bites you for whatever reason in Perl, you can sometimes use this:

Quoted 8 lines

    use POSIX ();
    POSIX::_exit($code);

This will call "exit" from "stdlib" instead of Perl's "exit". So go away *now* and let the OS deal with the mess. Perl's will run around cleaning up stuff, freeing memory, running destructors etc, all of which might have side effects you don't want/care about, and might (as maybe in this case?) cause some hang.

* Perl is running in foreground, so it cannot outlive test case and spoil the subsequent ones.
* From the dumps I have an impression that it waits legitimately - there are other processes to wait for. And anyway the waits happen before perl script comes to its exit.

Though I am already convinced that I should have done the helper in C. Let's see when I have time to fix it.