threads / patch / 15942

patchforce_object_loose: Fix memory leak

Subject: [PATCH] force_object_loose: Fix memory leak

## tl;dr

3 messages between Oct 18, 2008 and Oct 18, 2008. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Björn Steinbrink· Oct 18, 2008, 00:37 UTC · lore

read_packed_sha1 expectes its caller to free the buffer it returns, which force_object_loose didn't do.

This leak is eventually triggered by "git gc", when it is manually invoked or there are too many packs around, making gc totally unusable when there are lots of unreachable objects.

Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
---
Tested with a somewhat recent clone of linux-2.6.git:
git clone linux-2.6 test
cd test
git remote rm origin
git reset --hard v2.6.16
git for-each-ref --format='%(refname)' refs/tags | \
	xargs -i git update-ref -d {}
git reflog expire --expire=0 --all
git repack -Ad

Without the fix, I killed repack when its memory usage got past 3 _GB_. With the fix, it stays at around 300MB until it starts writing the new pack, then it goes up to about 550MB or so.

 sha1_file.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
Show changes to sha1_file.c +5 −1
diff --git a/sha1_file.c b/sha1_file.c
index 3fbb082..70bb453 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2333,6 +2333,7 @@ int force_object_loose(const unsigned char *sha1, time_t mtime)
 	enum object_type type;
 	char hdr[32];
 	int hdrlen;
+	int ret;
 
 	if (has_loose_object(sha1))
 		return 0;
@@ -2340,7 +2341,10 @@ int force_object_loose(const unsigned char *sha1, time_t mtime)
 	if (!buf)
 		return error("cannot read sha1_file for %s", sha1_to_hex(sha1));
 	hdrlen = sprintf(hdr, "%s %lu", typename(type), len) + 1;
-	return write_loose_object(sha1, hdr, hdrlen, buf, len, mtime);
+	ret = write_loose_object(sha1, hdr, hdrlen, buf, len, mtime);
+	free(buf);
+
+	return ret;
 }
 
 int has_pack_index(const unsigned char *sha1)

-- 
1.6.0.2.541.g46dc1.dirty
Nicolas Pitre· Oct 18, 2008, 01:25 UTC · re: Björn Steinbrink · lore

Re: [PATCH] force_object_loose: Fix memory leak

On Sat, 18 Oct 2008, Björn Steinbrink wrote:
Show 8 quoted lines
> read_packed_sha1 expectes its caller to free the buffer it returns, which
> force_object_loose didn't do.
> 
> This leak is eventually triggered by "git gc", when it is manually invoked
> or there are too many packs around, making gc totally unusable when there
> are lots of unreachable objects.
> 
> Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
Good catch.
Acked-by: Nicolas Pitre <nico@cam.org>
Show 50 quoted lines
> ---
> Tested with a somewhat recent clone of linux-2.6.git:
> 
> git clone linux-2.6 test
> cd test
> git remote rm origin
> git reset --hard v2.6.16
> git for-each-ref --format='%(refname)' refs/tags | \
> 	xargs -i git update-ref -d {}
> git reflog expire --expire=0 --all
> git repack -Ad
> 
> Without the fix, I killed repack when its memory usage got past 3 _GB_.
> With the fix, it stays at around 300MB until it starts writing the
> new pack, then it goes up to about 550MB or so.
> 
>  sha1_file.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/sha1_file.c b/sha1_file.c
> index 3fbb082..70bb453 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2333,6 +2333,7 @@ int force_object_loose(const unsigned char *sha1, time_t mtime)
>  	enum object_type type;
>  	char hdr[32];
>  	int hdrlen;
> +	int ret;
>  
>  	if (has_loose_object(sha1))
>  		return 0;
> @@ -2340,7 +2341,10 @@ int force_object_loose(const unsigned char *sha1, time_t mtime)
>  	if (!buf)
>  		return error("cannot read sha1_file for %s", sha1_to_hex(sha1));
>  	hdrlen = sprintf(hdr, "%s %lu", typename(type), len) + 1;
> -	return write_loose_object(sha1, hdr, hdrlen, buf, len, mtime);
> +	ret = write_loose_object(sha1, hdr, hdrlen, buf, len, mtime);
> +	free(buf);
> +
> +	return ret;
>  }
>  
>  int has_pack_index(const unsigned char *sha1)
> -- 
> 1.6.0.2.541.g46dc1.dirty
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Nicolas

← back to recent threads