tempfile: release deactivated strbufs instead of resetting
authorJeff King <peff@peff.net>
Tue, 5 Sep 2017 12:14:56 +0000 (08:14 -0400)
committerJunio C Hamano <gitster@pobox.com>
Wed, 6 Sep 2017 08:19:54 +0000 (17:19 +0900)
When a tempfile is deactivated, we reset its strbuf to the
empty string, which means we hold onto the memory for later
reuse.

Since we'd like to move to a system where tempfile structs
can actually be freed, deactivating one should drop all
resources it is currently using. And thus "release" rather
than "reset" is the appropriate function to call.

In theory the reset may have saved a malloc() when a
tempfile (or a lockfile) is reused multiple times. But in
practice this happened rarely. Most of our tempfiles are
single-use, since in cases where we might actually use many
(like ref locking) we xcalloc() a fresh one for each ref. In
fact, we leak those locks (to appease the rule that tempfile
storage can never be freed). Which means that using reset is
actively hurting us: instead of leaking just the tempfile
struct, we're leaking the extra heap chunk for the filename,
too.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
tempfile.c
index 3348ad59dd3beb44e3057856edffaddea40a24c1..e655e2847703b04df39a44de64d57a4b66b0312b 100644 (file)
@@ -128,7 +128,7 @@ static void activate_tempfile(struct tempfile *tempfile)
 static void deactivate_tempfile(struct tempfile *tempfile)
 {
        tempfile->active = 0;
-       strbuf_reset(&tempfile->filename);
+       strbuf_release(&tempfile->filename);
 }
 
 /* Make sure errno contains a meaningful value on error */