After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 627210 - Crash with X error
Crash with X error
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other Linux
: Normal major
: ---
Assigned To: Dan Winship
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2010-08-17 20:30 UTC by Giovanni Campagna
Modified: 2010-08-19 20:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Mutter verbose log (compressed) (19.83 KB, application/x-bzip)
2010-08-17 21:19 UTC, Giovanni Campagna
  Details
MutterShapedWindow: drop all indirect references to textures (4.99 KB, patch)
2010-08-18 21:00 UTC, Owen Taylor
committed Details | Review

Description Giovanni Campagna 2010-08-17 20:30:18 UTC
Reproducibly when moving windows, and at random times in other cases, but very soon any way, mutter crashes (SIGABRT) with the following output in the terminal:

Window manager warning: Log level 16: Using Cairo rendering requires the drawable argument to
have a specified colormap. All windows have a colormap,
however, pixmaps only have colormap by default if they
were created with a non-NULL window argument. Otherwise
a colormap must be set on them with gdk_drawable_set_colormap
Window manager warning: Log level 8: gdk_pixbuf_get_from_surface: assertion `surface != NULL' failed
Bug in window manager: Unexpected X error: BadDrawable (invalid Pixmap or Window parameter) serial 5955 error_code 9 request_code 137 minor_code 4)

Command line: LANG=C jhbuild -f gnome-shell.jhbuildrc run mutter --replace

Version is git master (with cairo theme rendering), cairo 1.9.14, libX11 1.3.11
Comment 1 Giovanni Campagna 2010-08-17 21:19:14 UTC
Created attachment 168140 [details]
Mutter verbose log (compressed)
Comment 2 Owen Taylor 2010-08-17 21:21:40 UTC
I have patches for this locally, need to sort out what parts are needed and why and what parts are just voodo.
Comment 3 Owen Taylor 2010-08-17 22:43:37 UTC
Pushed a trivial one-liner to fix this (move XFreePixmap
after clutter_x11_texture_pixmap_set_pixmap(..., None).
Comment 4 Owen Taylor 2010-08-18 21:00:25 UTC
Created attachment 168228 [details] [review]
MutterShapedWindow: drop all indirect references to textures

Last patch wasn't fully working because we weren't receiving notifications
as expected when unsetting the Pixmap from the ClutterTexturePixmapX11.
This patch works around it.

When we want to depend on Clutter 1.4 hard we should just derive
MutterShapedTexture directly from ClutterActor, and we'll be able to actuall
understand what is going wrong.
Comment 5 Dan Winship 2010-08-19 19:07:56 UTC
Comment on attachment 168228 [details] [review]
MutterShapedWindow: drop all indirect references to textures

I really don't know anything about this code, and I suspect that it would take me longer to figure it out than it would take for us to find bugs in your patch by just committing it and waiting for bug reports...

I can say that this looks weird though:

>+  mutter_texture_tower_set_base_texture (priv->paint_tower, COGL_INVALID_HANDLE);
>+
>+  if (priv->material != COGL_INVALID_HANDLE)
>+    cogl_material_set_layer (priv->material, 0, COGL_INVALID_HANDLE);
>+
>+  mutter_texture_tower_set_base_texture (priv->paint_tower, COGL_INVALID_HANDLE);

Assuming one of the two mutter_texture_tower_set_base_texture() calls isn't just a cut+pasto, it seems like the second one would only be necessary inside the body of the if statement?
Comment 6 Owen Taylor 2010-08-19 20:09:25 UTC
(In reply to comment #5)
> (From update of attachment 168228 [details] [review])
> I really don't know anything about this code, and I suspect that it would take
> me longer to figure it out than it would take for us to find bugs in your patch
> by just committing it and waiting for bug reports...
> 
> I can say that this looks weird though:
> 
> >+  mutter_texture_tower_set_base_texture (priv->paint_tower, COGL_INVALID_HANDLE);
> >+
> >+  if (priv->material != COGL_INVALID_HANDLE)
> >+    cogl_material_set_layer (priv->material, 0, COGL_INVALID_HANDLE);
> >+
> >+  mutter_texture_tower_set_base_texture (priv->paint_tower, COGL_INVALID_HANDLE);
> 
> Assuming one of the two mutter_texture_tower_set_base_texture() calls isn't
> just a cut+pasto, it seems like the second one would only be necessary inside
> the body of the if statement?

Just a cut-and-pasto - there should only be one. That was the kind of stuff I was hoping that would be found, though this isn't as bad as the mistake I pushed on Tuesday.
Comment 7 Owen Taylor 2010-08-19 20:13:15 UTC
Pushed with the duplicate line removed.

Attachment 168228 [details] pushed as 8b34b4b - MutterShapedWindow: drop all indirect references to textures