GNOME Bugzilla – Bug 779444
Fix a few leaks in plugins
Last modified: 2017-03-02 12:28:06 UTC
up
Created attachment 347001 [details] [review] lua-factory: Don't leak list in xml parser We were calling g_list_free() in the pointer that is the last element of the list due g_list_reverse() that was called previously. 1,176 (336 direct, 840 indirect) bytes in 14 blocks are definitely lost in loss record 3,289 of 3,315 at 0x4C2DB9D: malloc (vg_replace_malloc.c:299) by 0x4E895B8: g_malloc (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4EA1B12: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4E7F9E5: g_list_prepend (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x9377E69: build_table_recursively (lua-xml.c:64) by 0x9377FAF: build_table_recursively (lua-xml.c:81) by 0x9377FAF: build_table_recursively (lua-xml.c:81) by 0x9377FAF: build_table_recursively (lua-xml.c:81) by 0x9378182: build_table_from_xml_reader (lua-xml.c:121)
Created attachment 347002 [details] [review] tests: Do not leak missing keys in spotify 24 bytes in 1 blocks are definitely lost in loss record 1,235 of 3,381 at 0x4C2DB9D: malloc (vg_replace_malloc.c:299) by 0x4E895B8: g_malloc (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4EA1B12: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4E7F9E5: g_list_prepend (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x9372497: grl_lua_factory_source_may_resolve (grl-lua-factory.c:1717) by 0x53BBC13: grl_source_may_resolve (grl-source.c:3464) by 0x401C00: test_may_resolve_missing_key (test_spotify_cover.c:112) 48 (24 direct, 24 indirect) bytes in 1 blocks are definitely lost in loss record 2,010 of 3,381 at 0x4C2DB9D: malloc (vg_replace_malloc.c:299) by 0x4E895B8: g_malloc (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4EA1B12: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4E7FE19: g_list_copy_deep (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x93724D4: grl_lua_factory_source_may_resolve (grl-lua-factory.c:1696) by 0x53BBC13: grl_source_may_resolve (grl-source.c:3464) by 0x401A7B: test_may_resolve_missing_media (test_spotify_cover.c:133)
Created attachment 347003 [details] [review] lua-factory: fix leak from grl_config_get_string() As it uses g_key_file_get_string() which returns newly allocated. 27 bytes in 1 blocks are definitely lost in loss record 1,825 of 4,702 at 0x4C2DB9D: malloc (vg_replace_malloc.c:299) by 0x4E895B8: g_malloc (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4E7B24A: ??? (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4E7CA27: g_key_file_get_string (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x53D0514: grl_config_get_string (grl-config.c:278) by 0xA0A5BDD: all_mandatory_options_has_value (grl-lua-factory.c:1133) by 0xA0A5BDD: grl_lua_factory_source_new (grl-lua-factory.c:467) by 0xA0A68CB: grl_lua_factory_plugin_init (grl-lua-factory.c:235) by 0x53AD615: grl_plugin_load (grl-plugin.c:243) by 0x53AF3BF: activate_plugin (grl-registry.c:483) by 0x53B2584: grl_registry_activate_plugin_by_id (grl-registry.c:1382) by 0x40206C: test_lua_factory_setup (test_lua_factory_utils.c:60) by 0x40193E: test_acoustid_setup (test_lua_acoustid.c:194) by 0x40193E: main (test_lua_acoustid.c:200) 27 bytes in 1 blocks are definitely lost in loss record 1,839 of 4,775 at 0x4C2DB9D: malloc (vg_replace_malloc.c:299) by 0x4E895B8: g_malloc (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4E7B24A: ??? (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4E7CA27: g_key_file_get_string (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x53D0514: grl_config_get_string (grl-config.c:278) by 0xA0A5F06: lua_plugin_source_init (grl-lua-factory.c:580) by 0xA0A5F06: grl_lua_factory_source_new (grl-lua-factory.c:477) by 0xA0A68DB: grl_lua_factory_plugin_init (grl-lua-factory.c:235) by 0x53AD615: grl_plugin_load (grl-plugin.c:243) by 0x53AF3BF: activate_plugin (grl-registry.c:483) by 0x53B2584: grl_registry_activate_plugin_by_id (grl-registry.c:1382) by 0x40206C: test_lua_factory_setup (test_lua_factory_utils.c:60) by 0x40193E: test_acoustid_setup (test_lua_acoustid.c:194) by 0x40193E: main (test_lua_acoustid.c:200)
Created attachment 347004 [details] [review] metadata-store: ignore error on backwards compatibility query Failure happens due the fact that sql table already has those columns. This fixes the following leak. 40 bytes in 1 blocks are definitely lost in loss record 10,094 of 20,529 at 0x4C2DB9D: malloc (vg_replace_malloc.c:299) by 0x217107D6: sqlite3MemMalloc (sqlite3.c:20398) by 0x216EB967: mallocWithAlarm (sqlite3.c:24067) by 0x216EB967: sqlite3Malloc (sqlite3.c:24098) by 0x2175382B: sqlite3_exec (sqlite3.c:108798) by 0x2624FD1B: grl_metadata_store_source_init (grl-metadata-store.c:271) by 0x53C07FA: g_type_create_instance (gtype.c:1866) by 0x53A269A: g_object_new_internal (gobject.c:1783) by 0x53A45AD: g_object_new_valist (gobject.c:2042) by 0x53A4850: g_object_new (gobject.c:1626) by 0x2624F93C: grl_metadata_store_source_new (grl-metadata-store.c:194) by 0x2624F874: grl_metadata_store_source_plugin_init (grl-metadata-store.c:167) by 0x4E48615: grl_plugin_load (grl-plugin.c:243)
Created attachment 347005 [details] [review] metadata-store: set finalize function to clean up 514,040 (80 direct, 513,960 indirect) bytes in 1 blocks are definitely lost in loss record 19,311 of 19,314 at 0x4C2DB9D: malloc (vg_replace_malloc.c:299) by 0x222277D6: sqlite3MemMalloc (sqlite3.c:20398) by 0x22202967: mallocWithAlarm (sqlite3.c:24067) by 0x22202967: sqlite3Malloc (sqlite3.c:24098) by 0x22202A0D: sqlite3MallocZero (sqlite3.c:24398) by 0x222499F5: sqlite3BtreeOpen (sqlite3.c:60320) by 0x22297786: openDatabase (sqlite3.c:139437) by 0x2662D0B0: grl_metadata_store_source_init (grl-metadata-store.c:237) by 0x53C07FA: g_type_create_instance (gtype.c:1866) by 0x53A269A: g_object_new_internal (gobject.c:1783) by 0x53A45AD: g_object_new_valist (gobject.c:2042) by 0x53A4850: g_object_new (gobject.c:1626) by 0x2662CF9E: grl_metadata_store_source_new (grl-metadata-store.c:194) by 0x2662CF9E: grl_metadata_store_source_plugin_init (grl-metadata-store.c:167)
Review of attachment 347001 [details] [review]: ::: src/lua-factory/lua-library/lua-xml.c @@ +72,3 @@ guint i; xmlNodePtr node; + GList *first = it; Can you name it "list" instead of "first"?
Review of attachment 347002 [details] [review]: Sure.
Review of attachment 347003 [details] [review]: Please.
Review of attachment 347004 [details] [review]: Sure. Might be nice to port this to gom...
Review of attachment 347005 [details] [review]: Yep.
Created attachment 347035 [details] [review] lua-factory: Don't leak list in xml parser We were calling g_list_free() in the pointer that is the last element of the list due g_list_reverse() that was called previously. 1,176 (336 direct, 840 indirect) bytes in 14 blocks are definitely lost in loss record 3,289 of 3,315 at 0x4C2DB9D: malloc (vg_replace_malloc.c:299) by 0x4E895B8: g_malloc (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4EA1B12: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x4E7F9E5: g_list_prepend (in /usr/lib64/libglib-2.0.so.0.5000.3) by 0x9377E69: build_table_recursively (lua-xml.c:64) by 0x9377FAF: build_table_recursively (lua-xml.c:81) by 0x9377FAF: build_table_recursively (lua-xml.c:81) by 0x9377FAF: build_table_recursively (lua-xml.c:81) by 0x9378182: build_table_from_xml_reader (lua-xml.c:121)
(In reply to Bastien Nocera from comment #9) > Review of attachment 347004 [details] [review] [review]: > > Sure. > > Might be nice to port this to gom... Yes. Filed bug 779460 for it
Review of attachment 347035 [details] [review]: ++
Attachment 347002 [details] pushed as 85ac986 - tests: Do not leak missing keys in spotify Attachment 347003 [details] pushed as d3e7fed - lua-factory: fix leak from grl_config_get_string() Attachment 347004 [details] pushed as 83b76e0 - metadata-store: ignore error on backwards compatibility query Attachment 347005 [details] pushed as 475044b - metadata-store: set finalize function to clean up Attachment 347035 [details] pushed as b3ab9d0 - lua-factory: Don't leak list in xml parser