http-backend: enable cleaning up forked upload/receive-pack on exit
diff mbox series

Message ID 20181124134827.13932-1-max@max630.net
State New
Headers show
Series
  • http-backend: enable cleaning up forked upload/receive-pack on exit
Related show

Commit Message

Max Kirillov Nov. 24, 2018, 1:48 p.m. UTC
If http-backend dies because of errors, started upload-pack or
receive-pack are not killed and waited, but rather stay running for somtime
until they exits because of closed stdin. It may be undesirable in working
environment, and it also causes occasional failure of t5562, because the
processes keep opened act.err, and sometimes write there errors after next test
started using the file.

Fix by enabling cleaning of the command at http-backed exit.

Reported-by: Carlo Arenas <carenas@gmail.com>
Helped-by: Carlo Arenas <carenas@gmail.com>
Signed-off-by: Max Kirillov <max@max630.net>
---
This seems to fix the issue at NetBSD. I verified it manually with strace but could
not catch the visible timing effect in tests at Linux. So no tests for it.

the "t5562: do not reuse output files" patches are not needed then
 http-backend.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Junio C Hamano Nov. 26, 2018, 2:10 a.m. UTC | #1
Max Kirillov <max@max630.net> writes:

> If http-backend dies because of errors, started upload-pack or
> receive-pack are not killed and waited, but rather stay running for somtime

"sometime" (will fix locally, no reason for a resend).

> until they exits because of closed stdin. It may be undesirable in working

"they exit" (ditto)

> environment, and it also causes occasional failure of t5562, because the
> processes keep opened act.err, and sometimes write there errors after next test
> started using the file.
>
> Fix by enabling cleaning of the command at http-backed exit.

Thanks for a clear explanation.

Will queue.

> Reported-by: Carlo Arenas <carenas@gmail.com>
> Helped-by: Carlo Arenas <carenas@gmail.com>
> Signed-off-by: Max Kirillov <max@max630.net>
> ---
> This seems to fix the issue at NetBSD. I verified it manually with strace but could
> not catch the visible timing effect in tests at Linux. So no tests for it.
>
> the "t5562: do not reuse output files" patches are not needed then
>  http-backend.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/http-backend.c b/http-backend.c
> index 9e894f197f..29e68e38b5 100644
> --- a/http-backend.c
> +++ b/http-backend.c
> @@ -486,6 +486,8 @@ static void run_service(const char **argv, int buffer_input)
>  	if (buffer_input || gzipped_request || req_len >= 0)
>  		cld.in = -1;
>  	cld.git_cmd = 1;
> +	cld.clean_on_exit = 1;
> +	cld.wait_after_clean = 1;
>  	if (start_command(&cld))
>  		exit(1);

Patch
diff mbox series

diff --git a/http-backend.c b/http-backend.c
index 9e894f197f..29e68e38b5 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -486,6 +486,8 @@  static void run_service(const char **argv, int buffer_input)
 	if (buffer_input || gzipped_request || req_len >= 0)
 		cld.in = -1;
 	cld.git_cmd = 1;
+	cld.clean_on_exit = 1;
+	cld.wait_after_clean = 1;
 	if (start_command(&cld))
 		exit(1);