git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Josh Steadmon <steadmon@google.com>,
	git@vger.kernel.org, stolee@gmail.com
Subject: Re: [PATCH] commit-graph: fix memory leak
Date: Tue, 7 May 2019 18:26:01 -0400	[thread overview]
Message-ID: <20190507222601.GA976@sigill.intra.peff.net> (raw)
In-Reply-To: <87zhnyh9vu.fsf@evledraar.gmail.com>

On Tue, May 07, 2019 at 11:49:41AM +0200, Ævar Arnfjörð Bjarmason wrote:

> I wonder in general if there's a more sustainable solution to these
> one-at-a-time memory leak fixes we're doing to these
> libraries. E.g. marking some tests in the test suite as passing cleanly
> with valgrind's leak checker, and adding a test mode to run those tests.

I'd recommend going with the LeakSanitizer, since the resulting tests
run a lot faster.  We made some progress a while ago, and some tests do
pass, but there's a lot of manual inspection (and either fixing leaks,
or annotating with UNLEAK as appropriate) still to do.

Running "make SANITIZE=leak test" shows our current state.

If we just want to stop the bleeding, so to speak, I suspect that rather
than marking individual tests as "clean", we'd do better to collect all
of the results, sort and remove duplicates, and then just compare the
result before and after certain branches. That would tell us the new
leaks being added.

Something like:

  export LSAN_OPTIONS=exitcode=0:log_path=/tmp/lsan
  make SANITIZE=leak test

should dump a bunch of files in /tmp. (Note that when we tried this in
late 2017, log_path did not seem to work in pure-LSan mode, but I think
this was a bug; it works fine for me now).

Collating the results is a little tricky, because the top of the stack
when the leak was allocated is usually uninteresting (it's almost always
xmalloc).

There's some discussion and some scripts in:

  https://public-inbox.org/git/20170923163817.7ltmkav2ytk7n43k@sigill.intra.peff.net/

and

  https://public-inbox.org/git/20170925160835.aoomjaqrn2o2aosi@sigill.intra.peff.net/

I think just pumping the results of the second one through "sort -u"
would get you a starting point that you could use for before/after
diffs.

-Peff

      reply	other threads:[~2019-05-07 22:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-06 21:36 [PATCH] commit-graph: fix memory leak Josh Steadmon
2019-05-06 21:58 ` Emily Shaffer
2019-05-07  1:58   ` Derrick Stolee
2019-05-07  9:49 ` Ævar Arnfjörð Bjarmason
2019-05-07 22:26   ` Jeff King [this message]

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=20190507222601.GA976@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=steadmon@google.com \
    --cc=stolee@gmail.com \
    /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).