Thread (3 messages) flat view 3 messages, 2 authors, 2016-06-15
DORMANTno replies

[PATCH] Fix bunch of fd leaks in http-fetch

From: Petr Baudis <hidden>
Date: 2016-06-15 22:42:11
Subsystem: the rest · Maintainer: Linus Torvalds

Dear diary, on Fri, Nov 11, 2005 at 11:58:39PM CET, I got a letter
where Becky Bruce [off-list ref] said that...
I grabbed 0.99.9g this morning, and tried to clone Paul Mackerras' 
linux merge tree. The clone failed and reported errors in http-fetch 
with a bunch of messages of the form:

error: Couldn't create temporary file 
.git/objects/04/48fa7de8a416a48cd1977f29858be54e67c078.temp for .git
/objects/04/48fa7de8a416a48cd1977f29858be54e67c078: Error 24: Too many 
open files
---

The current http-fetch is rather careless about fd leakage, causing
problems while fetching large repositories. This patch does not reserve
exhaustiveness, but I covered everything I spotted. I also left some
safeguards in place in case I missed something, so that we get to know,
sooner or later.

Reported by Becky Bruce [off-list ref].

Signed-off-by: Petr Baudis <redacted>
---

 http-fetch.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/http-fetch.c b/http-fetch.c
index 99921cc..e7655d1 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -413,6 +413,8 @@ static void start_request(struct transfe
 	rename(request->tmpfile, prevfile);
 	unlink(request->tmpfile);
 
+	if (request->local != -1)
+		error("fd leakage in start: %d", request->local);
 	request->local = open(request->tmpfile,
 			      O_WRONLY | O_CREAT | O_EXCL, 0666);
 	/* This could have failed due to the "lazy directory creation";
@@ -511,7 +513,7 @@ static void start_request(struct transfe
 	/* Try to get the request started, abort the request on error */
 	if (!start_active_slot(slot)) {
 		request->state = ABORTED;
-		close(request->local);
+		close(request->local); request->local = -1;
 		free(request->url);
 		return;
 	}
@@ -525,7 +527,7 @@ static void finish_request(struct transf
 	struct stat st;
 
 	fchmod(request->local, 0444);
-	close(request->local);
+	close(request->local); request->local = -1;
 
 	if (request->http_code == 416) {
 		fprintf(stderr, "Warning: requested range invalid; we may already have all the data.\n");
@@ -557,6 +559,8 @@ static void release_request(struct trans
 {
 	struct transfer_request *entry = request_queue_head;
 
+	if (request->local != -1)
+		error("fd leakage in release: %d", request->local);
 	if (request == request_queue_head) {
 		request_queue_head = request->next;
 	} else {
@@ -613,6 +617,8 @@ static void process_curl_messages(void)
 					if (request->repo->next != NULL) {
 						request->repo =
 							request->repo->next;
+						close(request->local);
+							request->local = -1;
 						start_request(request);
 					}
 				} else {
@@ -743,6 +749,7 @@ static int fetch_index(struct alt_base *
 				     curl_errorstr);
 		}
 	} else {
+		fclose(indexfile);
 		return error("Unable to start request");
 	}
 
@@ -1025,6 +1032,7 @@ static int fetch_pack(struct alt_base *r
 				     curl_errorstr);
 		}
 	} else {
+		fclose(packfile);
 		return error("Unable to start request");
 	}
 
@@ -1087,6 +1095,7 @@ static int fetch_object(struct alt_base 
 			fetch_alternates(alt->base);
 			if (request->repo->next != NULL) {
 				request->repo = request->repo->next;
+				close(request->local); request->local = -1;
 				start_request(request);
 			}
 		} else {
@@ -1095,6 +1104,9 @@ static int fetch_object(struct alt_base 
 		}
 #endif
 	}
+	if (request->local != -1) {
+		close(request->local); request->local = -1;
+	}
 
 	if (request->state == ABORTED) {
 		release_request(request);

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help