Repository navigation
blame: increment reference count for origin's commit - #3842
ethomson merged 1 commit into
Conversation
Sorry, something went wrong.
|
Ok, I revert my previous comment about no additional memory leaks. Seems like Valgrind just wasn't able to catch them. More fiddeling with refcounts then :/ |
Sorry, something went wrong.
When we create a blame origin, we try to look up the blob that is to be blamed at a certain revision. When this lookup fails, e.g. because the file did not exist at that certain revision, we fail to create the blame origin and return `NULL`. The blame origin that we have just allocated is thereby free'd with `origin_decref`. The `origin_decref` function does not only decrement reference counts for the blame origin, though, but also for its commit and blob. When this is done in the error case, we will cause an uneven reference count for these objects. This may result in hard-to-debug failures at seemingly unrelated code paths, where we try to access these objects when they in fact have already been free'd. Fix the issue by refactoring `make_origin` such that we only allocate the object after the only function that may fail so that we do not have to call `origin_decref` at all. Also fix the `pass_blame` function, which indirectly calls `make_origin`, to free the commit when `make_origin` failed.
|
Phew. I've finally been able to get to a version that has no memory leaks in our test suite while seemingly fixing the issues encountered in #3793. Running the modified example of @ameily does not cause a segfault after a few seconds anymore. I've been running the test on the libgit2 repo now for 20 minutes and until now everything looks good. These small issues are always a tad depressing when one looks at how long it took to fix related to the actual outcome, which most often looks rather trivial :/ But meh, whatever. |
Sorry, something went wrong.
Yeah, and blame is a special kind of terrible. :( As always, thanks. |
Sorry, something went wrong.
blame: do not decrement commit refcount in make_origin
When we create a blame origin, we try to look up the blob that is
to be blamed at a certain revision. When this lookup fails, e.g.
because the file did not exist at that certain revision, we fail
to create the blame origin and return NULL. The blame origin
that we have just allocated is thereby free'd with
origin_decref.
The origin_decref function does not only decrement reference
counts for the blame origin, though, but also for its commit and
blob. When this is done in the error case, we will cause an
uneven reference count for these objects. This may result in
hard-to-debug failures at seemingly unrelated code paths, where
we try to access these objects when they in fact have already
been free'd.
Fix the issue by refactoring make_origin such that we only
allocate the object after the only function that may fail so that
we do not have to call origin_decref at all. Also fix the
pass_blame function, which indirectly calls make_origin, to
free the commit when make_origin failed.