GNOME Bugzilla – Bug 344858
gtk_menu_item_new_with_label() created widget with refcount 2
Last modified: 2011-08-24 12:17:44 UTC
This code: m = gtk_menu_item_new (); g_print ("gtk_menu_item_new has %d references\n", G_OBJECT (m)->ref_count); m = gtk_menu_item_new_with_label ("foo"); g_print ("gtk_menu_item_new_with_label has %d references\n", G_OBJECT (m)->ref_count); Produces: gtk_menu_item_new has 1 references gtk_menu_item_new_with_label has 2 references The second reference is due to the accel label holding a reference on its parent (the menu item itself). However won't this stop the idiom of gtk_container_remove (menu, menuitem) destroying the widget unless the programmer explicitly kept a reference from working?
Basically, this leaks depending on how the item is created, if it's with_label() it leaks, if it's created with gtk_menu_item_new() it doesn't. item = gtk_menu_item_new_with_label ("foo"); gtk_container_add (GTK_CONTAINER(menu), item); gtk_container_remove (GTK_CONTAINER(menu), item); Maybe accel_label should use weak refs?
I should clarify that when I said "gtk_container_remove (menu, menuitem) destroying the widget" I meant the idiom of the containing unrefing the widget and thus reducing the refcount to 0, and causing finalisation. I don't mean explicity gtk_widget_destroy().
using weak references for accel labels sounds reasonable. are you going to cook up a patch for this? note that the accel closure has to be properly disconencted/unset when the accel wiodget vanishes.
Not immediately, no. I think for our case using gtk_widget_destroy() is semantically more correct and will the issue, for now.
*** Bug 376443 has been marked as a duplicate of this bug. ***
This bug still exists in gtk+-3.0, it can be checked with the test attached. I see this bug marked as gnome-love bug and I want to try to fix it :) If I understood correcty I should move g_signal_handlers_disconnect_by_func to a GWeakNotify callback and removing the _ref/_unref thing, right? A patch, against gtk+-3 master branch, implementing it is attached.
Created attachment 173865 [details] Test program
Created attachment 173866 [details] [review] Patch using weak refs
Review of attachment 173866 [details] [review]: Patch looks reasonable to me.
(In reply to comment #9) > Review of attachment 173866 [details] [review]: > > Patch looks reasonable to me. FYI, the patch still works on gtk+ master (with two hunks, but it is valid).