GNOME Bugzilla – Bug 339791
Gtk::Menu crash with child
Last modified: 2007-04-16 13:17:59 UTC
See the test case.
Created attachment 64305 [details] menucrash2.cc This test case is a version of one that was originally in bug #104194. It shows this output on std::out: debug: before deleting parent. destructor for menu: parent debug: after deleting parent. debug: before deleting child. destructor for menu: child (a.out:2140): GLib-GObject-WARNING **: instance of invalid non-instantiatable type `(null)' (a.out:2140): GLib-GObject-CRITICAL **: g_signal_handlers_disconnect_matched: assertion `G_TYPE_CHECK_INSTANCE (instance)' failed (a.out:2140): Gtk-CRITICAL **: gtk_menu_item_detacher: assertion `GTK_IS_MENU_ITEM (widget)' failed (a.out:2140): GLib-GObject-WARNING **: invalid uninstantiatable type `(null)' in cast to `GObject' (a.out:2140): GLib-GObject-CRITICAL **: g_object_steal_data: assertion `G_IS_OBJECT (object)' failed (a.out:2140): GLib-GObject-WARNING **: invalid uninstantiatable type `(null)' in cast to `GObject' (a.out:2140): GLib-GObject-CRITICAL **: g_object_set_data: assertion `G_IS_OBJECT (object)' failed debug: after deleting child.
Here is the gdb backtrace with --g-fatal-warnings. valgrind's memcheck doesn't show any double-deletion, but valgrind might not work properly with the latest GTK+. Andrew, maybe you could try valgrind on your system. Program received signal SIGTRAP, Trace/breakpoint trap. IA__g_logv (log_domain=<value optimized out>, log_level=G_LOG_LEVEL_WARNING, format=0xb770027c "instance of invalid non-instantiatable type `%s'", args1=0xbfb2808c "") at gmessages.c:503 503 g_private_set (g_log_depth, GUINT_TO_POINTER (depth)); (gdb) bt
+ Trace 67865
Here is first part of valgrind output. Fedore Core 4, GTKMM-2.4, GTK+-2.4 Guess, it is obvious that child was deleted with gtk_container_foreach(). valgrind --tool=memcheck --num-callers=100 menutest ==8677== Memcheck, a memory error detector for x86-linux. ==8677== Copyright (C) 2002-2005, and GNU GPL'd, by Julian Seward et al. ==8677== Using valgrind-2.4.0, a program supervision framework for x86-linux. ==8677== Copyright (C) 2000-2005, and GNU GPL'd, by Julian Seward et al. ==8677== For more details, rerun with: -v ==8677== debug: before deleting parent. destructor for menu: parent debug: after deleting parent. debug: before deleting child. destructor for menu: child ==8677== Invalid read of size 4 ==8677== at 0x2BDC3B: g_type_check_instance (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2B9CF8: g_signal_handlers_disconnect_matched (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x526415: gtk_menu_detach (in /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x527A21: (within /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x1BA6A6EB: Gtk::Container_Class::destroy_callback(_GtkObject*) (container.cc:160) ==8677== by 0x2B17E6: g_cclosure_marshal_VOID__VOID (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2A5D9A: (within /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2A6284: g_closure_invoke (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2B4C65: (within /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2B5EAF: g_signal_emit_valist (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2B6222: g_signal_emit (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x53D1F0: (within /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x600614: (within /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x1BAE42C4: Gtk::Widget_Class::dispose_vfunc_callback(_GObject*) (widget.cc:508) ==8677== by 0x2A7C53: g_object_unref (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x1BA96730: Gtk::Object::_destroy_c_instance() (object.cc:134) ==8677== by 0x1BA9680E: Gtk::Object::destroy_() (object.cc:264) ==8677== by 0x1BA8D262: Gtk::Menu::~Menu() (menu.cc:201) ==8677== by 0x804C62F: MyMenu::~MyMenu() (menucrash2.cc:15) ==8677== by 0x804C915: MyBar::~MyBar() (menucrash2.cc:44) ==8677== by 0x804C08F: main (menucrash2.cc:82) ==8677== Address 0x1C033D70 is 0 bytes inside a block of size 96 free'd ==8677== at 0x1B909743: free (vg_replace_malloc.c:152) ==8677== by 0x314BC23: g_free (in /usr/lib/libglib-2.0.so.0.600.6) ==8677== by 0x2C2ADC: g_type_free_instance (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2A7C49: g_object_unref (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2A7CBB: g_object_run_dispose (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x53D1AC: gtk_object_destroy (in /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x5FB99D: gtk_widget_destroy (in /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x52F312: (within /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x1BA6AAFC: Gtk::Container_Class::forall_vfunc_callback(_GtkContainer*, int, void (*)(_GtkWidget*, void*), void*) (container.cc:396) ==8677== by 0x49E3ED: gtk_container_foreach (in /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x4A0150: (within /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x527A90: (within /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x1BA6A6EB: Gtk::Container_Class::destroy_callback(_GtkObject*) (container.cc:160) ==8677== by 0x2B17E6: g_cclosure_marshal_VOID__VOID (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2A5D9A: (within /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2A6284: g_closure_invoke (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2B4C65: (within /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2B5EAF: g_signal_emit_valist (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x2B6222: g_signal_emit (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x53D1F0: (within /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x600614: (within /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x1BAE42C4: Gtk::Widget_Class::dispose_vfunc_callback(_GObject*) (widget.cc:508) ==8677== by 0x2A7CB3: g_object_run_dispose (in /usr/lib/libgobject-2.0.so.0.600.6) ==8677== by 0x53D1AC: gtk_object_destroy (in /usr/lib/libgtk-x11-2.0.so.0.600.10) ==8677== by 0x1BA966ED: Gtk::Object::_destroy_c_instance() (object.cc:166) ==8677== by 0x1BA9680E: Gtk::Object::destroy_() (object.cc:264) ==8677== by 0x1BA8D262: Gtk::Menu::~Menu() (menu.cc:201) ==8677== by 0x804C62F: MyMenu::~MyMenu() (menucrash2.cc:15) ==8677== by 0x804C8A3: MyBar::~MyBar() (menucrash2.cc:40) ==8677== by 0x804C08F: main (menucrash2.cc:82) (menutest:8677): GLib-GObject-WARNING **: instance with invalid (NULL) class pointer (menutest:8677): GLib-GObject-CRITICAL **: g_signal_handlers_disconnect_matched: assertion `G_TYPE_CHECK_INSTANCE (instance)' failed
(In reply to comment #3) > Here is first part of valgrind output. Fedore Core 4, GTKMM-2.4, GTK+-2.4 > Guess, it is obvious that child was deleted with gtk_container_foreach(). > Hmmh, I am not quite sure. Actually deleting the parent menu does not delete the child menu. The problem arises when the test code does 'delete m_child' ; After a couple of gdb-ing, here is what I think happens during delete m_child: delete m_child somehow calls gtk_menu_destroy() on the child menu. At this point, everything is fine. Then, the gtk_menu_destroy() calls gtk_menu_detach() to apparently detach the child from it parent ... bad thing. Remember, the parent has already been destroyed ... It seems GtkMenu has not been designed in a way that allow a parent menu to be destroyed *before* its children. I am not sure this is a bug from Gtkmm then. To reproduce what I saw in gdb, try to run the sample code in gdb and put a break point in gtk_menu_destroy and gtk_menu_detach. Cheers Dodji.
(In reply to comment #4) > (In reply to comment #3) > > Here is first part of valgrind output. Fedore Core 4, GTKMM-2.4, GTK+-2.4 > > Guess, it is obvious that child was deleted with gtk_container_foreach(). > > > Hmmh, I am not quite sure. > Actually deleting the parent menu does not delete the child menu. > The problem arises when the test code does 'delete m_child' ; > > After a couple of gdb-ing, here is what I think happens during delete m_child: > delete m_child somehow calls gtk_menu_destroy() on the child menu. > At this point, everything is fine. > Then, the gtk_menu_destroy() calls gtk_menu_detach() to apparently detach > the child from it parent ... bad thing. > > Remember, the parent has already been destroyed ... > > It seems GtkMenu has not been designed in a way that allow a parent menu > to be destroyed *before* its children. I am not sure this is a bug from > Gtkmm then. > ok, then, If we aren't going to handle this with gtkmm functionality what should be done? 1. Make a note in documentation about order of menu child destructors. 2. Assign issue to GTK to let them check and fix it. I am not sure that 2nd way even has a place here, because GTK programmers never will go that way of menu object destruction as it is possible in GTKMM. Regards, -andrew
If this is indeed the problem (I have not yet investigated) then we would want to either a) Fix GTK+ b) Do some kind of workaround in gtkmm. Simply documenting a problem would be the least desirable solution.
Created attachment 65656 [details] Slightly altered test case. I altered the test case for this bug slightly. #1 I added a managed widget to the mix to make sure my changes weren't preventing managed widgets from being deleted properly. #2 I added a second un-manage()ed widget to the scheme to make sure my changes wouldn't choke on something that wasn't a menu.
Created attachment 65657 [details] [review] Patch for this bug The patch I came up with for this bug. Changes the Gtk::Menu_Helpers::MenuElem destructor to a custom implementation. Adds a simple check, if( this->has_submenu() ) this->remove_submenu() ; And thats that. I think it took me longer to get gtkmm built from source than to figure out the patch.
Sounds sensible, though I'd rather know why the submenu doesn't get informed of the death of its parent, and most sub-widgets do. Maybe it's a timing/sequence thing. At the least, we need a comment explaining why we do the remove, even if it's just the URL of bug. I think we have a similar solution/hack for Container (maybe not any more). Maybe submenus are nore listed like regular children.
A little more probing reveals: Submenus seem to be unconditionally destroyed. Thus negating all existance. Or at least some pretty big assumptions. check out: gtk+/gtk/gtkmenuitem.c Line: 296ish in gtk_menu_destroy if (menu_item->submenu) gtk_widget_destroy (menu_item->submenu); It would appear to be deleting the C object out from under the C++ object. Then when we delete our Menu, it tries to detach itself, but gets caught up by having a dead C object. Granted, I could be out in left field here too. I'm assuming that gtk_menu_item_destroy is being called when a Gtk::MenuItem is destroyed. And gtk_menu_item_destroy appears to be an internal API and I can't find any docs on whether this is expected behavior or not, so I'm not entirely sure whether to file this as a bug or pat someone on the back for writing it correctly.
> Submenus seem to be unconditionally destroyed This is also true of GtkContainer children in general. However, we prevent the destruction from actually happening. I think it's in some *dispose* vfunc/callback somewhere. Maybe it's not working properly for the sub menu.
I think the relevant part might be: " void Widget_Class::dispose_vfunc_callback(GObject* self) { ... // Remove the widget from its parent container so that it // won't be destroyed later by gtk_container_destroy(). ... " But maybe there's something else too. We changed how we did this at least once, so I get confused.
Aha! I think.... Gtk::MenuItem is a container. Duh. What MenuItem doesn't have is a custom remove() method that removes the reference to its submenu. I've added some g_warnings to track this down. In my custom destructor, we're still trying to destroy the submen of the unmanaged widget. But the problem is, void Gtk::Bin::remove( Widget& w ) ; is not virtual, so isn't it bad to override that? I saw virual void Gtk::Bin::on_remove( Widget& w ) ; But I can't find an example else where on how to implement this with all this crazy m4 stuff.
> Gtk::MenuItem is a container Yeah, but Gtk::Menu isn't a Gtk::MenuItem. I think it's more relevant that Gtk::Menu is a Gtk::Container. I don't see Gtk::Bin anywhere in the hierarchy. > What MenuItem doesn't have is a custom remove() method that removes the reference to its submenu. I'd expect GtkMenu to take care of making gtk_container_remove(submenu) actually remove the sub menu, possibly via some vfunc. Maybe it uses the GtkContainer::"remove"signal, but that seems odd.
By gtk_container_remove(submenu), I mean gtk_container_remove(menu, submenu).
Another aha! gtkmm/gtk/gtkmm/menu_elems.cc:93-96 MenuElem::MenuElem(const Glib::ustring& label, Menu& submenu) { set_child( manage(new MenuItem(label, true)) ); child_->set_submenu(submenu); child_->show(); } We're talking about containers, but we're not in a container. Or rather, this is not a container interface we're dealing with. In Gtk::MenuItem, the submenu is stored as a member variable. Its not stored with a container interface where we can just remove() it, we have to call remove_submenu() to get rid of the reference before we destroy it. The Widget_Class::dispose_vfunc_callback would remove the Gtk::Label if it happened to be unmanaged, but not touch the submenu. Now the question is, why does our managed menu get destroyed properly? And I cannot answer this. All I can say is that the MenuItem destructor never seems to get run. Which I find odd. But from my g_warning outputs it doesn't appear to be running.
You've certainly given me some stuff to investigate. If you are right about the problem, it looks like we can find a solution.
Oh, and the hiererarchy as best as I can tell for the unamanged widget: Gtk::Window -> Gtk::MenuBar -> Gtk::Menu ( m_parent ) -> Gtk::MenuElem -> Gtk::MenuItem ( member of MenuElem ) //This is a member variable, not a container relationship. //I'm pretty sure there's quite a difference, but I might be wrong. -> Gtk::Menu ( m_child1 ) Well, crap. Looking at this makes me think I'm wrong again. Damn it. (This is a run of my test case. Kind of. I dunno, but its got a g_warning in Widget_Class::dispose_vfunc_callback(). I copied one of the DEBUG_REFCOUNT warning message things to right below if( obj->referenced_ ) to see what pointer was being removed from what container. And the pointer for the First Child is definitely being removed from something. Athough, notice it then gets removed twice from nothing. Well I suppose those could be all the weird call back signals or what not. First Child: 0x80b4050 Second Child: 0x80b4150 Parent: 0x80b4250 BEFORE DELETE PARENT destructor for menu: parent (test:9256): gtkmm-WARNING **: REMOVING AN UNMANAGED WIDGET gtkmm__GtkMenu This line says we are in fact removing the Menu from some goddamn container. (test:9256): gtkmm-WARNING **: Widget_Class::dispose_vfunc_callback(): removing gobject_: 0x80b4050 from parent: 0x80b6800 (test:9256): gtkmm-WARNING **: REMOVING AN UNMANAGED WIDGET gtkmm__GtkMenu (test:9256): gtkmm-WARNING **: Widget_Class::dispose_vfunc_callback(): removing gobject_: 0x80b4050 from parent: (nil) (test:9256): gtkmm-WARNING **: Destroying object: 0x80b8468 (test:9256): gtkmm-WARNING **: Refcount: 1 (test:9256): gtkmm-WARNING **: Would've destroyed submenu: 0x80b4050 (test:9256): gtkmm-WARNING **: REMOVING AN UNMANAGED WIDGET gtkmm__GtkMenu (test:9256): gtkmm-WARNING **: Widget_Class::dispose_vfunc_callback(): removing gobject_: 0x80b4050 from parent: (nil) destructor for menu: Second child (test:9256): gtkmm-WARNING **: Destroying object: 0x80b84c8 AFTER DELETE PARENT ... So I did some more poking, and I'm finding that when m_child1 is removed from the container, it never detaches itself. I think I might have another line of inquiry. I wonder if when a Menu is removed from a container, it should be detached. So, judging by some quick reading of the code, this could be done by listening to on_parent_change and if the new parent is NULL, detach. Tried this quick by overriding on_parent_changed in Gtk::Menu. Didn't work. Doesn't look like it got called. :( There is something to this though I do believe. gtk_menu_detach is failing because its being called after the MenuItem is destroyed Judging from this: (test:27650): Gtk-CRITICAL **: gtk_menu_item_detacher: assertion `GTK_IS_MENU_ITEM (widget)' failed Which was part of the original output of the error program ( before my attempt at a patch ) So, the issue here is that when Gtk::Menu is removed from its container, it should figure out to detach itself somehow. I could check this real quick to see if it happens for any container or not. If this happens for any container, my patch is dead. And if it happens from any container, Gtk::Menu is going to have to detect when its been removed, which as far as I can tell is by on_parent_changed, cause thats all I see being called in the Bin::remove( Widget& w ) and Box::remove( Widget& w ) (just two random ones with different implementations for sampling). But my mind is absolutely fried, so this is it for tonight, today, whatever it is now. Hope that helps.
Created attachment 65915 [details] New test case. Adding the method below to the class derived from Gtk::Menu fixes the problem. virtual void on_parent_changed( Gtk::Widget* old_parent ) { fprintf( stderr, "Parent changed.\n" ) ; detach() ; }
That's a neat solution, and it makes sense. But maybe it should be done in GTK+ instead of gtkmm. Also, overrides of default signal handlers should usually call the base class implementation, or document why they choose not to.
So, while trying to make a C test case, I am wondering 1. Where do we attach() the menu item? 2. What does attach/detach mean for a menu item?
I still hope that someone finds the time to figure this out before I get to it.
Created attachment 85921 [details] Plain C test case This is a C test case for the problem. Destroying the child after the parent will result in a Gtk-Critical unless we ref() the child. Looks like this would be the expected behaviour for Gtk+. => Paul patch looks like a reasonable solution because the case is equal to the GtkContainer special case IMHO.
OK, after two hours of debugging I think I know what happens: The child_submenu is destroyed because gtk_menu_item_destroy destroys it unconditionally as Paul already pointed out. The menuitem is destroyed because the GtkContainer (parent of the menushell/menu) is destroyed. The GtkMenuShell uses the GtkContainer interface to manage its GtkMenuItems and thus all GtkMenuItems part of a Menu are destroyed once the GtkMenu is destroyed and the MenuItems destroy their submenus. The questions: So, the submenu is dead before we call "delete m_child"! So either we prevent Gtk::MenuItem from destroying its submenu or we have to document this somehow. Of course we can, as Paul already showed, detach the submenu before the Gtk::MenuItem is destroyed but what about the 99% cases were we would want that the submenu is destroyed with the parent?
Its been quite a while, but I thought calling detach() would take care of automatically deleting when the submenu was managed. Let me iterate its been a long time since I looked at this though.
Created attachment 86205 [details] [review] Patch for the issue This patch is based on Paul's work and uses the on_parent_changed()-vfunc of Gtk::Menu to ensure the Menu is not destroyed together with it's MenuItem. I tested this will all testcases that are available here and also checked if it works with Gtk::manage(). Everything worked as expected so I think this solution would be OK. As I explained earlier I don't think that changing the behaviour in Gtk+ would make sense.
There's something not right here... I've spent a couple hours reading up on what was actually causing this bug. If I have everything right in my head, the bug can be boiled down to this: GtkMenuItem unconditionally destroy's its submenu. Have we brought this up on the Gtk list to see what their response was? My feeling is that the submenu in GtkMenuItem should be treated like a container/child relationship, but the way the Gtk code is written, it appears to be more of a sole owner relationship. The more I think about it the more I dislike the Gtk::Menu::on_parent_changed() patch. It would seem to me that we're implicitly assuming that the parent will always be a Gtk::MenuItem. I can't point to a specific test case, but it seems like this assumption could bite us in the ass when its invalid. First things first, I think we should bring this up on the Gtk list and see what they have to say. Maybe its a bug and it'll get fixed there. If its expected behavior, then I'm thinking that we should be looking at a patch for the Gtk::MenuItem destructor, which could be a call to submenu->detach() or something. Anyway, those are my thoughts for right now.
Hi Paul! Yes, I see your point! I have written a mail to gtk-devel-list (CC'd you), we will see what they say. A menu can be attached to any widget but I assume no widget other than GtkMenuItem destroys the menu.
Created attachment 86232 [details] [review] Patch to add a new special case for Gtk::Menu to Widget_Class::dispose_vfunc_callback() Tim Janik pointed out on gtk-devel-list that the behaviour in Gtk+ is correct and consistent. At least it works like GtkContainer works but with the small difference that gtk_menu_attach_to_widget cannot set widget->parent to the widget it is attached to because it needs a GtkWindow as parent to be able to popup. So we need a new special case in Widget_Class::dispose_vfunc_callback to be able to catch the Gtk::Menu case. I decided to use Widget_Class and not Menu_Class because a) I did not get it working correctly with Menu_Class b) more important: Using Menu_Class might break ABI
else if (GTK_IS_MENU(pWidget) && gtk_menu_get_attach_widget(GTK_MENU(pWidget)) != NULL) Shouldn't we make this: if( GTK_IS_MENU( pWidget ) && GTK_IS_MENU_ITEM( gtk_menu_get_attach_widget( pWidget ) ) ) Granted I have no idea how robust GTK_IS_MENU_ITEM is to passing in NULL or garbage pointers, but again, it seems like we should be testing for the specific case of being attached to a MenuItem. Also, in the source comment, you might want to add a line to the effect of: Gtk::Menu does not use a parent widget because it must be contained in its Gtk::Window so that it can be displayed as a Popup. Other than that, I'm satisified.
The question is what other widgets do with their menus. Usually they should also destroy them for consistence with other GtkContainer-like widgets but I am not sure that they do. Will it harm if we detach the menu before it is destroyed?
I did a quick google through the gtk sources to see if there were any other places where submenu's were getting destroyed. grep "gtk_widget_destroy.*(.*submenu.*)" /usr/local/src/gnome/gtk+/gtk/* And it only pulled up the line from gtkmenuitem.c so I think this is at least some evidence that its the only place to worry about for now. I'm not certain if it'd harm anything or not. I do know that we know that we want to detach non-managed widgets from menuitems, and without any idea on what happens anywhere else, I would feel most comfortable making this patch as specific to this case as possible. Then if we find in the future that there are other corner casese, we can relax this patch to include those cases as well.
This looks like the ideal solution. I am now much happier that we understand the problem and solution properly. Please apply the patch to svn trunk. Well done all. Paul's specialization sounds wise. And I'd like his extra comments to be in the source. As a matter of style, I'd prefer to remove the unnecessary "!= NULL".
Committed modified version of the patch - Closing the bug report.