Re: Re: GdkWindow ref counting

[Johan Bilien <johan bilien nokia com>]

ext Owen Taylor wrote:
>> i've just chased GdkWindow reference counts with the help of Mitch and Tko
>> and figured there's either a leak in Gdk or missing API docs.
>>
>> basically, what i would have expected to work and not leak are:
>> 1) w = gdk_window_new();
>>     gdk_window_destroy (w);
>> 2) w = gdk_window_foreign_new (xid);
>>     gdk_window_destroy (w);
>> 3) w = gdk_window_foreign_new();
>>     gdk_window_unref (w);
>>
>> what can be observed is that (1) and (2) both yield new gdkwindow
>> objects with ref_count==2. also, (2) and (3) are apparently leaking
>> a GdkWindow with a single reference.
>>
>> for the uninitiated,
>>    http://developer.gnome.org/doc/API/2.0/gdk/gdk-Windows.html#gdk-window-destroy
>> has the docs on why
>>     w = gdk_window_new();
>>     gdk_window_unref (w);
>> can NOT be expected to work leak-free.
>>
>> (1) appears to work, because gdk_window_destroy() does an unref and
>> an XDestroyWindow. when the corresponding DestroyNotify is released,
>> the second unref occours and the window is removed from the xid table
>> (gdk/x11/gdkwindow-x11.c:gdk_window_destroy_notify).
>>     
>
> Yes, this is exactly as intended; there is one reference held by
> GTK+ and released on destroy (much like GtkWindow), and one reference
> held by the XID table.
>
>   
>> (2) however appears to leak one reference count, because we don't
>> issue XDestroyWindow for foreign gdk windows, and thus never trigger
>> the gdk_window_destroy_notify code.
>>     
>
>  gdk_window_destroy() on a foreign window:
>    - For windows in our hierarchy, hides them, reparents them to the
>      root window, and sends them a WM_DELETE event
>    - For windows not in our heirarchy, issues XDestroyWindow()
>
> The first case is meant to handle the case of GtkPlug and similar
> embedding situations.
>
> I don't know what your test cases looked like, but perhaps you
> used XCreateWindow() to create a window inside your hierarchy,
> and then called gdk_window_foreign_new() on it? That case is
> certainly a bit problematical now:
>
>  - The reparent-to-the-root behavior isn't right for that case
>  - If you call XDestroyWindow() yourself the window won't be
>    removed from parent->children().
>
> So, to make it work, you'd need to gdk_window_destroy() first,
> then call XDestroyWindow()
>
> The other possibility is that you didn't select for StructureNotify
> on the foreign window, then GTK+ will never get notification 
> and not remove the window from the XID table.
>   


Let me try to explain the usecase: the code is from the Task Navigator
on the Hildon Desktop. We are trying to keep track of all the top-level
window creation and destruction. When we detect (from the WM client
list) that a new window has appeared, we need to install an event filter
on it. We use the following code:

  gdk_wrapper_win = gdk_window_foreign_new
(hn_wm_watched_window_get_x_win(win));
 
  if (gdk_wrapper_win != NULL)
    {
      /* Monitor the window for prop changes */
      gdk_window_set_events(gdk_wrapper_win,
                gdk_window_get_events(gdk_wrapper_win)
                | GDK_PROPERTY_CHANGE_MASK);
     
      gdk_window_add_filter(gdk_wrapper_win,
                hn_wm_x_event_filter,
                NULL);
    }


Now what would be the correct way to dispose of this GdkWindow?
Currently, when the window disappears from the WM's client list, we
called gdk_window_destroy () on the window. But the refcount at this
point seems to be quite unpredictable, and we end up leaking the object.