[PATCH 5/5] Fix window mapping with re-used window ids.

Forest Bond forest at alittletooquiet.net
Sun Jun 13 08:58:44 PDT 2010


The X server can re-use window ids once a window has been destroyed.
However, window ids persist in xcompmgr until the window has finished
fading out.  As a result, xcompmgr can have multiple win structures
with the same window id.

If a window is destroyed and a new window with the same id is added
and subsequently mapped before the first window fades out, xcompmgr
finds the wrong win structure and tries to map the destroyed window,
failing due to X errors.  The new window is never mapped and never
appears.

This patch fixes this problem by adding a "destroyed" field to the win
structure and only considering non-destroyed windows when matching wins
on window id.

Signed-off-by: Forest Bond <forest at alittletooquiet.net>
---
 xcompmgr.c |   99 ++++++++++++++++++++++++++++++++----------------------------
 1 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/xcompmgr.c b/xcompmgr.c
index a345bbe..c5a7dea 100644
--- a/xcompmgr.c
+++ b/xcompmgr.c
@@ -67,6 +67,7 @@ typedef struct _win {
     Bool		usable;		    /* mapped and all damaged at one point */
     XRectangle		damage_bounds;	    /* bounds of damage */
 #endif
+    int			destroyed;
     int			mode;
     int			damaged;
     Damage		damage;
@@ -753,7 +754,7 @@ find_win (Display *dpy, Window id)
     win	*w;
 
     for (w = list; w; w = w->next)
-	if (w->id == id)
+	if ((!w->destroyed) && (w->id == id))
 	    return w;
     return NULL;
 }
@@ -1456,7 +1457,7 @@ add_win (Display *dpy, Window id, Window prev)
     if (prev)
     {
 	for (p = &list; *p; p = &(*p)->next)
-	    if ((*p)->id == prev)
+	    if ((!(*p)->destroyed) && ((*p)->id == prev))
 		break;
     }
     else
@@ -1471,6 +1472,7 @@ add_win (Display *dpy, Window id, Window prev)
 #endif
 	return;
     }
+    new->destroyed = 0;
     new->damaged = 0;
 #if CAN_DO_USABLE
     new->usable = False;
@@ -1537,7 +1539,7 @@ restack_win (Display *dpy, win *w, Window new_above)
 	/* rehook */
 	for (prev = &list; *prev; prev = &(*prev)->next)
 	{
-	    if ((*prev)->id == new_above)
+	    if ((!(*prev)->destroyed) && ((*prev)->id == new_above))
 		break;
 	}
 	w->next = *prev;
@@ -1628,58 +1630,61 @@ circulate_win (Display *dpy, XCirculateEvent *ce)
 }
 
 static void
-finish_destroy_win (Display *dpy, Window id, Bool gone)
+finish_destroy_win (Display *dpy, win *w, Bool gone)
 {
-    win	**prev, *w;
+    win *prev;
 
 #if DEBUG_WINDOWS
-    printf ("finish_destroy_win: 0x%x\n", id);
+    printf ("finish_destroy_win: 0x%x\n", w->id);
 #endif
 
-    for (prev = &list; (w = *prev); prev = &w->next)
-	if (w->id == id)
-	{
-	    if (gone)
-		finish_unmap_win (dpy, w);
-	    *prev = w->next;
-	    if (w->picture)
-	    {
-		set_ignore (dpy, NextRequest (dpy));
-		XRenderFreePicture (dpy, w->picture);
-		w->picture = None;
-	    }
-	    if (w->alphaPict)
-	    {
-		XRenderFreePicture (dpy, w->alphaPict);
-		w->alphaPict = None;
-	    }
-	    if (w->shadowPict)
-	    {
-		XRenderFreePicture (dpy, w->shadowPict);
-		w->shadowPict = None;
-	    }
-	    if (w->shadow)
-	    {
-		XRenderFreePicture (dpy, w->shadow);
-		w->shadow = None;
-	    }
-	    if (w->damage != None)
-	    {
-		set_ignore (dpy, NextRequest (dpy));
-		XDamageDestroy (dpy, w->damage);
-		w->damage = None;
-	    }
-	    cleanup_fade (dpy, w);
-	    free (w);
+    for (prev = list; prev; prev = prev->next)
+	if (prev->next == w)
 	    break;
-	}
+
+    if (! prev)
+	/* Couldn't find the window? */
+	return;
+
+    if (gone)
+	finish_unmap_win (dpy, w);
+    prev->next = w->next;
+    if (w->picture)
+    {
+	set_ignore (dpy, NextRequest (dpy));
+	XRenderFreePicture (dpy, w->picture);
+	w->picture = None;
+    }
+    if (w->alphaPict)
+    {
+	XRenderFreePicture (dpy, w->alphaPict);
+	w->alphaPict = None;
+    }
+    if (w->shadowPict)
+    {
+	XRenderFreePicture (dpy, w->shadowPict);
+	w->shadowPict = None;
+    }
+    if (w->shadow)
+    {
+	XRenderFreePicture (dpy, w->shadow);
+	w->shadow = None;
+    }
+    if (w->damage != None)
+    {
+	set_ignore (dpy, NextRequest (dpy));
+	XDamageDestroy (dpy, w->damage);
+	w->damage = None;
+    }
+    cleanup_fade (dpy, w);
+    free (w);
 }
 
 #if HAS_NAME_WINDOW_PIXMAP
 static void
 destroy_callback (Display *dpy, win *w, Bool gone)
 {
-    finish_destroy_win (dpy, w->id, gone);
+    finish_destroy_win (dpy, w, gone);
 }
 #endif
 
@@ -1690,13 +1695,14 @@ destroy_win (Display *dpy, Window id, Bool gone, Bool fade)
 #if DEBUG_WINDOWS
     printf ("destroy_win: 0x%x\n", w->id);
 #endif
+    w->destroyed = 1;
 #if HAS_NAME_WINDOW_PIXMAP
     if (w && w->pixmap && fade && fadeWindows)
 	set_fade (dpy, w, w->opacity*1.0/OPAQUE, 0.0, fade_out_step, destroy_callback, gone, False, True);
     else
 #endif
     {
-	finish_destroy_win (dpy, id, gone);
+	finish_destroy_win (dpy, w, gone);
     }
 }
 
@@ -1704,8 +1710,9 @@ destroy_win (Display *dpy, Window id, Bool gone, Bool fade)
 static void
 dump_win (win *w)
 {
-    printf ("\t%08lx: %d x %d + %d + %d (%d)\n", w->id,
-	    w->a.width, w->a.height, w->a.x, w->a.y, w->a.border_width);
+    printf ("\t%08lx: %d x %d + %d + %d (%d)%s\n", w->id,
+	    w->a.width, w->a.height, w->a.x, w->a.y, w->a.border_width,
+	    w->destroyed ? " (destroyed)" : "");
 }
 
 
-- 
1.7.0.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100613/fe3efe9d/attachment-0001.pgp>


More information about the xorg-devel mailing list