Comment 6 for bug 753269

Revision history for this message
Ugo Riboni (uriboni) wrote :

@RIchard

Thanks for your great analysis so far. It was extremely helpful in figuring out this bug.
From my debugging it looks like the root cause of the bug is the following:

- We send a window to another workspace, and this cause the WindowInfo object associated with it to be removed from the proxy model managing the origin workspace and to be added to the proxy model managing the destination workspace.

- The WM also unmaps the window when it is sent to the other workspace.

- When this happens, QML creates a new delegate for the window in QML associated to the new model. This causes a new request to the WindowImageProvider to get the image for the window (this is actually a major waste, see notes at the end).

- When loading the image there are essentially two paths, one if the window is still mapped that will directly use the window's X11 drawable. The other is when the window is already unmapped and it will try to use an X11 pixmap that metacity has stored in a property of the window with a screenshot of the image before it was unmapped. In both cases the drawable is then converted to a QImage so that is can be used freely in QML without worrying about its X11 counterpart.

- When the crash happens it is because of a timing issue: when we check if the window is mapped, it is still in the current workspace and thus mapped. So we just grab the window drawable. However by the time we get to convert this drawable to a QImage the WM has unmapped the window, so the drawable is not valid anymore and the call to XGetImage inside QX11PixmapData::toImage fails and returns NULL. QT has a Q_CHECK_PTR just after that which raises the exception you discovered.

On a higher level it appears that the wnck event "workspace-changed" that we get from WNCK is wrong and the window isn't fully on the other workspace yet when it is emitted.

This seems to be an issue in Qt more than anything else since it shouldn't raise an exception there, it should just return an empty QImage (which can then be checked for .isNull()). We can however try to catch the exception and try the entire function again.

One more indirect solution is to wait to change the "workspace" property of the WindowInfo until we receive the notification that the window has been unmapped.

Note that there's also another issue here: no new windows should be added to the WindowsList while the spread is not visible !
Fixing that other problem will also make very very rare the possibility of having this crash happen again, since while the spread is visible there's no way for the user to change the workspace of a window. So we should be safe unless in two cases:
- some other process moves windows around between workspaces while we're in the spread. or the window moves itself.
- in the future we implement this feature in the spread (with drag and drop like Unity already has)