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

Fix memory leaks #193

Merged
merged 3 commits into from Jun 24, 2018
Merged

Fix memory leaks #193

merged 3 commits into from Jun 24, 2018

Conversation

PhilMiller
Copy link

Together, these were leaking megabytes at a time in the downstream program isenkramd every time I plugged in my mouse.

Leaks were observed in isenkramd on Debian

Diagnosis and confirmation of the fix via valgrind
@PhilMiller
Copy link
Author

There are still a couple much lower-rate leaks shown in valgrind, but I don't have the energy to wrap them up tonight. I can deal with them tomorrow:

==26466== 67,550 (16,160 direct, 51,390 indirect) bytes in 404 blocks are definitely lost in loss record 5,530 of 5,538
==26466== at 0x4C2CB8F: malloc (vg_replace_malloc.c:299)
==26466== by 0x6D70858: g_malloc (gmem.c:99)
==26466== by 0x6D88675: g_slice_alloc (gslice.c:1025)
==26466== by 0x6D88B28: g_slice_alloc0 (gslice.c:1051)
==26466== by 0x6AFF604: g_type_create_instance (gtype.c:1848)
==26466== by 0x6AE05A7: g_object_new_internal (gobject.c:1799)
==26466== by 0x6AE1D44: g_object_new_with_properties (gobject.c:1967)
==26466== by 0x6AE27C0: g_object_new (gobject.c:1639)
==26466== by 0x75285FD: _g_local_file_info_get (glocalfileinfo.c:1746)
==26466== by 0x7525B8B: g_local_file_enumerator_next_file (glocalfileenumerator.c:407)
==26466== by 0x7497304: g_file_enumerator_next_file (gfileenumerator.c:236)
==26466== by 0x135F3094: as_utils_find_files_matching (as-utils.c:293)
==26466==
==26466== 72,364 bytes in 4,495 blocks are definitely lost in loss record 5,532 of 5,538
==26466== at 0x4C2CB8F: malloc (vg_replace_malloc.c:299)
==26466== by 0x6D70858: g_malloc (gmem.c:99)
==26466== by 0x6D8A13E: g_strdup (gstrfuncs.c:363)
==26466== by 0x135FBF24: as_component_set_origin (as-component.c:1029)
==26466== by 0x13602E50: as_component_set_from_variant (as-component.c:5352)
==26466== by 0x1360602A: as_cache_file_read (as-pool.c:1758)
==26466== by 0x13606799: as_pool_load_cache_file (as-pool.c:1036)
==26466== by 0x13606E43: as_pool_load_collection_data (as-pool.c:659)
==26466== by 0x13606F3C: as_pool_load (as-pool.c:1004)
==26466== by 0x703AFCD: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==26466== by 0x703A93E: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==26466== by 0x6674EA0: ??? (in /usr/lib/python2.7/dist-packages/gi/_gi.x86_64-linux-gnu.so)

@PhilMiller
Copy link
Author

PhilMiller commented Jun 14, 2018

The added third commit addresses the leak in as_utils_find_files_matching.

I don't have enough knowledge of the code to immediately see where the last leak loses its reference to the affected object, so someone else will have to address that. I've opened bug #194 for this.

@PhilMiller PhilMiller mentioned this pull request Jun 14, 2018
@ximion
Copy link
Owner

ximion commented Jun 14, 2018

Thank you for your patch! I will review & merge it later today.
One thing that is notable is that the patch can be simplified by using the g_auto macro to automatically finalize a GVariantDict when going out of the current scope. That prevents the issue from happening again and simplifies the code.
The same can likely be done for the GVariant (via g_autoptr) related issue and for the path string in find_files_matching using g_autofree there.

@PhilMiller
Copy link
Author

PhilMiller commented Jun 14, 2018 via email

@ximion
Copy link
Owner

ximion commented Jun 24, 2018

Could I let you fix it the preferred way?

Yes, sorry for taking so long to merge this (I wanted to make sure that when I merge the changes I can also adjust them to use g_auto* immediately).

Thank you for your contribution!

@ximion ximion merged commit 82f5a56 into ximion:master Jun 24, 2018
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.

None yet

2 participants