heavy window redrawing can lead to creash

[Willem Rein Oudshoorn]

The method -visibleRectForWindow:  can fail if during the traversal of the
tree the (X)Window tree is changed.  If it fails it lead to an crash,
or an infinite sequence of X protocol errors.

This is not as unlikely as it appears, a sure way for me to reproduce it was:

(1) Open GSTest.app
(2) Select NSColorWell test.
(3) Click on border of colorwell to produce the color chooser
(4) Switch to another desktop
(5) Swithc back and BINGO

The strange thing is that GWorkspace with 6 window open did not
exhibit this behaviour.  But with some effort I could also
get GWorkspace down.  (Moving the mouse frantically over the
application so that windows are mapped in an out a lot).

But this patch should fix this.

It does this by Grabbing the X-Server.  I know, it is not
nice and if someone knows a better way I would like to hear it.

Wim Oudshoorn.




2001-11-21  Willem Rein Oudshoorn  <woudshoo@xs4all.nl>

        * xgps/Source/XGGState.m ([XGGState -visibleRectForWindow:win]): Grab X 
server.


Index: Source/XGGState.m
===================================================================
RCS file: /cvsroot/gnustep/gnustep/core/xgps/Source/XGGState.m,v
retrieving revision 1.66
diff -c -r1.66 XGGState.m
*** Source/XGGState.m   2001/11/21 03:35:32     1.66
--- Source/XGGState.m   2001/11/21 22:21:25
***************
*** 2299,2317 ****
  }
  
  
  - (XRectangle) visibleRectForWindow: (Window) win
  {
    XPoint     offs;    // offset of the argument window in the current window
    XRectangle visible;  // part of argument window that is visible in current 
window
- 
    Window root;     // root is where we stop.
    Window parent;    
    Window *children; // to be ignored
    int x, y, w, h;
    int ignored;
- 
  
!   XGetGeometry (XDPY,
                  win,
                  &root,
                  &x, &y,
--- 2299,2337 ----
  }
  
  
+ /*
+ Why do we Grab the X-server?
+    
+ - well it is possible that during the traversal of this tree
+   other windows are mapped in or are destroyed.  If this happen
+   it will generate X-protocol errors. And crash the application.
+ 
+ It might seem that this is very unlikely, but it is NOT.  One example
+ when it is bound to happen is when you switch desktops.  The window
+ manager will be busy with mapping windows and we will be redrawing
+ and therefore querying the tree.
+ 
+ I could be completely wrong on this (my X-window programming knowledge
+ is very limited). And if there is a way to do it
+ without grabbing the server, PLEASE improve this code.
+ 
+ Wim Oudshoorn 21 Nov 2001.
+ */
+   
  - (XRectangle) visibleRectForWindow: (Window) win
  {
+   Display   *xdpy = XDPY;  // cache this value to avoid refetching it during 
the loop.
    XPoint     offs;    // offset of the argument window in the current window
    XRectangle visible;  // part of argument window that is visible in current 
window
    Window root;     // root is where we stop.
    Window parent;    
    Window *children; // to be ignored
    int x, y, w, h;
    int ignored;
  
!   XGrabServer (xdpy);
!   
!   XGetGeometry (xdpy,
                  win,
                  &root,
                  &x, &y,
***************
*** 2321,2344 ****
  
    visible = XGMakeRect (0, 0, w, h);
    offs = XGMakePoint (x, y);
! 
    do
      {
        XRectangle bounds;
        
!       XQueryTree (XDPY,
!                   win,
!                   &root,
!                   &parent,
!                   &children,
!                   &ignored);
        
        if (children != NULL)
          XFree (children);
  
        win = parent;
        
!       XGetGeometry (XDPY,
                      win,
                      &root,
                      &x, &y,
--- 2341,2372 ----
  
    visible = XGMakeRect (0, 0, w, h);
    offs = XGMakePoint (x, y);
!   
    do
      {
        XRectangle bounds;
+       int        ret;
        
!       ret = XQueryTree (xdpy,
!                         win,
!                         &root,
!                         &parent,
!                         &children,
!                         &ignored);
! 
!       if (!ret)
!         {
!           // if an error occured, assume nothing is visible
!           XUngrabServer (XDPY);
!           return XGMakeRect (0, 0, 0, 0);
!         }
        
        if (children != NULL)
          XFree (children);
  
        win = parent;
        
!       XGetGeometry (xdpy,
                      win,
                      &root,
                      &x, &y,
***************
*** 2360,2365 ****
--- 2388,2395 ----
        
      } while (root != win);
    
+ 
+   XUngrabServer (xdpy);
  
    return visible;
  }