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 399529 - apps requesting huge windows cause BadAlloc in metacity
apps requesting huge windows cause BadAlloc in metacity
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: general
trunk
Other Linux
: Normal normal
: ---
Assigned To: Thomas Thurman
Metacity maintainers list
Depends on:
Blocks:
 
 
Reported: 2007-01-22 20:13 UTC by Benjamin Otte (Company)
Modified: 2007-04-12 16:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
kill metacity in 5 easy steps (119 bytes, text/x-python)
2007-01-22 20:14 UTC, Benjamin Otte (Company)
  Details
Stop this killing metacity (1.35 KB, patch)
2007-01-23 00:44 UTC, Thomas Thurman
none Details | Review
Clamp the rectangle to an area slightly larger than the screen (5.20 KB, patch)
2007-01-26 02:50 UTC, Thomas Thurman
committed Details | Review

Description Benjamin Otte (Company) 2007-01-22 20:13:53 UTC
The attached program crashes metacity for me. You might want to experiment with height and width in it to get the crash-effect.
Comment 1 Benjamin Otte (Company) 2007-01-22 20:14:38 UTC
Created attachment 80920 [details]
kill metacity in 5 easy steps
Comment 2 Thomas Thurman 2007-01-23 00:44:19 UTC
Created attachment 80945 [details] [review]
Stop this killing metacity

What's happening here is a bug in gdk, I suppose. If you try to call gdk_window_begin_paint_rect with an insanely huge rectangle, it dies with BadAlloc, as you know from your own program; since metacity uses gtk to paint the windows, it itself will attempt to call gdk_window_begin_paint_rect with an insanely huge rectangle, and die with BadAlloc in the process. (You'll need to set METACITY_SYNC=1 to see this.) That's why other WMs don't have a problem here: they don't use gdk.

So, what can we do about it? I don't know. The attached is a patch to make metacity complain and not decorate the window if it can't do so because of an X error when calling gdk_window_begin_paint_rect. I don't know whether this is the best solution. Elijah, Havoc, others-- any thoughts?
Comment 3 Thomas Thurman 2007-01-23 00:45:02 UTC
s/paint the windows/decorate the windows/, you know what I meant.
Comment 4 Havoc Pennington 2007-01-23 14:53:00 UTC
One possible fix would be to clip the painting to the screen extents (there is a race condition here - the window could move in relation to the screen while drawing - maybe one fix for that would be to clip to a bit larger area than the screen, and skip the clipping for windows smaller than the screen, so only large windows that get moved a lot would repaint incorrectly)

I think metacity might already draw the frame as four stripes on the four sides instead of one big rect, which was also a past attempt to fix this bug if so...
Comment 5 Thomas Thurman 2007-01-26 02:50:57 UTC
Created attachment 81243 [details] [review]
Clamp the rectangle to an area slightly larger than the screen

Good plan. So, here's a patch which clamps the rectangle values not to stray >200px outside the edges of the screen.

While I was working on that, I noticed that we carry out all these calculations for every drawable and then throw them away 1/3 of the time when GDK_IS_WINDOW is false. I refactored the code a little bit to only do these calculations when GDK_IS_WINDOW is true.

Results: The kill.py program no longer kills metacity. Also, window decoration is slightly faster (about 0.1ms for a small simple window, but it all adds up).

Without the patch, metacity-theme-viewer says:

Drew 100 frames in 0.07 client-side seconds (0.7 milliseconds per frame) and 0.493933 seconds wall clock time including X server resources (4.93933 milliseconds per frame)

With the patch:

Drew 100 frames in 0.08 client-side seconds (0.8 milliseconds per frame) and 0.486864 seconds wall clock time including X server resources (4.86864 milliseconds per frame)
Comment 6 Elijah Newren 2007-04-03 17:32:05 UTC
I haven't looked very closely at the patch, but considering the source and the long list of patches needing review...  :)
Comment 7 Thomas Thurman 2007-04-12 15:16:09 UTC
Great, thanks. Committed.
Comment 8 Elijah Newren 2007-04-12 16:24:39 UTC
Assuming Thomas meant to mark this bug fixed too, since he didn't ask for testing/verification.  ;-)