gdk_window_get_root_origin can cause BadWindow error and program crash

["Hjortland" <hahjortland tande com>]

I think I have found a bug in gdk_window_get_root_origin. Here's my theory:

Look at this piece of code from gdk_window_get_root_origin:
******************************************************
  do
    {
      xwindow = xparent;
      if (!XQueryTree (private->xdisplay, xwindow,
                       &root, &xparent,
                       &children, &nchildren))
        return;

      if (children)
        XFree (children);
    }
  while (xparent != root);
******************************************************

The problem is that a window can be deleted between two calls to XQueryTree.
Then, when querying the xwindow, it doesn't exist, and X gives a BadWindow
error.
Yes, it's a rare bug, but when it strikes, it's serious. The program gives a
GDK error (BadWindow), then quits momentarily.

I experienced this bug when I was working on a program. I assigned a key as
the accelerator for a menu item which shows/hides a small window. When
starting the program (which is a bit CPU-intensive), and holding down the
key at the same time, the little window will appear and disappear rapidly.
Then, at some point, the program suddenly gives the GDK error and quits.


Quickly and briefly looking through the GTK+ code, I saw other functions
which might have the bug too. These are:
gdk_input_get_root_relative_geometry
gdk_input_find_root_child
gdk_window_get_deskrelative_origin
gxi_find_root_child


Here is my suggestion for a fix for gdk_window_get_root_origin.
I'm quite new to this, though, so for all I know, this code could be even
buggier...

Maybe XGrabServer could be used instead of gdk_error_trap_push, because then
it wouldn't be necessary with the error check, right?

******************************************************
void
gdk_window_get_root_origin (GdkWindow *window,
                            gint      *x,
                            gint      *y)
{
  GdkWindowPrivate *private;
  Window xwindow;
  Window xparent;
  Window root;
  Window *children;
  unsigned int nchildren;
  int xerrors;
  xerrors = 10; /* Tolerate at most this many errors */

  g_return_if_fail (window != NULL);

  private = (GdkWindowPrivate*) window;
  if (x)
    *x = 0;
  if (y)
    *y = 0;
  if (private->destroyed)
    return;

  while (private->parent && ((GdkWindowPrivate*) private->parent)->parent)
    private = (GdkWindowPrivate*) private->parent;
  if (private->destroyed)
    return;

  query:
  xparent = private->xwindow;
  do
    {
      xwindow = xparent;
      /* A window may disappear between two calls to XQueryTree,
         and we could therefor accidentaly run XQueryTree on a non-existing
         window, so don't let X errors be fatal to the program */

      /* FIXME: Should we flush, so that we don't get errors that are not
ours?
      gdk_flush(); */
      gdk_error_trap_push();
      if (!XQueryTree (private->xdisplay, xwindow,
                       &root, &xparent,
                       &children, &nchildren))
        {
          gdk_flush (); /* Catch errors now */
          gdk_error_trap_pop ();
          if (--xerrors >= 0)
            goto query;
          else
            return;
        }

      if (children)
        XFree (children);

      gdk_flush (); /* Catch errors now */
      if (gdk_error_trap_pop ())
        {
          if (--xerrors >= 0)
            goto query;
          else
            return;
        }
    }
  while (xparent != root);

  if (xparent == root)
    {
      unsigned int ww, wh, wb, wd;
      int wx, wy;

      /* FIXME: Should we flush, so that we don't get errors that are not
ours?
      gdk_flush(); */

      gdk_error_trap_push ();
      if (XGetGeometry (private->xdisplay, xwindow, &root, &wx, &wy, &ww,
&wh, &wb, &wd))
        {
          if (x)
            *x = wx;
          if (y)
            *y = wy;
        }

      gdk_flush (); /* Catch errors now */
      if (gdk_error_trap_pop ())
        {
          if (--xerrors >= 0)
            goto query;
          else
            return;
        }
    }
}
******************************************************




-- Haakon Andre Hjortland