GdkWindow ref counting

[Tim Janik <timj imendio com>]

hey Owen.

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).

(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.

also, note that (2) isn't always applicable for applications, because
gdk_window_foreign_new() may simply hand out a new reference to an
existing window if there already is a GdkWindow for this xid (and it
might even be a valid internal, i.e. "non-foreign" one).

that's why (3) should work for foreigns when it's not sure that the
xid is really a foreign one. however, this case is leaking for the
same reasons as (2).

now we're wondering whether there's really a leak here (basically
removing the additional g_object_ref near the end of foreign_new
could fix this, since it doesn't pair with a future DestroyNotify
to be received).

or, if that "leak" is established/promoted API practise already,
and users are actually expected to treat foreigns like:

4) w = gdk_window_foreign_new();
   if (GDK_WINDOW_IS_FOREIGN (w) &&
       !GDK_WINDOW_IS_DESTROYED (w))
     gdk_window_destroy (w);
   gdk_window_unref (w);

to avoid leaks.

in any case, the intended usage should be made clear in the docs
(it's prolly foreign_new that needs amending here), and properly
documented in the code (i.e. we should add comments next to
go_bject_ref in gdk_window_new and foreign_new which state what
the paired unref is).

as an aside, we wondered whether there's a reason that
gdk_window_destroy does *not* guard its unref call into
if (!GDK_WINDOW_IS_DESTROYED (w)):

void
gdk_window_destroy (GdkWindow *window)
{
  _gdk_window_destroy_hierarchy (window, FALSE, FALSE); // this function DOES guard it's body with !GDK_WINDOW_IS_DESTROYED()
  g_object_unref (window);
}

thanks for consideration.

---
ciaoTJ