GNOME Bugzilla – Bug 752938
Leak fixes for telepathy-accounts-widgets
Last modified: 2015-07-31 19:07:21 UTC
Here are 3 patches fixing small leaks I found in telepathy-accounts-manager when running gnome-online-accounts in valgrind.
Created attachment 308250 [details] [review] Free TpawProtocol::service_name in finalize() It will be leaked otherwise.
Created attachment 308251 [details] [review] Free GList returned by tp_list_connection_managers_finish() Make sure it's not leaked in tpaw_connection_managers_listed_cb()
Created attachment 308252 [details] [review] Fix GetProtocolsData leak A GetProtocolsData instance gets associated with a GSimpleAsyncResult in tpaw_protocol_get_all_async() using g_simple_async_result_set_op_res_gpointer(). A GDestroyNotify is set so that the GetProtocolsData instance is freed when the GSimpleAsyncResult is freed. However, this is not working as expected as the GetProtocolsData instance is keeping a reference on the GSimpleAsyncResult it's associated with, which creates a circular reference and causes the GetProtocolsData instance to never be freed. This commit drops the reference on the GSimpleAsyncResult instance owned by the GetProtocolsData instance as soon as it's no longer needed so that the GSimpleAsyncResult can be disposed, which triggers the destruction of the GetProtocolsData too.
Review of attachment 308250 [details] [review]: ++
Review of attachment 308251 [details] [review]: Thanks, Christophe. One small issue: ::: tp-account-widgets/tpaw-connection-managers.c @@ +224,3 @@ g_object_ref (cm)); } + g_list_free (cms); We need to unref the elements within the list too - either by stealing the references in the loop or by using g_list_free_full. I prefer the later because it is more explicit.
Review of attachment 308252 [details] [review]: ::: tp-account-widgets/tpaw-protocol.c @@ +486,3 @@ g_simple_async_result_take_error (data->result, error); g_simple_async_result_complete_in_idle (data->result); + g_object_unref (data->result); I would have used a 'goto out', but whatever.
Could you please update the telepathy-account-widgets submodules in empathy and gnome-online-accounts once the patches have landed?
Attachment 308250 [details] pushed as 40982ee - Free TpawProtocol::service_name in finalize() Attachment 308251 [details] pushed as 103ed62 - Free GList returned by tp_list_connection_managers_finish() Attachment 308252 [details] pushed as 6a4a30f - Fix GetProtocolsData leak
Review of attachment 308251 [details] [review]: Grmble, I only saw that review comment hours after pushing all 3 patches (including that one), sorry about that. I'll send a follow up patch to address this.
(In reply to Christophe Fergeau from comment #9) > Grmble, I only saw that review comment hours after pushing all 3 patches > (including that one), sorry about that. I'll send a follow up patch to > address this. That's ok.
Created attachment 308440 [details] [review] Properly fix tp_list_connection_managers_finish() leak Commit 103ed6232 attempted to fix a leak in tp_list_connection_managers_finish() but it was incomplete as it only freed the returned GList with g_list_free() while the list elements must be freed too. This commit addresses that using g_list_free_full() ==9013== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==9013== by 0x7F150AC: g_malloc (gmem.c:97) ==9013== by 0x7F2E0F7: g_slice_alloc (gslice.c:1007) ==9013== by 0x7F091B7: g_list_copy_deep (glist.c:678) ==9013== by 0x7F09184: g_list_copy (glist.c:633) ==9013== by 0x370B157A64: _tp_g_list_copy_deep (util.c:2117) ==9013== by 0x4FB06C5: tpaw_connection_managers_listed_cb (tpaw-connection-managers.c:209) ==9013== by 0x7929107: g_simple_async_result_complete (gsimpleasyncresult.c:801) ==9013== by 0x7929153: complete_in_idle_cb (gsimpleasyncresult.c:813) ==9013== by 0x7F0F470: g_idle_dispatch (gmain.c:5397) ==9013== by 0x7F0CAB8: g_main_dispatch (gmain.c:3122) ==9013== by 0x7F0D8FC: g_main_context_dispatch (gmain.c:3737) ==9013== by 0x7F0DAE0: g_main_context_iterate (gmain.c:3808) ==9013== by 0x7F0DF06: g_main_loop_run (gmain.c:4002) ==9013== by 0x4F803F3: wait_for_account_settings_ready (goatelepathyprovider.c:102) ==9013== by 0x4F8127F: edit_connection_parameters (goatelepathyprovider.c:534) ==9013== by 0x4F81F16: edit_parameters_clicked_cb (goatelepathyprovider.c:835) ==9013== by 0x7C7727D: g_cclosure_marshal_VOID__VOIDv (gmarshal.c:905) ==9013== by 0x7C744D1: _g_closure_invoke_va (gclosure.c:864) ==9013== by 0x7C8F771: g_signal_emit_valist (gsignal.c:3246) ==9013== by 0x7C908E9: g_signal_emit (gsignal.c:3393) ==9013== by 0x6838A45: gtk_button_clicked (gtkbutton.c:1488) ==9013== by 0x6839966: gtk_button_do_release (gtkbutton.c:1899) ==9013== by 0x6839D45: gtk_real_button_released (gtkbutton.c:2017) ==9013== by 0x7C7727D: g_cclosure_marshal_VOID__VOIDv (gmarshal.c:905) ==9013== by 0x7C7490F: g_type_class_meta_marshalv (gclosure.c:1021) ==9013== by 0x7C744D1: _g_closure_invoke_va (gclosure.c:864) ==9013== by 0x7C8F771: g_signal_emit_valist (gsignal.c:3246) ==9013== by 0x7C908E9: g_signal_emit (gsignal.c:3393) ==9013== by 0x68364F5: multipress_released_cb (gtkbutton.c:613) ==9013== by 0x36E5A05DAF: ffi_call_unix64 (unix64.S:76) ==9013== by 0x36E5A05817: ffi_call (ffi64.c:525) ==9013== by 0x7C75F7C: g_cclosure_marshal_generic_va (gclosure.c:1594) ==9013== by 0x7C744D1: _g_closure_invoke_va (gclosure.c:864) ==9013== by 0x7C8F771: g_signal_emit_valist (gsignal.c:3246) ==9013== by 0x7C908E9: g_signal_emit (gsignal.c:3393) ==9013== by 0x69303DB: gtk_gesture_multi_press_end (gtkgesturemultipress.c:273) ==9013== by 0x7C78B90: g_cclosure_marshal_VOID__BOXEDv (gmarshal.c:1950) ==9013== by 0x7C7490F: g_type_class_meta_marshalv (gclosure.c:1021) ==9013== by 0x7C744D1: _g_closure_invoke_va (gclosure.c:864)
Created attachment 308441 [details] [review] account-widget: Fix 'default_display_name' leak do_constructed() leaks the string returned by tpaw_account_widget_get_default_display_name(): ==15713== 21 bytes in 1 blocks are definitely lost in loss record 3,415 of 16,664 ==15713== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==15713== by 0x36E2E76B57: vasprintf (vasprintf.c:76) ==15713== by 0x7F60DD2: g_vasprintf (gprintf.c:316) ==15713== by 0x7F30D70: g_strdup_vprintf (gstrfuncs.c:507) ==15713== by 0x7F30E15: g_strdup_printf (gstrfuncs.c:533) ==15713== by 0x4FAAD98: tpaw_account_widget_get_default_display_name (tpaw-account-widget.c:2192) ==15713== by 0x4FAA444: do_constructed (tpaw-account-widget.c:1960) ==15713== by 0x7C7CC10: g_object_new_internal (gobject.c:1814) ==15713== by 0x7C7D71A: g_object_new_valist (gobject.c:2033) ==15713== by 0x7C7C6C5: g_object_new (gobject.c:1617) ==15713== by 0x4FAABCC: tpaw_account_widget_new_for_protocol (tpaw-account-widget.c:2127) ==15713== by 0x4F81352: edit_connection_parameters (goatelepathyprovider.c:544)
Created attachment 308442 [details] [review] account-widget: Fix 'login_id' leak in _get_default_display_name() tpaw_account_get_default_display_name() returns early when login_id is non-NULL, and only tries to g_free() it in the codepath after this early return, which causes a leak. This commit moves the call to g_free (login_id) before the early return in the (login_id != NULL) case. This fixes: ==26855== 8 bytes in 1 blocks are definitely lost in loss record 13,180 of 34,041 ==26855== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26855== by 0x8A120AC: g_malloc (gmem.c:97) ==26855== by 0x8A123C5: g_malloc_n (gmem.c:336) ==26855== by 0x8A2DB9A: g_strdup (gstrfuncs.c:356) ==26855== by 0x8A4C05B: g_variant_dup_string (gvariant.c:1500) ==26855== by 0x4CB9B41: tpaw_account_settings_dup_string (tpaw-account-settings.c:843) ==26855== by 0x4CC0F8B: tpaw_account_widget_get_default_display_name (tpaw-account-widget.c:2155) ==26855== by 0x4CBDFE2: tpaw_account_widget_apply_and_log_in (tpaw-account-widget.c:999) ==26855== by 0x4CBE05C: account_widget_apply_clicked_cb (tpaw-account-widget.c:1017) ==26855== by 0x87741EA: g_cclosure_marshal_VOID__VOID (gmarshal.c:875) ==26855== by 0x8771238: g_closure_invoke (gclosure.c:801) ==26855== by 0x878E070: signal_emit_unlocked_R (gsignal.c:3581) ==26855== by 0x878D3A7: g_signal_emit_valist (gsignal.c:3337) ==26855== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==26855== by 0x7326A22: gtk_button_clicked (gtkbutton.c:1488) ==26855== by 0x7327943: gtk_button_do_release (gtkbutton.c:1899) ==26855== by 0x7327D22: gtk_real_button_released (gtkbutton.c:2017) ==26855== by 0x877427D: g_cclosure_marshal_VOID__VOIDv (gmarshal.c:905) ==26855== by 0x877190F: g_type_class_meta_marshalv (gclosure.c:1021) ==26855== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==26855== by 0x878C771: g_signal_emit_valist (gsignal.c:3246) ==26855== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==26855== by 0x73244D2: multipress_released_cb (gtkbutton.c:613) ==26855== by 0x36E5A05DAF: ffi_call_unix64 (unix64.S:76) ==26855== by 0x36E5A05817: ffi_call (ffi64.c:525) ==26855== by 0x8772F7C: g_cclosure_marshal_generic_va (gclosure.c:1594) ==26855== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==26855== by 0x878C771: g_signal_emit_valist (gsignal.c:3246) ==26855== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==26855== by 0x7421792: gtk_gesture_multi_press_end (gtkgesturemultipress.c:273) ==26855== by 0x8775B90: g_cclosure_marshal_VOID__BOXEDv (gmarshal.c:1950) ==26855== by 0x877190F: g_type_class_meta_marshalv (gclosure.c:1021) ==26855== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==26855== by 0x878C771: g_signal_emit_valist (gsignal.c:3246)
Created attachment 308443 [details] [review] account-settings: Always take ownership of variant in tpaw_account_settings_set In one codepath, tpaw_account_settings_set() takes ownership of the passed-in GVariant through g_variant_ref_sink, but in the other codepath, ownership is left to the caller, which is unusual with functions with GVariant parameters. This commit makes sure we unref() the passed in GVariant when we don't want to keep it around so that the caller gets the expected behaviour, and so that we don't leak memory. This fixes ==26855== 507 (240 direct, 267 indirect) bytes in 6 blocks are definitely lost in loss record 32,940 of 3 ==26855== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26855== by 0x8A120AC: g_malloc (gmem.c:97) ==26855== by 0x8A2B0F7: g_slice_alloc (gslice.c:1007) ==26855== by 0x8A52C0B: g_variant_alloc (gvariant-core.c:476) ==26855== by 0x8A52C9B: g_variant_new_from_bytes (gvariant-core.c:512) ==26855== by 0x8A4A60F: g_variant_new_from_trusted (gvariant.c:295) ==26855== by 0x8A4BAF6: g_variant_new_string (gvariant.c:1232) ==26855== by 0x4CBBE2E: account_widget_entry_changed_common (tpaw-account-widget.c:281) ==26855== by 0x4CBBED8: account_widget_entry_changed_cb (tpaw-account-widget.c:299) ==26855== by 0x87741EA: g_cclosure_marshal_VOID__VOID (gmarshal.c:875) ==26855== by 0x8771238: g_closure_invoke (gclosure.c:801) ==26855== by 0x878E070: signal_emit_unlocked_R (gsignal.c:3581) ==26855== by 0x878D3A7: g_signal_emit_valist (gsignal.c:3337) ==26855== by 0x878DA56: g_signal_emit_by_name (gsignal.c:3433) ==26855== by 0x73C0612: end_change (gtkentry.c:2928) ==26855== by 0x73C5F0D: gtk_entry_real_insert_text (gtkentry.c:5360) ==26855== by 0x7495B9A: _gtk_marshal_VOID__STRING_INT_POINTER (gtkmarshalers.c:7351) ==26855== by 0x87719A3: g_type_iface_meta_marshal (gclosure.c:1045) =
Created attachment 308444 [details] [review] avatar-chooser: Don't call clear_image() from ::init() GtkButton currently has a bug where calling gtk_button_set_image() in the ::init() vfunc of a subclass will cause the GtkImage to be leaked. This bug does not happen if the image is set after the GtkButton has been constructed, so this commit moves the call to avatar_chooser_clear_image() (which calls gtk_button_set_image()) from ::init() to ::constructed(). See https://bugzilla.gnome.org/show_bug.cgi?id=753048 This fixes: ==388== 768 (304 direct, 464 indirect) bytes in 1 blocks are definitely lost in loss record 33,655 of 34, ==388== at 0x8792D9C: g_type_create_instance (gtype.c:1849) ==388== by 0x8779AC5: g_object_new_internal (gobject.c:1774) ==388== by 0x8779FD4: g_object_newv (gobject.c:1921) ==388== by 0x8779676: g_object_new (gobject.c:1614) ==388== by 0x7453142: gtk_image_new_from_icon_name (gtkimage.c:792) ==388== by 0x4CC2CEE: avatar_chooser_clear_image (tpaw-avatar-chooser.c:394) ==388== by 0x4CC45AA: tpaw_avatar_chooser_init (tpaw-avatar-chooser.c:1195) ==388== by 0x8792E7D: g_type_create_instance (gtype.c:1870) ==388== by 0x8779AC5: g_object_new_internal (gobject.c:1774) ==388== by 0x877A71A: g_object_new_valist (gobject.c:2033) ==388== by 0x87796C5: g_object_new (gobject.c:1617) ==388== by 0x4CC4673: tpaw_avatar_chooser_new (tpaw-avatar-chooser.c:1213) ==388== by 0x4CD41D2: tpaw_user_info_constructed (tpaw-user-info.c:543) ==388== by 0x8779C10: g_object_new_internal (gobject.c:1814) ==388== by 0x877A71A: g_object_new_valist (gobject.c:2033) ==388== by 0x87796C5: g_object_new (gobject.c:1617) ==388== by 0x4CD4826: tpaw_user_info_new (tpaw-user-info.c:663) ==388== by 0x4C97906: edit_personal_details (goatelepathyprovider.c:667) ==388== by 0x4C980A7: edit_personal_details_clicked_cb (goatelepathyprovider.c:859) ==388== by 0x877427D: g_cclosure_marshal_VOID__VOIDv (gmarshal.c:905) ==388== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==388== by 0x878C771: g_signal_emit_valist (gsignal.c:3246) ==388== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==388== by 0x7326A22: gtk_button_clicked (gtkbutton.c:1488) ==388== by 0x7327943: gtk_button_do_release (gtkbutton.c:1899) ==388== by 0x7327D22: gtk_real_button_released (gtkbutton.c:2017) ==388== by 0x877427D: g_cclosure_marshal_VOID__VOIDv (gmarshal.c:905) ==388== by 0x877190F: g_type_class_meta_marshalv (gclosure.c:1021) ==388== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==388== by 0x878C771: g_signal_emit_valist (gsignal.c:3246) ==388== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==388== by 0x73244D2: multipress_released_cb (gtkbutton.c:613) ==388== by 0x36E5A05DAF: ffi_call_unix64 (unix64.S:76) ==388== by 0x36E5A05817: ffi_call (ffi64.c:525) ==388== by 0x8772F7C: g_cclosure_marshal_generic_va (gclosure.c:1594) ==388== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==388== by 0x878C771: g_signal_emit_valist (gsignal.c:3246) ==388== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==388== by 0x7421792: gtk_gesture_multi_press_end (gtkgesturemultipress.c:273) ==388== by 0x8775B90: g_cclosure_marshal_VOID__BOXEDv (gmarshal.c:1950) =
Created attachment 308445 [details] [review] account-settings: Fix 'account_req' leak tpaw_account_settings_do_create_account() creates a TpAccountRequest using tp_account_request_new() but never release its ref when it no longer needs it. Adding the missing unref() fixes the following leak: ==21491== 848 (40 direct, 808 indirect) bytes in 1 blocks are definitely lost in loss record 18,879 of 19,643 ==21491== at 0x8792D3A: g_type_create_instance (gtype.c:1848) ==21491== by 0x8779AC5: g_object_new_internal (gobject.c:1774) ==21491== by 0x877A71A: g_object_new_valist (gobject.c:2033) ==21491== by 0x87796C5: g_object_new (gobject.c:1617) ==21491== by 0x567A173: tp_account_request_new (account-request.c:750) ==21491== by 0x4CBAF7F: tpaw_account_settings_do_create_account (tpaw-account-settings.c:1382) ==21491== by 0x4CBB270: tpaw_account_settings_apply_async (tpaw-account-settings.c:1464) ==21491== by 0x4CBE023: tpaw_account_widget_apply_and_log_in (tpaw-account-widget.c:1009) ==21491== by 0x4CBE042: account_widget_apply_clicked_cb (tpaw-account-widget.c:1017) ==21491== by 0x87741EA: g_cclosure_marshal_VOID__VOID (gmarshal.c:875) ==21491== by 0x8771238: g_closure_invoke (gclosure.c:801) ==21491== by 0x878E070: signal_emit_unlocked_R (gsignal.c:3581) ==21491== by 0x878D3A7: g_signal_emit_valist (gsignal.c:3337) ==21491== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==21491== by 0x7326A22: gtk_button_clicked (gtkbutton.c:1488) ==21491== by 0x7327943: gtk_button_do_release (gtkbutton.c:1899) ==21491== by 0x7327D22: gtk_real_button_released (gtkbutton.c:2017) ==21491== by 0x877427D: g_cclosure_marshal_VOID__VOIDv (gmarshal.c:905) ==21491== by 0x877190F: g_type_class_meta_marshalv (gclosure.c:1021) ==21491== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==21491== by 0x878C771: g_signal_emit_valist (gsignal.c:3246) ==21491== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==21491== by 0x73244D2: multipress_released_cb (gtkbutton.c:613) ==21491== by 0x36E5A05DAF: ffi_call_unix64 (unix64.S:76) ==21491== by 0x36E5A05817: ffi_call (ffi64.c:525) ==21491== by 0x8772F7C: g_cclosure_marshal_generic_va (gclosure.c:1594) ==21491== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==21491== by 0x878C771: g_signal_emit_valist (gsignal.c:3246) ==21491== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==21491== by 0x7421792: gtk_gesture_multi_press_end (gtkgesturemultipress.c:273) ==21491== by 0x8775B90: g_cclosure_marshal_VOID__BOXEDv (gmarshal.c:1950) ==21491== by 0x877190F: g_type_class_meta_marshalv (gclosure.c:1021) ==21491== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==21491== by 0x878C771: g_signal_emit_valist (gsignal.c:3246) ==21491== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==21491== by 0x741D17F: _gtk_gesture_set_recognized (gtkgesture.c:275) ==21491== by 0x741D28D: _gtk_gesture_check_recognized (gtkgesture.c:315) ==21491== by 0x741DBD6: gtk_gesture_handle_event (gtkgesture.c:623) ==21491== by 0x7423555: gtk_gesture_single_handle_event (gtkgesturesingle.c:220) ==21491== by 0x73DBD1E: gtk_event_controller_handle_event (gtkeventcontroller.c:224) =
Created attachment 308446 [details] [review] account-settings: Fix GSimpleAsyncResult leaks tpaw_account_setting_set_display_name_async() and tpaw_account_settings_set_icon_name_async() can return immediatly in an idle when they don't have work to do. However in that case, these methods don't drop the reference they own on the GSimpleAsyncResult used for the async operation. This commit adds the missing g_object_unref() calls.
Review of attachment 308440 [details] [review]: ++
Review of attachment 308441 [details] [review]: ++
Review of attachment 308442 [details] [review]: ++
Review of attachment 308443 [details] [review]: Thanks for catching this, Christophe. Nitpick: the subject of the commit message exceeds 72 characters. Maybe consider dropping the prefix. ::: tp-account-widgets/tpaw-account-settings.c @@ +1033,3 @@ g_free (settings->priv->password); settings->priv->password = g_variant_dup_string (v, NULL); + g_variant_unref (v); I think we should only unref it if the GVariant is floating. Otherwise, we would be trespassing if the caller had explicitly kept a reference to the GVariant for its own use. So, we either need a g_variant_is_floating check, or we need to add a g_variant_ref_sink at the beginning of the branch.
Review of attachment 308444 [details] [review]: Although it smells like a workaround for the gtk+ leak, it doesn't hurt to make the code robust across different gtk+ versions. ::: tp-account-widgets/tpaw-avatar-chooser.c @@ +187,3 @@ G_OBJECT_CLASS (tpaw_avatar_chooser_parent_class)->constructed (object); + avatar_chooser_clear_image (self); Could you please add a comment pointing to the gtk+ bug? Otherwise, it is too innocuous for someone to realize the importance of keeping this statement here. I, myself, wouldn't think of checking the git history before touching this line. :)
Review of attachment 308445 [details] [review]: ++
Review of attachment 308446 [details] [review]: ++
Thanks for tracking down these leaks, Christophe.
(In reply to Debarshi Ray from comment #25) > Thanks for tracking down these leaks, Christophe. I'm just the messenger, the guy who did most of the work is called Valgrind ;)
Review of attachment 308444 [details] [review]: Yes it's definitely a workaround for that gtk+ bug, but it's simple enough that it doesn't hurt to have it, especially as it avoids head scratching when running in valgrind ;) ::: tp-account-widgets/tpaw-avatar-chooser.c @@ +187,3 @@ G_OBJECT_CLASS (tpaw_avatar_chooser_parent_class)->constructed (object); + avatar_chooser_clear_image (self); Good point, added now.
Created attachment 308484 [details] [review] avatar-chooser: Don't call clear_image() from ::init() GtkButton currently has a bug where calling gtk_button_set_image() in the ::init() vfunc of a subclass will cause the GtkImage to be leaked. This bug does not happen if the image is set after the GtkButton has been constructed, so this commit moves the call to avatar_chooser_clear_image() (which calls gtk_button_set_image()) from ::init() to ::constructed(). See https://bugzilla.gnome.org/show_bug.cgi?id=753048 This fixes: ==388== 768 (304 direct, 464 indirect) bytes in 1 blocks are definitely lost in loss record 33,655 of 34, ==388== at 0x8792D9C: g_type_create_instance (gtype.c:1849) ==388== by 0x8779AC5: g_object_new_internal (gobject.c:1774) ==388== by 0x8779FD4: g_object_newv (gobject.c:1921) ==388== by 0x8779676: g_object_new (gobject.c:1614) ==388== by 0x7453142: gtk_image_new_from_icon_name (gtkimage.c:792) ==388== by 0x4CC2CEE: avatar_chooser_clear_image (tpaw-avatar-chooser.c:394) ==388== by 0x4CC45AA: tpaw_avatar_chooser_init (tpaw-avatar-chooser.c:1195) ==388== by 0x8792E7D: g_type_create_instance (gtype.c:1870) ==388== by 0x8779AC5: g_object_new_internal (gobject.c:1774) ==388== by 0x877A71A: g_object_new_valist (gobject.c:2033) ==388== by 0x87796C5: g_object_new (gobject.c:1617) ==388== by 0x4CC4673: tpaw_avatar_chooser_new (tpaw-avatar-chooser.c:1213) ==388== by 0x4CD41D2: tpaw_user_info_constructed (tpaw-user-info.c:543) ==388== by 0x8779C10: g_object_new_internal (gobject.c:1814) ==388== by 0x877A71A: g_object_new_valist (gobject.c:2033) ==388== by 0x87796C5: g_object_new (gobject.c:1617) ==388== by 0x4CD4826: tpaw_user_info_new (tpaw-user-info.c:663) ==388== by 0x4C97906: edit_personal_details (goatelepathyprovider.c:667) ==388== by 0x4C980A7: edit_personal_details_clicked_cb (goatelepathyprovider.c:859) ==388== by 0x877427D: g_cclosure_marshal_VOID__VOIDv (gmarshal.c:905) ==388== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==388== by 0x878C771: g_signal_emit_valist (gsignal.c:3246) ==388== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==388== by 0x7326A22: gtk_button_clicked (gtkbutton.c:1488) ==388== by 0x7327943: gtk_button_do_release (gtkbutton.c:1899) ==388== by 0x7327D22: gtk_real_button_released (gtkbutton.c:2017) ==388== by 0x877427D: g_cclosure_marshal_VOID__VOIDv (gmarshal.c:905) ==388== by 0x877190F: g_type_class_meta_marshalv (gclosure.c:1021) ==388== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==388== by 0x878C771: g_signal_emit_valist (gsignal.c:3246) ==388== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==388== by 0x73244D2: multipress_released_cb (gtkbutton.c:613) ==388== by 0x36E5A05DAF: ffi_call_unix64 (unix64.S:76) ==388== by 0x36E5A05817: ffi_call (ffi64.c:525) ==388== by 0x8772F7C: g_cclosure_marshal_generic_va (gclosure.c:1594) ==388== by 0x87714D1: _g_closure_invoke_va (gclosure.c:864) ==388== by 0x878C771: g_signal_emit_valist (gsignal.c:3246) ==388== by 0x878D8E9: g_signal_emit (gsignal.c:3393) ==388== by 0x7421792: gtk_gesture_multi_press_end (gtkgesturemultipress.c:273) ==388== by 0x8775B90: g_cclosure_marshal_VOID__BOXEDv (gmarshal.c:1950)
Created attachment 308485 [details] [review] account-settings: Always take ownership of variant in ::set() In one codepath, tpaw_account_settings_set() takes ownership of the passed-in GVariant through g_variant_ref_sink, but in the other codepath, ownership is left to the caller, which is unusual with functions with GVariant parameters. This commit unconditionnally calls g_variant_sink_ref() on the GVariant arg rather than doing it in just one codepath, and then makes sure we unref() the passed in GVariant when we don't want to keep it around. This fixes a memory leak in tpaw_account_settings_set() This fixes ==26855== 507 (240 direct, 267 indirect) bytes in 6 blocks are definitely lost in loss record 32,940 of 3 ==26855== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26855== by 0x8A120AC: g_malloc (gmem.c:97) ==26855== by 0x8A2B0F7: g_slice_alloc (gslice.c:1007) ==26855== by 0x8A52C0B: g_variant_alloc (gvariant-core.c:476) ==26855== by 0x8A52C9B: g_variant_new_from_bytes (gvariant-core.c:512) ==26855== by 0x8A4A60F: g_variant_new_from_trusted (gvariant.c:295) ==26855== by 0x8A4BAF6: g_variant_new_string (gvariant.c:1232) ==26855== by 0x4CBBE2E: account_widget_entry_changed_common (tpaw-account-widget.c:281) ==26855== by 0x4CBBED8: account_widget_entry_changed_cb (tpaw-account-widget.c:299) ==26855== by 0x87741EA: g_cclosure_marshal_VOID__VOID (gmarshal.c:875) ==26855== by 0x8771238: g_closure_invoke (gclosure.c:801) ==26855== by 0x878E070: signal_emit_unlocked_R (gsignal.c:3581) ==26855== by 0x878D3A7: g_signal_emit_valist (gsignal.c:3337) ==26855== by 0x878DA56: g_signal_emit_by_name (gsignal.c:3433) ==26855== by 0x73C0612: end_change (gtkentry.c:2928) ==26855== by 0x73C5F0D: gtk_entry_real_insert_text (gtkentry.c:5360) ==26855== by 0x7495B9A: _gtk_marshal_VOID__STRING_INT_POINTER (gtkmarshalers.c:7351) ==26855== by 0x87719A3: g_type_iface_meta_marshal (gclosure.c:1045)
Review of attachment 308484 [details] [review]: ++
Review of attachment 308485 [details] [review]: ++
Attachment 308440 [details] pushed as a46f431 - Properly fix tp_list_connection_managers_finish() leak Attachment 308441 [details] pushed as 568fd67 - account-widget: Fix 'default_display_name' leak Attachment 308442 [details] pushed as a72609e - account-widget: Fix 'login_id' leak in _get_default_display_name() Attachment 308445 [details] pushed as f4738e4 - account-settings: Fix 'account_req' leak Attachment 308446 [details] pushed as 906c887 - account-settings: Fix GSimpleAsyncResult leaks Attachment 308484 [details] pushed as 45e652f - avatar-chooser: Don't call clear_image() from ::init() Attachment 308485 [details] pushed as ef1e65f - account-settings: Always take ownership of variant in ::set()