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

[Auditbeat] Package: Close librpm handle #12215

Merged
merged 2 commits into from
May 28, 2019

Conversation

cwurm
Copy link
Contributor

@cwurm cwurm commented May 20, 2019

Closes the librpm handle we dlopen() when the dataset is closed.

Lots of lines changed because I expanded and renamed cFun to be librpm, but the important part is saving the handle as librpm.handle and closing it in the new closeDataset() function.

Following #12168 this further reduces the leftover memory Valgrind reports (running valgrind --leak-check=full --show-leak-kinds=all ./auditbeat -e -d "*"):

Before

==8072== LEAK SUMMARY:
==8072==    definitely lost: 116 bytes in 1 blocks
==8072==    indirectly lost: 0 bytes in 0 blocks
==8072==      possibly lost: 3,336 bytes in 10 blocks
==8072==    still reachable: 36,492 bytes in 98 blocks
==8072==         suppressed: 0 bytes in 0 blocks

After

==7839== LEAK SUMMARY:
==7839==    definitely lost: 403 bytes in 3 blocks
==7839==    indirectly lost: 0 bytes in 0 blocks
==7839==      possibly lost: 2,844 bytes in 9 blocks
==7839==    still reachable: 16,840 bytes in 46 blocks
==7839==         suppressed: 0 bytes in 0 blocks

I'm not sure where the remainder is coming from, or if those numbers are even valid given that Valgrind is not really compatible with Golang.

@cwurm cwurm requested a review from adriansr May 20, 2019 23:43
@cwurm cwurm requested a review from a team as a code owner May 20, 2019 23:43
@elasticmachine
Copy link
Collaborator

Pinging @elastic/secops

@cwurm cwurm merged commit bc72158 into elastic:master May 28, 2019
@cwurm cwurm deleted the package_valgrind_dlopen branch May 28, 2019 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants