GNOME Bugzilla – Bug 764374
Busy loop while "Displays" page is active
Last modified: 2016-04-05 16:34:46 UTC
Created attachment 325028 [details] backtrace from gdb attached to g-c-c while in busy loop Reproducible: Always for me on the only machine tested (Gnome + Wayland) Steps to reproduce: 1. open gnome-control-center 2. open "Display" page 3. look into gnome-system-monitor or htop What happens: gnome-control-center is running a busy loop with 100% CPU on one core What should happen: No busy loop eating battery power if accidentally left gnome-control-center open Affected version: gnome-control-center 3.20.0 gtk 3.20.0 Additional info: I haven't tested this on Gnome+X11, just on Gnome+Wayland
In fact, this happens on every panel. It's caused by code in cc-window.c calling gtk_window_resize() on configure-event signals. Now, this might well be considered a bug in g-c-c but it seems to me that the gdk wayland backend could also fizzle out in gdk_wayland_window_configure() if width, height and scale aren't changing. Except that that short circuiting was explicitly removed in bug 758901 . CCing Olivier to get his opinion.
(In reply to Rui Matos from comment #1) > In fact, this happens on every panel. It's caused by code in cc-window.c > calling gtk_window_resize() on configure-event signals. You're right about that.
(In reply to Rui Matos from comment #1) > In fact, this happens on every panel. It's caused by code in cc-window.c > calling gtk_window_resize() on configure-event signals. > > Now, this might well be considered a bug in g-c-c but it seems to me that > the gdk wayland backend could also fizzle out in > gdk_wayland_window_configure() if width, height and scale aren't changing. > > Except that that short circuiting was explicitly removed in bug 758901 . > CCing Olivier to get his opinion. Yes, this shortcut was removed because there might be a pending configure from the compositor that gdk has not processed yet, so ignoring the configure leads sometimes to the wrong size being applied (this was bug 75890). Yet I reckon g-c-c responding to a configure event with a window resize can lead to all sort of races (and not just in Wayland) so we should avoid that. A possibility is to avoid to loop in gdk_window_wayland_move_resize() instead (gdk_wayland_window_configure() is being called from both gdk_window_wayland_move_resize() and xdg_surface_configure() -and window_update_scale() as well but that's irrelevant for this case-), that would fix this issue without reverting the fix for bug 75890
Created attachment 325068 [details] [review] [PATCH] wayland: do not resize with the same size I think such a patch in gdk Wayland backend would fix the busy loop in g-c-c without reverting the fix for bug 758901 but might possibly have other side effects I might have not foreseen, so needs testing!
Actually, thinking of it, I think this patch is pretty safe because it just goes back to where we were before bug 758901 for gtk_window_resize() -ie not resize/configure if size is the same- while at the same time it's not reverting the fix for bug 758901. If you agree, I'd move that to gtk+ for review of the fix...
=> Moving to gtk+
Still needs testing ?
(In reply to Matthias Clasen from comment #7) > Still needs testing ? For some reaon, I cannot reproduce the issue described in comment #0 so I cannot test if this is fixed with attachment 325068 [details] [review] But, based on comment #1 I reckon it should fix it. Would be good if we could get a confirmation of the fix though.
Created attachment 325336 [details] [review] [PATCH] wayland: Do not resize with the same size Updated patch
Created attachment 325339 [details] [review] [PATCH] wayland: Do not resize with the same size Argh, sorry, forgot tot amend the patch to fix the typo prior to uploading...
Review of attachment 325339 [details] [review]: this works for me
Review of attachment 325339 [details] [review]: I must say that I find the "from_configure" parameter naming mildly confusing in a function called gdk_wayland_window_configure, and looking at the callers, the extra TRUE/FALSE doesn't really communicate what is going on. An alternative approach might be to have a separate function, say gdk_wayland_window_maybe_configure() which would do the shortcut, and then call gdk_wayland_window_configure().
Created attachment 325357 [details] [review] [PATCH v4] wayland: Do not resize with the same size (In reply to Matthias Clasen from comment #12) > Review of attachment 325339 [details] [review] [review]: > > I must say that I find the "from_configure" parameter naming mildly > confusing in a function called gdk_wayland_window_configure, and looking at > the callers, the extra TRUE/FALSE doesn't really communicate what is going > on. An alternative approach might be to have a separate function, say > gdk_wayland_window_maybe_configure() which would do the shortcut, and then > call gdk_wayland_window_configure(). Sure, makes sense, updated patch attached.
Review of attachment 325357 [details] [review]: thanks, looks ok to me
Comment on attachment 325357 [details] [review] [PATCH v4] wayland: Do not resize with the same size attachment 325357 [details] [review] pushed as commit be6784c on master and as commit dee6d13 on gtk-3-20