git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jon Smirl" <jonsmirl@gmail.com>
To: "Git Mailing List" <git@vger.kernel.org>
Subject: dmalloc and leaks in git
Date: Sat, 8 Dec 2007 15:53:44 -0500	[thread overview]
Message-ID: <9e4733910712081253t7cd43f87o6001f32fddc01565@mail.gmail.com> (raw)

It is very easy to use dmalloc with git. Follow the instructions here,
http://dmalloc.com/docs/latest/online/dmalloc_4.html

But using dmalloc shows a pervasive problem, none of the git commands
are cleaning up after themselves. For example I ran a simple command,
git-status, and thousands of objects were not freed.

Normally this doesn't hurt since exiting the process obviously frees
all of the memory. But when programming this way it becomes impossible
to tell which leaks were on purpose and which were accidental.

To sort this out an #ifdef DMALLOC needs to be created and then code
for freeing all of the 'on purpose' leaks needs to be added in an
IFDEF right before the process exits. The test scripts can then be
modified to ensure that everything is freed when the command exits.

I've used this process on several projects I've managed and it is a
very good thing to do. Once the new infrastructure is in place leaks
can be detected automatically and nipped in the bud before they get
out of control. The key to making this work is getting code in place
in the #ifdef to free those "on-purpose" leaks.

I tried a couple of year ago to add leak detection to Mozilla but
Mozilla is way too far gone. There are 10,000 places where things are
allocated and not being freed. It is a huge manually intensive task
sorting out which of these were on-purpose vs accidental. If Mozilla
had followed a discipline of ensuring that nothing was every leaked
(by using the scheme above) a lot of recent leak clean up work could
have been avoided.

I don't know enough about the structure of git to add the cleanups in
#ifdefs before exit. People who wrote the commands are going to have
to help out with this.

diff --git a/Makefile b/Makefile
index 0a5df7a..426830c 100644
--- a/Makefile
+++ b/Makefile
@@ -752,7 +752,7 @@ SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))

-LIBS = $(GITLIBS) $(EXTLIBS)
+LIBS = $(GITLIBS) $(EXTLIBS) -ldmalloc

 BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
 	$(COMPAT_CFLAGS)
diff --git a/git-compat-util.h b/git-compat-util.h
index 79eb10e..8894c30 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -428,3 +428,5 @@ static inline int strtol_i(char const *s, int
base, int *result)
 }

 #endif
+
+#include "dmalloc.h"

-- 
Jon Smirl
jonsmirl@gmail.com

             reply	other threads:[~2007-12-08 20:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-08 20:53 Jon Smirl [this message]
2007-12-08 20:58 ` dmalloc and leaks in git Johannes Schindelin
2007-12-08 21:02   ` Jon Smirl
2007-12-08 21:19     ` Johannes Schindelin
2007-12-09 20:57 ` Linus Torvalds
2007-12-10 16:34   ` Linus Torvalds
2007-12-10 16:54     ` Nicolas Pitre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9e4733910712081253t7cd43f87o6001f32fddc01565@mail.gmail.com \
    --to=jonsmirl@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).