[PATCH] Close transport helper on protocol error

[thibault.jamet@gmail.com]

From: Thibault Jamet <thibault.jamet@gmail.com>

We have noticed that in some cases, when the transport is not fully
compliant with the protocol, git exits with a status of 128 without
closing the transport.

This usually does not have consequences in a standard unix
environment as the process gets then attached to the init one which will
then take care of closing properly the remaining transport.

We remarkably noticed this behaviour on GitHub Enterprise v2.14.24 when
a repository has been migrated to another GitHub Enterprise instance.
The output of the transport is then:

remote: Repository `org/repo' is disabled. Please ask the owner to check their account.
fatal: unable to access 'https://github.example.com/org/repo/': The requested URL returned error: 403

and the code exits inside function get_refs_list

In container contexts, there might not be such an init process
to take care of terminating the transport process and hence they remain
as zombies, as mentioned in
https://github.com/rancher/rancher/issues/13858 or
https://github.com/coreos/clair/issues/441.

Although there is a work-around running an init inside the container,
clean-up the processes created at the time git exits.

Signed-off-by: Thibault Jamet <thibault.jamet@gmail.com>
---
 transport-helper.c | 44 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index cec83bd663..34caa75a72 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -101,6 +101,12 @@ static void do_take_over(struct transport *transport)
 
 static void standard_options(struct transport *t);
 
+static int disconnect_helper(struct transport *transport);
+
+static int disconnect_helper_data(struct helper_data *data);
+
+static int release_helper(struct transport *transport);
+
 static struct child_process *get_helper(struct transport *transport)
 {
 	struct helper_data *data = transport->data;
@@ -155,8 +161,10 @@ static struct child_process *get_helper(struct transport *transport)
 	while (1) {
 		const char *capname, *arg;
 		int mandatory = 0;
-		if (recvline(data, &buf))
+		if (recvline(data, &buf)){
+			release_helper(transport);
 			exit(128);
+		}
 
 		if (!*buf.buf)
 			break;
@@ -215,10 +223,14 @@ static struct child_process *get_helper(struct transport *transport)
 
 static int disconnect_helper(struct transport *transport)
 {
-	struct helper_data *data = transport->data;
+	return disconnect_helper_data(transport->data);
+}
+
+static int disconnect_helper_data(struct helper_data *data)
+{
 	int res = 0;
 
-	if (data->helper) {
+	if (data && data->helper) {
 		if (debug)
 			fprintf(stderr, "Debug: Disconnecting.\n");
 		if (!data->no_disconnect_req) {
@@ -261,8 +273,10 @@ static int strbuf_set_helper_option(struct helper_data *data,
 	int ret;
 
 	sendline(data, buf);
-	if (recvline(data, buf))
+	if (recvline(data, buf)) {
+		disconnect_helper_data(data);
 		exit(128);
+	}
 
 	if (!strcmp(buf->buf, "ok"))
 		ret = 0;
@@ -366,10 +380,12 @@ static void standard_options(struct transport *t)
 static int release_helper(struct transport *transport)
 {
 	int res = 0;
-	struct helper_data *data = transport->data;
-	refspec_clear(&data->rs);
-	res = disconnect_helper(transport);
-	free(transport->data);
+	if (transport){
+		struct helper_data *data = transport->data;
+		refspec_clear(&data->rs);
+		res = disconnect_helper(transport);
+		free(transport->data);
+	}
 	return res;
 }
 
@@ -394,8 +410,10 @@ static int fetch_with_fetch(struct transport *transport,
 	sendline(data, &buf);
 
 	while (1) {
-		if (recvline(data, &buf))
+		if (recvline(data, &buf)){
+			release_helper(transport);
 			exit(128);
+		}
 
 		if (starts_with(buf.buf, "lock ")) {
 			const char *name = buf.buf + 5;
@@ -561,8 +579,10 @@ static int run_connect(struct transport *transport, struct strbuf *cmdbuf)
 	setvbuf(input, NULL, _IONBF, 0);
 
 	sendline(data, cmdbuf);
-	if (recvline_fh(input, cmdbuf))
+	if (recvline_fh(input, cmdbuf)){
+		release_helper(transport);
 		exit(128);
+	}
 
 	if (!strcmp(cmdbuf->buf, "")) {
 		data->no_disconnect_req = 1;
@@ -1074,8 +1094,10 @@ static struct ref *get_refs_list(struct transport *transport, int for_push,
 
 	while (1) {
 		char *eov, *eon;
-		if (recvline(data, &buf))
+		if (recvline(data, &buf)){
+			release_helper(transport);
 			exit(128);
+		}
 
 		if (!*buf.buf)
 			break;
-- 
2.21.0