Creating windows above just-destroyed windows causes newly created windows to receive invalid stack positions

Bug #1088399 reported by Sam Spilsbury
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Compiz
Fix Released
High
Sam Spilsbury
0.9.9
Fix Released
High
Sam Spilsbury
compiz (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

There's a race condition in the stacking code which can cause windows to receive invalid stack positions.

This sample program shows how that race condition might happen over two display connections:

#include <X11/Xlib.h>
#include <stdio.h>

int main ()
{
    Display *d = XOpenDisplay (NULL);
    Display *other = XOpenDisplay (NULL);
    Window root = DefaultRootWindow (d);
    Window container = XCreateSimpleWindow (d, root, 100, 100, 100, 100, 0, 0, 0);

    Window w1 = XCreateSimpleWindow (d, container, 0, 0, 10, 10, 0, 0xffff00, 0xffff00);
    Window w2 = XCreateSimpleWindow (d, container, 1, 1, 10, 10, 0, 0xff0000, 0xff0000);
    Window w3 = XCreateSimpleWindow (d, container, 2, 2, 10, 10, 0, 0x00ff00, 0x00ff00);

    /* Map all three */
    XMapWindow (d, w1);
    XMapWindow (d, w2);
    XMapWindow (d, w3);

    XMapWindow (d, container);

    /* Destroy 1 */
    XDestroyWindow (d, w1);
    XSync (d, 0);

    /* Stack 3 above 1 */
    XWindowChanges xwc;
    xwc.stack_mode = Above;
    xwc.sibling = w1;
    XConfigureWindow (other, w3, CWSibling | CWStackMode, &xwc);

    Window *children = NULL;
    unsigned int nchildren;
    Window dummy;

    XQueryTree (other, container, &dummy, &dummy, &children, &nchildren);
    int i = 0;
    for (i = 0; i < nchildren; ++i)
    {
 printf ("0x%x\n", (unsigned int) children[i]);
    }

    XCloseDisplay (d);
    XCloseDisplay (other);
    return 0;
}

The problem is that configuring a window to be stacked relative to a destroyed window is an invalid operation, however, there's a race condition where if we are between handling events, we may find a suitable candidate to stack above, however we may not have received a DestroyNotify for that candidate yet. If we issue a ConfigureWindow request to stack above that candidate, it will fail with a BadWindow error when we later flush the output buffer, however we didn't really have a way of knowing it was going to fail (and furthermore, we ignore X errors, a bad thing in itself, but a separate bug), so that would cause us to mark the window as having successfully being restacked internally whereas in reality it wasn't restacked at all. Then when we restack other windows relative to the window that failed the restack, they are going to receive positions which are relative to its /original/ position (and not its changed position), which is usually above everything else and invalid.

From a user point of view, I can reproduce this by opening ~20 or so gnome-terminals and restarting compiz in the classic session. The terminals will end up above the panels.

Related branches

Changed in compiz:
importance: Undecided → High
assignee: nobody → Sam Spilsbury (smspillaz)
milestone: none → 0.9.9.0
status: New → In Progress
Changed in compiz:
milestone: 0.9.9.0 → 0.9.9.2
Changed in compiz:
status: In Progress → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package compiz - 1:0.9.9~daily13.02.19-0ubuntu1

---------------
compiz (1:0.9.9~daily13.02.19-0ubuntu1) raring; urgency=low

  [ Sam Spilsbury ]
  * debian/patches/100_workaround_virtualbox_hang.patch
    - VirtualBox uses a shared-memory texture_from_pixmap_implementation
    which is not compatible with our server grab usage, so force
    a different bind method when running with virtualbox and binding
    an externally managed texture

  [ Brandon Schaefer ]
  * Unity rendered behind windows (LP: #906231)

  [ MC Return ]
  * unmaximize_window_key instead of unmaximize_or_minimize_window_key
    exposed in g-c-c (LP: #1115128)

  [ Sam Spilsbury ]
  * [needs-packaging] Please upload: esvn merged (LP: #112433)
  * [nvidia] Windows appear blank white (LP: #729979)
  * Threads not found on CI (LP: #1124133)
  * Unity rendered behind windows (LP: #906231)
  * Compiz hangs in glXBindTexImageEXT in VirtualBox (LP: #1127866)
  * 1:0.9.8+bzr3319-0ubuntu1 regression: keeps setting gsettings keys to
    wrong values (LP: #1063617)
  * Creating windows above just-destroyed windows causes newly created
    windows to receive invalid stack positions (LP: #1088399)

  [ Marco Trevisan (Treviño) ]
  * Calling setOptionForPlugin does not work for core options (LP:
    #1122228)

  [ Automatic PS uploader ]
  * Automatic snapshot from revision 3611
 -- Automatic PS uploader <email address hidden> Tue, 19 Feb 2013 08:46:08 +0000

Changed in compiz (Ubuntu):
status: New → Fix Released
Changed in compiz:
milestone: 0.9.9.2 → 0.9.10.0
Stephen M. Webb (bregma)
Changed in compiz:
status: Fix Committed → Fix Released
Revision history for this message
Stephen M. Webb (bregma) wrote :

Closing Compiz 0.9.9 task (0.9.9 series is obsolete)

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.