Re: GtkPlug issue ...

[Owen Taylor <otaylor redhat com>]

Michael Meeks <michael ximian com> writes:

> Hi there,
> 
> 	I'm having a rather evil issue - that we've seen before with the
> correct freeing of Gdk resources associated with plug / sockets. Last time
> the fix was to hoist the unrealize chaining above the local releasing of
> the plug / socket resources in the plug->unrealize handler.
>
> 	I assume this had the effect of ensuring that widget->window was
> freed before it's parent was. Either way; even having done that I get a
> strack trace a little like this:
> 
> (gdb) bt
> #0  _gdk_window_destroy_hierarchy (window=0x460befac, recursing=0,
> foreign_destroy=0) at gdkwindow.c:308
> #1  0x40305f63 in gdk_window_destroy (window=0x460befac) at

[...]

> #11 0x40256ff5 in gtk_widget_unrealize (widget=0x460a0f5c) at
> gtkwidget.c:1954
> #12 0x4025d5f4 in gtk_widget_dispose (object=0x460a0f5c) at
> gtkwidget.c:5314
> #13 0x40261892 in gtk_window_dispose (object=0x460a0f5c) at
> gtkwindow.c:1374
> #14 0x40747d85 in g_object_run_dispose (object=0x460a0f5c) at
> gobject.c:469
> #15 0x401b2976 in gtk_object_destroy (object=0x460a0f5c) at
> gtkobject.c:357
> #16 0x40256379 in gtk_widget_destroy (widget=0x460a0f5c) at
> gtkwidget.c:1592
> #17 0x402691f7 in gtk_plug_filter_func (gdk_xevent=0xbffff2f0,
> event=0x418fc018, data=0x460a0f5c) at gtkplug.c:1088
> #18 0x40313c4f in gdk_event_apply_filters (xevent=0xbffff2f0,
> event=0x418fc018, filters=0x40d54ae8) at gdkevents-x11.c:267

> 	So - my question is: why; I mean, if we have created some children
> of a foreign window, perhaps we don't want to go round doing a violence to
> them if the parent dies ( though probably we do ). But we should at least
> not leave stale parent pointers lying around in them; should there not be
> a little loop over the children here, unparenting them ?

Your diagnosis of the situation is correct. GtkPlug does:

  if (plug->socket_window != NULL)
    {
      gdk_window_set_user_data (plug->socket_window, NULL);
      gdk_window_unref (plug->socket_window);
      plug->socket_window = NULL;
    }

Which causes the GdkWindow for plug->socket_window to be released
before it chains to the parent:

  if (GTK_WIDGET_CLASS (parent_class)->unrealize)
    (* GTK_WIDGET_CLASS (parent_class)->unrealize) (widget);

Which actually destroys widget->window. So, waiting until after
we chain to destroy plug->socket_window would fix the problem
here.

But it's definitely not right to fix this here - it shouldn't
be possible to cause GDK to segfault by doing things in the
wrong order.

One possibility is to simply to make native children of a foreign
window have ->parent set to the root window GdkWindow. Reasons
for this:

 * We don't know all children of the foreign window. 

 * It's pretty normal for children of foreign windows to be
   reparented without our knowledge. In fact, a ReparentNotify
   for such a situation is what triggered the above backtrace!

   I don't want to have to track this in GDK.

 * If the foreign GdkWindow is created after the local child,
   the ->children pointer won't be correct.
 
The ->children pointer for a foreign window really isn't
interesting because it isn't reliably correct.

 * It corresponds to the idea that a GtkSocket is like a window
   manager frame. Typically windows with ->parent == GDK_PARENT_ROOT()
   actually have the window manager frames as their real
   X parent.

The other possibility would be to, as you suggest, to make
_gdk_window_destroy_heirarchy() clean up the parent pointers
for the children of a foreign window.

I think the former is better; I'll look at implementing the
invariant:

 GDK_WINDOW_TYPE (window->parent) != GDK_WINDOW_FOREIGN

Regards,
                                        Owen