Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leaks #4477

Merged
merged 4 commits into from
Jan 17, 2018
Merged

Memory leaks #4477

merged 4 commits into from
Jan 17, 2018

Conversation

pks-t
Copy link
Member

@pks-t pks-t commented Jan 12, 2018

Two fixes for memory leaks highlighted by AppVeyor.

Two tests in network::fetchlocal explicitly set a cleanup function to
free and remove the created sandbox repositories. This is not necessary,
though, as the cleanup function executed after each test already takes
care of cleaning up after them. Remove the code to avoid needless code
duplication.
Upon downloading the pack file, the local transport will iterate through
every reference using `git_reference_foreach`. The function is a bit
tricky though in that it requires the passed callback to free the
references, which does not currently happen.

Fix the memory leak by freeing all passed references in the callback.
References passed to the callback function of `git_reference_foreach`
are expected to be owned by the callback. As such, they are never being
freed by `git_reference_foreach`, but will have to be freed by the
caller. This small detail is never mentioned in the function's
documentation, though, making it easy to get wrong. Document this to
make it discoverable.
The test refs::iterator::foreach_name iterates through every reference
and copies its name into a local vector. While the test makes sure to
free the vector afterwards, the copied reference names are not being
free'd. Fix that.
@carlosmn carlosmn merged commit ecd55ce into libgit2:master Jan 17, 2018
@pks-t pks-t deleted the pks/memleaks branch July 11, 2019 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants