On 20 September 2017 at 22:02, Jeff King <peff@xxxxxxxx> wrote: > On Wed, Sep 20, 2017 at 09:47:24PM +0200, Martin Ågren wrote: > >> Instead of conditionally freeing `rev.pending.objects`, just call >> `object_array_clear()` on `rev.pending`. This means we don't poke as >> much into the implementation, which is already a good thing, but also >> that we free the individual entries as well, thereby fixing a >> memory-leak. > Looks good. A similar bug was the exact reason for adding the function > in 46be82312. I did a grep for 'free.*\.objects' to see if there were > other cases. Ah. I grepped for "pending.objects", but didn't go more general than that. > There are some hits. E.g., the one in orphaned_commit_warning(). It does > something funny with setting revs.leak_pending. But I _think_ after the > whole thing is done and we call free(refs.objects), that probably ought > to be an object_array_clear(). I'll look into that one in the evening. > As I suspect you're working your way through leak-checker results, I'm > OK if you want to punt on digging into more cases for now and just fix > ones that the tool has identified as real leaks. I am indeed going through ASan results. Your UNLEAK helps immensely! (I'm collecting UNLEAKs on the side. I see now that UNLEAK is in master, so I should probably submit those where I believe things are "UNLEAK-complete".) Thanks for looking at this. Martin
